-
Notifications
You must be signed in to change notification settings - Fork 88
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
Improve click handling on touch devices #683
Improve click handling on touch devices #683
Conversation
* use active otherwise
@@ -10,7 +10,13 @@ | |||
cursor: pointer; | |||
} | |||
|
|||
&:hover { | |||
@media (hover: hover) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only adds the hover style on devices where hover is supported.
…ling-on-touch-devices
@@ -102,7 +102,8 @@ export class DynamicSettingsPanelItem extends SettingsPanelItem<DynamicSettingsP | |||
const handleItemClick = () => { | |||
this.displayItemsSubPage(); | |||
}; | |||
this.getDomElement().on('click', (e) => { | |||
this.getDomElement().on('click touchend', (e) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dweinber found an issue with this approach. The touchend triggers and opens the sub menu when releasing the touch after scrolling within the settings panel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixed again by removing the touchend
listeners again in: d596dbf
Note: The touchend
listeners were a solution for an issue that the click event did not trigger every time on non button components. However, this was only observed in combination with the iOS Player SDK. This issue is already known and the rootcause is within the Player SDK where we call the native code from the Javascript context. This is not performant enough and blocks the Javascript execution. We will tackle this in the iOS Player SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are missing a changelog, otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK since it's "only" going into the V4 branch, not to main
Description
There is an issue on touch input devices where some settings panel elements need to be touched twice in order to change them. This is due to the
:hover
effect which we currently use.In addition sometimes the click event is not triggered on non button elements
on touch devicesin combination with our iOS Player SDK.To improve this situation I added theThis will be addressed in the iOS Player SDK itself.touchend
to those elements. To keep the functionality for buttons which are inside such elemnts, we also need to add thetouchend
to the button component.