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

Changes header navigation to a <nav> element, style by ID #1369

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

devanubis
Copy link
Contributor

@devanubis devanubis commented Jun 1, 2023

Use a <nav> element for the top mavigation bar, instead of role="navitation".

These are semantically equivalent, but its better to style by class or ID than by role/element type, as other content on the page may be marked up as <nav> in future (e.g. the docs side bar).

@cgl
Copy link
Contributor

cgl commented Jun 2, 2023

🐼 Thank you for the PR, could you please enter a description of what it is solving, etc.

djangoproject/scss/_style.scss Outdated Show resolved Hide resolved
djangoproject/scss/_style.scss Outdated Show resolved Hide resolved
Copy link
Member

@sabderemane sabderemane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates @devanubis, I'm afraid there are others changes needed: role="navigation" is used in 3 others pages : search_result.html, feeditem_list.html, blog_pagination.html so changing the style in _style.scss will impact those 3 elements.

Plus, did you check if it's rendering correctly on mobile ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants