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

Add config to allow default action for first touch #624

Closed
wants to merge 6 commits into from

Conversation

hawk23
Copy link

@hawk23 hawk23 commented May 16, 2024

ticket: https://bitmovin.atlassian.net/browse/PI-2655

Description

Currently, when the small-screen ads UI is used on a touch device, two touches are needed to either hit the "skip ad" button or to open the ad click-through URL. Especially for the "skip ad" button this is a confusing user experience as it is always visible and one touch should be sufficient to tap it.

To fix this, a new configuration option allowDefaultActionOnFirstTouch is introduced on UIContainerConfig that can be used to control whether the default action should be executed after showing the hidden UI on a first touch. The allowDefaultActionOnFirstTouch flag is now set to true for modernSmallScreenAdsUI, allowing first touches to trigger the click-through URL and also the "skip ad" button.

Discussion

A drawback of this approach is that also buttons from the title bar can receive this first touch now during ads. Specifically, the fullscreen button, which is the only one that is visible there during ad playback.

There would be some ways to improve on this situation:

  • (1) Attach a payload to the onClick event that determines if it is a first touch. Event receiving components could check for this flag and decide on their own if they want to handle onClick in case they are first touches. The "skip ad" button and the ad click overlay would be then implemented in a way that they accept onClicks that are first touches and the fullscreen button would ignore such onClick events.
  • (2) Decide in UIContainer.touchend event handler if the default action should be prevented or not based on the event target.

(1) seems a bit more elegant than (2) as the components are allowed to decide for themselves if they want to react to first touches.

Tests

  • Please do manual testing

Checklist (for PR submitter and reviewers)

  • CHANGELOG entry

@hawk23 hawk23 self-assigned this May 16, 2024
// instead. The first touch is not prevented to let other listeners receive the event and trigger an
// initial action, e.g. the huge playback button can directly start playback instead of requiring a double
// tap which 1. reveals the UI and 2. starts playback.
if (isFirstTouch && !player.isPlaying()) {
Copy link
Author

Choose a reason for hiding this comment

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

isFirstTouch flag was removed as it seems redundant. As we are inside an if that checks for !isUiShown and because we always call showUi() from inside this if, we will always just enter this code path on first touch after UI was hidden.

@stonko1994
Copy link
Collaborator

Closing in favour of #626 & #627

@stonko1994 stonko1994 closed this May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants