buddypress / next-template-packs

is this the next BuddyPress template pack?
35 stars 9 forks source link

Use HTML5 `nav` element for semantic markup #166

Closed mercime closed 7 years ago

mercime commented 7 years ago

While adding aria-labels for #165, noticed that the divs are used as wrappers for the navigation links instead of navs. Will need to double-check if anything in CSS or JS is hooking into e.g. div.subnav instead of .subnav before changing anything.

hnla commented 7 years ago

Left a response to this that's vanished - did I not hit 'comment' sigh

Yes please change them, don't be too concerned about selectors I haven't been using element selectors and any around either are for a reason or we'll remove. Markup should take precedence if necessary I can go around and fix any issues afterwards.

mercime commented 7 years ago

Left a response to this that's vanished - did I not hit 'comment' sigh

It happened to me before. Nowadays, if I have to write more than a paragraph especially on BP Trac, I do it on Notepad first :D

Yes please change them, don't be too concerned about selectors I haven't been using element selectors and any around either are for a reason or we'll remove. Markup should take precedence if necessary I can go around and fix any issues afterwards.

Thank you. Working on it.

mercime commented 7 years ago

@hnla, Did not change this linked div to nav as I noticed that it was not wrapping anything, i.e. empty div container, unless I missed something.

hnla commented 7 years ago

It's a Backbone placeholder (for want of the proper term) the view is injected into it via the invites.js setupNav: function() { this concerns me a little tbh as we don't have a sub menu if no JS but hey ho.

I'll change up and commit.

mercime commented 7 years ago

Thanks hnla.