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 double touch needed in Ads UI #627

Merged
merged 5 commits into from
May 28, 2024

Conversation

stonko1994
Copy link
Collaborator

@stonko1994 stonko1994 commented May 23, 2024

Description

Problem

When BAM (Bitmovin Advertising Module) is used on touch devices, two touch interactions are necessary to skip an ad or open the ad click-through URL.

On touch input devices, the first touch is expected to display the UI controls and not be propagated to other components. When buttons are always visible, such as the AdSkipButton, the first touch seems as if it's not 'recognized'.

Possible approaches which we elaborated

Always show the Ads UI

#622

Was not considered as the solution as other elements which should not be there are also always visible.

Global setting to allow default touch on first touch

#624

Was not considered as the solution as non visible elements, e.g. the fullscreen button, would receive the click and perform their actions.

Using display: none

Most likely the cleanest solution would be to switch from opacity: 0; to display: none which would prevent any click listners completely. However, display: none is not animatable. Technically there are still ways to achieve a display: none approach but we did not want to do such a 'core' change for this.

Respecting the visibility of elements

In a POC I implemented an approach to traverse the view hierarchy and detect the visibility of the target element of the touch event. However, this yield other problems. Mainly with the PlaybackToggleOverlay which is strictly speaking not visible but technically still there in the DOM tree (there are other elements too). Those would need changes in their visibility handling which was considered out of scope for this fix.

Changes

A new attribute was added to the ButtonConfig (acceptsTouchWithUiHidden) which indicates if a button wants to receive the first touch input even if the UI is currently not visible. This follows the idea from @hawk23 as outlined in #624 (section 1)) but with an inverted dependency.
The idea is that every Button component can decide if it wants to receive the click (touchend) even if the UI is hidden.
Ths setting is then respected in the UIContainer on the first touch input if the touchend event should be stop bubbling or not.

The AdSkipButton and the AdClickOverlay set this new property to true

Manual Testing

  • Include the BAM module and configure the Player to use BAM.
+ <script src="https://cdn.bitmovin.com/player/web/8/modules/bitmovinplayer-advertising-bitmovin.js"></script>

// Before instantiating a Player instance register the BAM module
+ bitmovin.player.Player.addModule(window.bitmovin.player['advertising-bitmovin'].default);
  • Enable ads:
- var adsEnabled = false;
+ var adsEnabled = true;
  • Run the playground on a touch device (or a browser with touch input simulated)
  • Make sure you are testing the SmallScreenUI
  • Try to skip the Ad before and after the changes

Checklist (for PR submitter and reviewers)

  • CHANGELOG entry

@stonko1994 stonko1994 marked this pull request as ready for review May 23, 2024 13:21
@stonko1994 stonko1994 requested a review from Andr3wid May 23, 2024 13:22
Base automatically changed from feature/assign-components-to-html-elements to develop May 28, 2024 06:05
@stonko1994 stonko1994 merged commit 0395090 into develop May 28, 2024
2 checks passed
@stonko1994 stonko1994 deleted the feature/improve-ad-ux-on-touch-devices branch May 28, 2024 06:05
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