Prevent hover from acting on navbar dropdowns in collapsed mode
this commit fixes #73
This will prevent hover from activating a dropdown in a navbar in collapsed mode. If someone has set their navbar collapse breakpoint to something other than the default, then they will need to enter that into the settings.
Do you think we should then get rid of the
if('ontouchstart' in document) return this;
line?
I don't think so. That line is to disable the plugin on touch enabled devices. My updates are to disable the plugin when the menu is in collapsed mode (i.e. acting like an accordion). In collapsed mode each dropdown can have a different height, and so, when moving the mouse from the top down, some items may be skipped if opening on hover. It's not about "mobile" or not, it's about whether the menu is layed out vertically or horizontally.
What about #68?
Well, like I say, it's a different issue. #68 is trying to figure out how to make hover work in situations where there is both touch and mouse control. Here I'm saying, touch or not, you don't want hover to be opening menu items when in collapsed mode. for example you have a menu like:
item 1 -sub 1 -sub 2 -sub 3 item 2 -sub 1 item 3 -sub 1
In collapsed mode, when you move the mouse off the bottom of item 1-sub 3 the menu collapses down, and you will skip over item 2 entirely (maybe item 3 as well, not testing right now). That's why in collapsed mode you want to require a click to open, even if you have a mouse.
Here's an example of the effect I'm discussing http://www.bootply.com/3dPu0COV1A Be sure to size down your browser to less than 768px wide.
So is the use case people with really really small laptops?
I'll admit it is a less common use case (I'm not sure if I noticed it before reading issue #75). I'm developing a site where I have my collapse breakpoint at 992px, and I'm sure some have it higher, but still you'd be unlikely to run into this issue except on a very small netbook, or if someone is browsing without their browser maximised. Maybe the most common case would be someone with their browser at half screen. I know I do that at times, but I'm a web developer, not a normal human being :)
Just running into the same issue. I also admit to having more than one window open :-) I cannot think of any case where an user wants hover functionality in the collapsed menu.
To eliminate the extra needed config for the breakpoint i would change the width < breakpoint check to .navbar-toggle is visible
Addendum:
Also the delayed close after mouseover (line 57) needs this check. Otherwise the menu items in the collapsed menu close automatically.
I've incorporated the changes suggested by Sloothword.
This is looking better. I will try and go over it and see how it behaves
I am nervous to basically change anything at this point without getting some tests to make sure we don't regress. So many little edge cases! See #69.
//Disabled menu hover < 768px .disabled menu hover. It work (with name class bootstraps) you can try
$(window).resize(function () {
if ($(window).width() < 768) {
var horizontalmenu = $("body").find(".horizontal-menu");
horizontalmenu.find(".nav li.dropdown .dropdown-toggle").each(function (e) {
$(this).removeAttr("data-hover");
});
$(".horizontal-menu").html(horizontalmenu.html());
} else {
$(".nav li.dropdown .dropdown-toggle").each(function (e) {
$(this).dropdownHover();
});
};
});
I implemented the changes form @neil-h (commit d0320cb and 5ed1400) Tested in Safari and Chrome without problems. Will also test in iOS Safari...
So does this not break buttons (stuff not in navbars) when on mobile?
@CWSpear can't imagine why this should break any stuff outside the navbar!? Tested on iPhone = OK Tested on iPAD = OK Tested on Desktop Safari = OK Tested on Desktop Chrome = OK Tested on Desktop Windows IE 11 = OK
Sorry, I was thinking about it backwards.
But looking at this PR, if you don't have a navbar, etc,
$(".navbar-toggle").filter(":visible").length
will always be 0, and mess up that isCollapsed() function.
@CWSpear you are right - didn't think about. But this should be fixable easily. I'm not so into JavaScript but how about a first check if the .navbar-toggle exist and then do the length check?
@CWSpear Is your concern that some people will have a navbar without a toggle icon? I'm not sure of a solution for that, but remember that the isCollapsed function works to -disable- the plugin. If there is no .navbar-toggle, then it will return false, and thus the items you hover over -will- dropdown. Only if the item is inside a .navbar-collapsed, and a .navbar-toggle is visible, will the isCollapsed function work to prevent a hover from activating the dropdown.
@gadgetto $(".navbar-toggle").filter(":visible").length basically means "if there is at least one .navbar-toggle, which is visible".
@neil-h ok, I see what you're saying about it really only affecting the nav bar, but it looks like it is still a little dangerous.
By the way, this issue goes away if we use PointerEvents (see commit 6d163831).
That approach requires PointerEvents tho (duh), which are currently only supported on IE, but coming to Chrome and Firefox, and requires jQuery's PointerEvent Polyfill in the interim. However it's the only solution I've found that really solves issues like this one and #68 in a real, solid manner.
This sort of stuff is also very hard to test in an automated fashion, and one of the goals for v3.0 of this plugin is tests. We haven't been able to really get any help on that front, and so the going is very slow.