Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New SideNav structure #1933

Closed
wants to merge 1 commit into from
Closed

Conversation

Mil4n0r
Copy link
Collaborator

@Mil4n0r Mil4n0r commented Apr 2, 2024

Checklist

  • Build process is done without errors. All tests pass in the /lib directory.
  • Self-reviewed the code before submitting.
  • Meets accessibility standards.
  • Added/updated documentation to /website as needed.
  • Added/updated tests as needed.

Description
The previous HTML structure of the SideNav was very poor in terms of semantics.

Closes #1642

@Mil4n0r
Copy link
Collaborator Author

Mil4n0r commented Apr 2, 2024

Required feedback from the rest of the team.

There is still one problem to address. Section compound component is now treated as a ul, however, there are some scenarios where its child is a paragraph or any other text. In my opinion, we should create a specific compound component regarding that scenario. (For example TextSection) in case we don't want to lose that functionality, another more opinionated option would be to simply force the user to use list items as children. (Which is generally the correct case scenario for a SideNav)

@Mil4n0r
Copy link
Collaborator Author

Mil4n0r commented Apr 9, 2024

Closed for now until the topic is discussed.

@Mil4n0r Mil4n0r closed this Apr 9, 2024
@Mil4n0r Mil4n0r added the help wanted Extra attention is needed label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reimplement the Sidenav as a <nav> element
1 participant