I'm using Responsive Menus along with an Omega 3.1 subtheme. According to the Omega documentation these are the breakpoints in my theme:

0px to 740px wide - global.css
741px to 979px wide - global.css + default.css + narrow.css
980px to 1219px wide - global.css + default.css + narrow.css + normal.css
1220px and wider - global.css + default.css + narrow.css + normal.css + wide.css

I set the Responsive Menu breakpoint at 740/741px. Unfortunately, it doesn't break in sync with Omega (= CSS media queries), but at a much greater width.

This issue was reported previously, now marked "fixed" and "closed": #2145933: Screen width to respond to is not working properly Please let me know if there's a way to use this module so it works nicely with Omega. Thanks.

CommentFileSizeAuthor
#24 window_innerwidth-2193735-24.patch10.81 KBloopduplicate
#24 interdiff-2193735-8-10.txt10.81 KBloopduplicate
#10 window_innerwidth-2193735-10.patch3.54 KBAnonymous (not verified)
#8 window_innerwidth-2193735-8.patch14.35 KBAnonymous (not verified)

Comments

jwjoshuawalker’s picture

jwjoshuawalker commented

Correct. document.documentElement.clientWidth does not always line up with CSS Media Queries (what Omega is using) IF there is a vertical scroll bar present.

I made a demo page showing an example:
https://drastikbydesign.com/demo/media-query-vs-documentdocumentelementc...

The media query is set to react at 800px.
The numbers match up perfectly, but if you create a vertical scroll bar (open console/inspector in Chrome/Firefox etc - in order for page to have a scroll bar), and you will see that media query still responds as if 800px, but window width reports 15px less than what media query thinks.

If your page always has a scroll bar, just try setting your Responsive Menus trigger 15px offset. (In my versions, Chromium is 15px, FF is 13px offset).

jwjoshuawalker’s picture

jwjoshuawalker commented

More information here:
http://www.quirksmode.org/css/tests/mediaqueries/width.html
Good info down in the "Equality notes" section, however #4 is wrong. The media query is excluded, but is not equal to document.documentElement.clientWidth.

It is possible that window.innerWidth would match your media queries, but it has it's own pitfalls.

Basically we are in a state of flux on this topic, and the width reported is going to vary between browsers and between OS+browsers.
Media queries seem to always report full browser window width (scroll bar ignored), whereas the javascript methods are affected by scroll bar (and the methods that aren't have other issues e.g. window.innerWidth). I originally made this module using window.innerWidth but changed to:
document.documentElement.clientWidth || document.body.clientWidth for a few reasons.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Closed (fixed)

Offsetting the trigger by 15px seems to work fine. Many thanks for the detailed explanations.

jwjoshuawalker’s picture

jwjoshuawalker commented

This is still bugging me. Since it varies between browsers, I don't want to leave this in a "sort of works" state.

I'm going to dig more on using JS native window.innerWidth (not jQuery(window).innerWidth()), since the latter reports the same as our current method. We lose IE8 and under by doing so, but we can have the fallback to the current method for those.

jwjoshuawalker’s picture

jwjoshuawalker commented
Title: Support Omega theme » Change window detection back to window.innerWidth
Version: 7.x-1.5 » 7.x-1.x-dev
Component: Simple Style » Code
Status: Closed (fixed) » Needs work
  • Make sure there are no pitfalls with window.innerWidth.
  • Fallback to document.documentElement.clientWidth || document.body.clientWidth for IE8 & under.
  • Test with major browsers to ensure they all report the same.
    • Chrome / Chromium
    • FireFox
    • Safari
    • Opera
    • Internet Exploder
Kendall Totten’s picture

Kendall Totten commented

Any update on this? Thank you!

jwjoshuawalker’s picture

jwjoshuawalker commented

Hi Kendall,

This one is a super quick fix. Of course patches are welcome :)
I should have some time in June to start knocking out a bunch of the items in queue.

Anonymous’s picture

Anonymous (not verified) commented
Category: Support request » Feature request
Status: Needs work » Needs review
StatusFileSize
new window_innerwidth-2193735-8.patch14.35 KB

I noticed that the window width I entered in Superfish's settings exactly matched my media queries (as I'd like this module to do). It seems they use window.innerWidth (https://github.com/mehrpadin/Superfish-for-Drupal/blob/master/sfsmallscr...), so it can't be that bad a solution...

Here's a patch that simply replaces all instances of
= document.documentElement.clientWidth || document.body.clientWidth;
with
= window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;.

jwjoshuawalker’s picture

jwjoshuawalker commented

@BWPanda,

Thanks for the patch.

I noticed that jquery.meanmenu.js was using document.documentElement.clientWidth.
It looks like they were compensating for the px offset elsewhere in the code, and this change is not desired for the MM library.

Based on testing here: http://drastikbydesign.com/demo/media-query-vs-documentdocumentelementcl...
And here: http://drastikbydesign.com/demo/responsive-menus-demo-1-mean-menu
(It is set to change @ 800px).

Let me know if I'm seeing it wrong.

All the others look good, and we can commit with the MM change taken out (if it is working correctly with the prior detection method).

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new window_innerwidth-2193735-10.patch3.54 KB

Here's the same patch but without the changes to the meanMenu style.

jwjoshuawalker’s picture

jwjoshuawalker commented
Status: Needs review » Fixed

Thanks for the patch BWPanda!

  • drastik committed 6636241 on 7.x-1.x authored by BWPanda 
    Issue #2193735 by BWPanda | Edith Illyés: Added Change window detection...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

flaw’s picture

flaw commented
Status: Closed (fixed) » Needs review

First off, thanks for a great module!

I'm having the same problem with the breakpoints not syncing after the updating to the latest dev version 2014-Sep-06. (i'm using the Foundation theme which is breaking correctly)
I want the break point at 640px and that will break correctly without a vertical scroll bar at this point. But when the scroll bars are visible (which is most of the time on this site) it will break at 657px in Chrome.

So that means in order to break at 640px with a scroll bar in chrome I need to set the value in responsive menus to 623px.

jwjoshuawalker’s picture

jwjoshuawalker commented

@flaw
Which style?

flaw’s picture

flaw commented

sorry mean-menu

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
Status: Needs review » Reviewed & tested by the community

Same issue with menu simple expanding (responsive_menus_simple) on version 1.5.
Patch #10 works fine.

#14 you have to change every occurence of document.documentElement.clientWidth || document.body.clientWidth; by window.innerWidth || document.documentElement.clientWidth; in jquery.meanmenu.js and jquery.meanmenu.min.js (not easy in the minified version).

@drastik : Seems that this module uses version 2.0.6 of jQuery meanMenu. The version 2.0.8 (https://github.com/meanthemes/meanMenu) fix this issue. Maybe you could update this library, in the next release ?

tim_dj’s picture

tim_dj
Location Netherlands
commented

I can confirm that updating mean menu to 2.0.8 fixes this for mean menu.

steve65140’s picture

steve65140 commented

I too can confirm that updating Mean Menu to version 2.0.8 (I'm using Responsive Menus 7.x-1.5) solved this for me.

jwjoshuawalker’s picture

jwjoshuawalker commented

Just update to responsive_menus 7.x-dev version which includes newer MeanMenu.

There will finally be a 7.x-1.6 soonish.

matkeane’s picture

matkeane commented

Just a note: While working locally, I found that the iOs simulator doesn't return correct values for window.innerWidth, but does for document.body.clientWidth.

In order to get things working with the simulator, I had to change the order of detection to: document.body.clientWidth || window.innerWidth || document.documentElement.clientWidth.

loopduplicate’s picture

loopduplicate commented

Updating to 7.x-1.6 did not fix this for me. Using 7.x-1.5 with #8, however, works. I'm using the Mean Menu style. Could it be that updating mean menu's width detection is necessary?

jwjoshuawalker’s picture

jwjoshuawalker commented

Sounds like it. That's what the patch in #8 did.

loopduplicate’s picture

loopduplicate commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new interdiff-2193735-8-10.txt10.81 KB
new window_innerwidth-2193735-24.patch10.81 KB

Here's a rerolled patch which includes the mean menu update from #8.

jwjoshuawalker’s picture

jwjoshuawalker commented
Status: Needs review » Needs work

Is this still needed? Applying patch barks about line endings.

checking file styles/meanMenu/jquery.meanmenu.js Hunk #1 FAILED at 50 (different line endings). Hunk #2 FAILED at 89 (different line endings). Hunk #3 FAILED at 242 (different line endings). Hunk #4 FAILED at 261 (different line endings). 4 out of 4 hunks FAILED checking file styles/meanMenu/jquery.meanmenu.min.js Hunk #1 FAILED at 3. 1 out of 1 hunk FAILED