Summary: | [Media Controls] Allow for a single mute and volume button | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||
Component: | Media | Assignee: | Antoine Quint <graouts> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dino, eric.carlson, ews-watchlist, glenn, hi, jer.noble, joepeck, philipj, sam, sergio, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Antoine Quint
2021-09-23 08:55:24 PDT
Created attachment 439049 [details]
Patch
Committed r282971 (242061@main): <https://commits.webkit.org/242061@main> Comment on attachment 439049 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439049&action=review > Source/WebCore/Modules/modern-media-controls/controls/range-button.js:75 > + if (event.currentTarget === this.element && event.type == "pointerdown") oops `==` > Source/WebCore/Modules/modern-media-controls/controls/range-button.js:101 > + if (this.uiDelegate && typeof this.uiDelegate.controlValueWillStartChanging === "function") > + this.uiDelegate.controlValueWillStartChanging(this); NIT: you can simplify this as ``` this.uiDelegate?.controlValueWillStartChanging?.(this); ``` > Source/WebCore/Modules/modern-media-controls/controls/range-button.js:134 > + if (this.uiDelegate && typeof this.uiDelegate.controlValueDidStopChanging === "function") > + this.uiDelegate.controlValueDidStopChanging(this); ditto (:100) ``` this.uiDelegate?.controlValueDidStopChanging?.(this); ``` > Source/WebCore/Modules/modern-media-controls/controls/range-button.js:138 > + if (this._enabled && this.uiDelegate && typeof this.uiDelegate.buttonWasPressed === "function") > + this.uiDelegate.buttonWasPressed(this); ditto (:100) ``` if (this._enabled) this.uiDelegate?.buttonWasPressed?.(this); ``` > Source/WebCore/Modules/modern-media-controls/controls/volume-button.js:26 > +class VolumeButton extends RangeButton I wonder if instead of basically duplicating the entirety of `MuteButton` we could've had `MuteButton` extend from `RangeButton` and just have a `set allowsSlidingToAdjustVolume` to switch behavior at runtime or something 🤔 |