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

fix(dropdown): rework touch input handling #2331

Merged
merged 4 commits into from
May 1, 2022

Conversation

mhthies
Copy link
Contributor

@mhthies mhthies commented Apr 19, 2022

Description

This PR fixes #1989 in a similar way as my previous PR for the slider (#2327): First, I removed all special event handling for touch events and the whole distiction between touch- and non-touch devices, so mouse events will be handled on touch devices as well. Then, I added only a few touch event bindings for the things that don't work via mouse event emulation of the mobile web browsers.

In addition, I fixed two minor bugs with nested submenus being not correctly hidden in some cases (see commit messages for more details).

I tested everything on

  • Chrome mobile on Android (with touch and Bluetooth mouse)
  • Firefox mobile on Android (with touch and Bluetooth mouse)
  • Chromium on Linux/X11 (with multitouch screen and mouse)
  • Firefox on Linux/X11 (with multitouch screen and mouse)

Testcase

Screenshot (if possible)

(looks the same as before)

Closes

mhthies added 3 commits April 19, 2022 17:15
Instead of distinguishing between touch and non-touch devices and
binding to different events, we always bind to mouse events. For touch
input, we mostly rely on the mouse event emulation (esp. click and mouseenter
events) of modern browsers.

This fixes fomantic#1989
Before this fix, nested sub-menus/dropdowns with more than two
menu layers would not hide correctly when moving the mouse cursor from
one menu directly to a "grand-parent" or higher menu (e.g. from the most inner
to the most outer menu in a three layer menu).
…ction

Some browsers seem to trigger mouse leave events only when moving the
mouse cursor. I noticed that on mobile browsers, especially when using
the touch screen. This somehow causes the submenus to stay visible even when
the root menu is hidden (e.g. due to clicking/selecting a menu item).

Thus, this commit forces hiding of all nested submenus, when hiding a menu.
@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews device/mobile Any issues relating to mobile devices labels Apr 19, 2022
@lubber-de lubber-de modified the milestones: 2.9.x, 2.9.0 Apr 19, 2022
Copy link
Member

@lubber-de lubber-de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very good work. Thanks! and i learned a lot 😄

@lubber-de lubber-de changed the title fix(dropdown): Rework touch input handling fix(dropdown): rework touch input handling May 1, 2022
@lubber-de lubber-de merged commit d72943e into fomantic:develop May 1, 2022
@lubber-de lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label May 1, 2022
lubber-de added a commit that referenced this pull request Jun 11, 2023
When a dropdown hides, the transition gets called twice messing up with upward menus and underlaying menu transparency.
This is because submenu hiding needs to be called separately (invented by #2331 ) , but in case a select is used the submenu is the dropdown itself, so it should not be (hide) animated again when it is already (hide) animating
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
device/mobile Any issues relating to mobile devices lang/javascript Anything involving JavaScript type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dropdown] cannot use mouse on iPad
2 participants