Bug 230692 - [Media Controls] Allow for a single mute and volume button
Summary: [Media Controls] Allow for a single mute and volume button
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-23 08:55 PDT by Antoine Quint
Modified: 2021-09-23 12:08 PDT (History)
11 users (show)

See Also:


Attachments
Patch (30.22 KB, patch)
2021-09-23 09:01 PDT, Antoine Quint
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2021-09-23 08:55:24 PDT
[Media Controls] Allow for a single mute and volume button
Comment 1 Antoine Quint 2021-09-23 09:01:47 PDT
Created attachment 439049 [details]
Patch
Comment 2 Antoine Quint 2021-09-23 09:01:53 PDT
<rdar://problem/79956248>
Comment 3 Antoine Quint 2021-09-23 10:44:58 PDT
Committed r282971 (242061@main): <https://commits.webkit.org/242061@main>
Comment 4 Devin Rousso 2021-09-23 12:08:18 PDT
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 🤔