[Modern Media Controls] Add an alternative overflow icon
Created attachment 452231 [details] Patch
Created attachment 452233 [details] Patch
Comment on attachment 452233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452233&action=review r=me > Source/WebCore/Modules/modern-media-controls/controls/icon-service.js:38 > + Ellipsis : { name: "Ellipsis", type: "svg", label: UIString("More\u2026") }, please alphabetize this by moving it right above `EllipsisCircle` > Source/WebCore/Modules/modern-media-controls/controls/layout-traits.js:94 > + overflowButtonHasCircle() { Style: `{` goes on a separate line :) > Source/WebCore/Modules/modern-media-controls/controls/media-controls.js:46 > + if (layoutTraits.overflowButtonHasCircle()) I think you can actually get to the `layoutTraits` inside `OverflowButton` via something like `layoutDelegate?.layoutTraits`. So I'd leave this as-is and then change the `constructor` of `OverflowButton` to be like so: ``` super({ cssClassName: "overflow", iconName: layoutDelegate?.layoutTraits.overflowButtonHasCircle() ? Icons.EllipsisCircle : Icons.Ellipsis, layoutDelegate, }); ```
Comment on attachment 452233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452233&action=review >> Source/WebCore/Modules/modern-media-controls/controls/icon-service.js:38 >> + Ellipsis : { name: "Ellipsis", type: "svg", label: UIString("More\u2026") }, > > please alphabetize this by moving it right above `EllipsisCircle` Sure! >> Source/WebCore/Modules/modern-media-controls/controls/layout-traits.js:94 >> + overflowButtonHasCircle() { > > Style: `{` goes on a separate line :) Yes. >> Source/WebCore/Modules/modern-media-controls/controls/media-controls.js:46 >> + if (layoutTraits.overflowButtonHasCircle()) > > I think you can actually get to the `layoutTraits` inside `OverflowButton` via something like `layoutDelegate?.layoutTraits`. So I'd leave this as-is and then change the `constructor` of `OverflowButton` to be like so: > ``` > super({ > cssClassName: "overflow", > iconName: layoutDelegate?.layoutTraits.overflowButtonHasCircle() ? Icons.EllipsisCircle : Icons.Ellipsis, > layoutDelegate, > }); > ``` I like this idea! Thanks!
Created attachment 452288 [details] Patch for landing
I cancelled https://ews-build.webkit.org/#/builders/68/builds/8368 to speed up ios-wk2 queue (as it is backlogged). That build indicated 30+ layout-test failures on Patch 452233. Please check.
Created attachment 452392 [details] Fix test failures
Created attachment 452393 [details] Fix an issue on watchOS
Created attachment 452448 [details] [fast-cq] Revise the patch based on Devin's suggestions
Committed r290133 (247477@main): <https://commits.webkit.org/247477@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452448 [details].
<rdar://problem/89150124>