-
Notifications
You must be signed in to change notification settings - Fork 212
Fix issue #337, #339, #223 #345
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
Conversation
@elfman Had no chance to test it carefully, though, during a brief try I've noted the following problem:
|
Fixed, please test again. |
@elfman Wow :) These are big changes, and they seem to be working fine :) There is just one more enhancement that would be handy: now if I use switch browser into touch device emulation mode, use touches to play with the menu, then switch to the normal mode and try to interact with the menu neither hovering nor clicks work anymore until I reload the app. I understand, that it is not an issue for production use, as user, in the same session, will not switch like this. Though, for dev/testing routine, it would be super-cool to fix it, so that such switches do not break anything. Also, I guess, it should be just a small modification somewhere, as there is no real need to disable all hover-related logic once you have opened a menu with touch, right? |
@birdofpreyru MouseEnter & MouseLeave Event can also be triggered in mobile device. But their event logic is not fit to mobile device and can't work with touch event. For example, I tap on a menu button when menu opened, the menu should be closed as touch event do so, but the MouseEnter will also be trigger, and it will open the menu again, then the menu looks like nothing happened. |
@elfman No, I don't like listening the width change, as it probably will slow down window resizing (which is already not that fast). Can we just re-enable |
That's a good idea, I'll do it |
@birdofpreyru Updated. |
#339 rely on #337 be fixed, so I rise PR for them together