ampproject / ampproject/amphtml

amp-ima-video: `hideControls()` does not fully hide controls

Open
#20,787 4 comments 4 reactions 1 assignee Claimed by @alanorozco View on GitHub
Component: amp-ima-video P3: When Possible Stale Type: Feature Request WG: monetization
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

#20760 introduces support for ``. In docked mode, custom mini controls are displayed. Even though `docking` requested `ima-video` to `hideControls`, the latter will still display them when the video is paused, effectively showing two sets of controls at a time.

This can be approached in one of two ways:

1. **Honor the result of dynamic `hideControls` to hide them regardless of playing state.** I don't believe this would break any use-cases, since `hideControls` calls are dynamic and should in theory be superseded by `showControls` when the calling module requires it. This may require slight `amp-ima-video` refactoring (maybe) and additional internal `postMessage` APIs (doubtful).

2. **Support `force` in `hideControls`**. This has the unfortunate side-effect of having to backfill all players to support this parameter. This would still require additional changes to `amp-ima-video`, including necessary `postMessage` API changes to pass the flag through. However, flags like `force` should generally be avoided, so I'm not in favor of this API unless, for some reason, `amp-ima-video` UX requires decoupling of hiding use-cases (unlikely).

/to @torch2424
/cc @aghassemi

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.