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

Replace small-menu.js with navigation.js inspired by Twenty Twelve. #66

Merged
merged 1 commit into from
Feb 27, 2013
Merged

Replace small-menu.js with navigation.js inspired by Twenty Twelve. #66

merged 1 commit into from
Feb 27, 2013

Conversation

kovshenin
Copy link
Contributor

Doesn't use jQuery, slightly easier to understand (is it?), uses a media query for max-width, rather than JS events making it faster to respond.

I have a feeling this could be cleaned up even more. Thoughts?

Doesn't use jQuery, slightly easier to understand, uses a media query for max-width, rather than JS events making it faster to respond.
@ianstewart
Copy link
Contributor

I found this slightly harder to understand but it seems like it would be cleaner/faster. If it doesn't break anything, plus 1 from me.

@kovshenin
Copy link
Contributor Author

I would also like to change the h1 tag in the navigation menu (header.php) and post navigation menu (in template-tags.php) to a different tag, maybe h3 or better just an a or something. I'm not sure I understand why they're headings at all, is that an accessibility thing?

How do you feel about changing the tag? What do you think the new tag should be?

@ianstewart
Copy link
Contributor

It's a HTML5 semantics thing, using headings inside sectioning elements, and using h1 for the first heading inside of each sectioning element for consistency, sanity, and portability (you can move that block of code around the document and the heading will always be correctly nested).

@kovshenin
Copy link
Contributor Author

@ianstewart oh, in that case it makes perfect sense, thanks for explaining it :)

@danielimmke
Copy link

This is a good addition. I have used this on a few themes now and it would be nice not to have to manually switch it out.

@philipnewcomer
Copy link

There are plenty of themes that don't use jQuery for their functionality, so it seems a bit like overkill to require jQuery to be loaded on all pages just for small-menu.js.

@mfields
Copy link
Contributor

mfields commented Dec 22, 2012

+1 I used the menu script from Twenty Twelve in my last project and it works great!

@sixhours
Copy link
Contributor

+1!

@sixhours sixhours mentioned this pull request Feb 27, 2013
@michiecat
Copy link
Contributor

I think we should convert to the navigation.js script from Twenty Twelve as soon as we can. The Jetpack mobile theme uses the same small-menu.js as _s, and there were reports of it not working properly in Firefox on Android devices. Switching it to Twenty Twelve's navigation.js fixed the problem.

sixhours added a commit that referenced this pull request Feb 27, 2013
Replace small-menu.js with navigation.js inspired by Twenty Twelve.
@sixhours sixhours merged commit 620b8e5 into Automattic:master Feb 27, 2013
@mtekk mtekk mentioned this pull request Jun 1, 2013
CostyEffe pushed a commit to silverbackstudio/wp-theme that referenced this pull request Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants