feat: customize border radius of Views #42320

samuelmaddock · 2024-05-30T22:01:20Z

Description of Change

resolves #42288
resolves #33426

Adds View.setBorderRadius(radius) which applies a rounded corner mask to the view. Note that there are two separate implementations depending on whether it's purely a Chromium view (View/ImageView/etc.) or a native view (WebView).

Windows	Mac

Border radius demo: https://gist.github.com/samuelmaddock/4905763d665905c9134447ab9a9ac283
Animated demo: https://gist.github.com/samuelmaddock/99f0c66a584d4134139d3c9608df6652

I unfortunately don't have a Linux machine setup to test this. However, that platform relies on //ui/aura, same as Windows, so I figure it'll Just Work™️

Also, maybe worth calling out that this depends on #41326 for a consistent implementation across platforms.

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant documentation, tutorials, templates and examples are changed or added
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Added View.setBorderRadius(radius) for customizing the border radius of views—with compatibility for WebContentsView.

erickzhao

API LGTM

samuelmaddock · 2024-05-30T22:09:14Z

shell/browser/api/electron_api_view.cc

+    r = std::min(r, size.height() / 2.f);
+    r = std::max(r, 0.f);
+    radius = std::floor(r);
+  }


I'd like to upstream a CL to make ash::RoundedRectCutoutPathBuilder easier to work with, but for now I think this is simple enough to workaround here.

samuelmaddock · 2024-05-30T22:14:05Z

docs/api/view.md

+
+* `radius` Integer - Border radius size in pixels.
+
+**Note:** The area cutout of the view's border still captures clicks.


I don't imagine this would be too difficult to fix, but I'd rather do this in a future PR if a need arises.

nornagon · 2024-05-30T23:59:33Z

shell/browser/api/electron_api_view.cc

+    builder.CornerRadius(radius);
+    view_->SetClipPath(builder.Build());
+  } else {
+    view_->SetClipPath(SkPath());


Any chance we could expose SetClipPath more directly? I'd generally like to err on the side of power over ease of use in the Views API.

The difficulty here is that the native views use a different code path for faster clipping
https://source.chromium.org/chromium/chromium/src/+/main:ui/views/controls/native/native_view_host_aura.cc;l=349;drc=8b6aaad8027390ce6b32d82d57328e93f34bb8e5

Arguably the WebContentsView border radius is the dominant use case. The clipping for the underlying View allows us to also clip the background color.

edit: to elaborate, this fast path goes down to the renderer level which has special masking for rounded corners. See skia_renderer.cc and search for "rounded"

Hm, afaict this code here doesn't use the fast path, right?

WebContentsView is using SetCornerRadii which uses the fast path. This is required to apply a mask to the underlying views::NativeViewHost.

This results in two implementations of setBorderRadius

View: uses SetClipPath to mask rendering

WebContentsView: uses SetCornerRadii to apply the fast path render clipping to the WebView

Also calls into the View's mask rendering so its background color gets a mask applied as well.

If we feel strongly about exposing a more powerful SetClipPath API, we could always introduce one in the future and reimplement View.prototype.setBorderRadius using it in an internal JS API.

Are we comfortable unblocking this? @nornagon @electron/wg-api

Fair enough! I'll unblock this.

codebytere

API LGTM

nornagon

Throwing a request changes on here as I feel there are unresolved questions about this API

nornagon

API LGTM

shell/browser/api/electron_api_view.cc

test: initial setBorderRadius tests fix: robustly set border radius chore: add PAUSE_CAPTURE_TESTS for easier screencap dev feat: add view border radius support test: view border radius refactor: cleanup view code

jkleinsc

The changes made to tests here break macOS arm64. It also appears for some reason the changes here are causing linux tests to fail with a disk out of space error.

spec/api-browser-window-spec.ts

release-clerk · 2024-07-17T00:16:28Z

Release Notes Persisted

Added View.setBorderRadius(radius) for customizing the border radius of views—with compatibility for WebContentsView.

* fix: remove InspectableWebContentsViewMac (#43033) * Revert "refactor: remove InspectableWebContentsViewMac in favor of the Views version (#41326)" This reverts commit e67ab9a. * build: fix gn check * chore: implement setCornerRadii in inspectable_web_contents_view_mac Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: pass in cornerRadii value in setCornerRadii Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: forward declaration * 5578714: Remove 0-arg (default) constructor for views::Widget::InitParams. https://chromium-review.googlesource.com/c/chromium/src/+/5578714 * fix: contents_web_view_ -> contents_view_ * chore: remove extraneous includes --------- Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: remove custom border radius feat (#42320) * fixup! remove custom border radius feat --------- Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* fix: remove InspectableWebContentsViewMac (#43033) * Revert "refactor: remove InspectableWebContentsViewMac in favor of the Views version (#41326)" This reverts commit e67ab9a. * build: fix gn check * chore: implement setCornerRadii in inspectable_web_contents_view_mac Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: pass in cornerRadii value in setCornerRadii Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: forward declaration * 5578714: Remove 0-arg (default) constructor for views::Widget::InitParams. https://chromium-review.googlesource.com/c/chromium/src/+/5578714 * fix: contents_web_view_ -> contents_view_ * chore: remove extraneous includes --------- Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * fix: remove custom border radius feat (#42320) * fix: fix views::Widget::InitParams params --------- Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

liuyike98 · 2024-10-23T16:22:54Z

@samuelmaddock any solution to set left-bottom and right-bottom border radius? webContentsView just as a part of my custom view.

samuelmaddock · 2024-10-23T16:36:47Z

@samuelmaddock any solution to set left-bottom and right-bottom border radius? webContentsView just as a part of my custom view.

It's not currently possible. However, based on the implementation of gfx::RoundedCornerF, it should be possible if someone is interested in submitting a pull request.

electron-cation bot added the new-pr 🌱 PR opened recently label May 30, 2024

samuelmaddock added the semver/minor backwards-compatible functionality label May 30, 2024

electron-cation bot added the api-review/requested 🗳 label May 30, 2024

samuelmaddock added the no-backport label May 30, 2024

erickzhao approved these changes May 30, 2024

View reviewed changes

samuelmaddock commented May 30, 2024

View reviewed changes

nornagon reviewed May 30, 2024

View reviewed changes

codebytere reviewed Jun 3, 2024

View reviewed changes

samuelmaddock requested a review from nornagon June 4, 2024 12:48

electron-cation bot added api-review/approved ✅ and removed new-pr 🌱 PR opened recently api-review/requested 🗳 labels Jun 6, 2024

nornagon suggested changes Jun 6, 2024

View reviewed changes

electron-cation bot added api-review/requested 🗳 and removed api-review/approved ✅ labels Jun 6, 2024

samuelmaddock requested a review from nornagon June 6, 2024 23:35

nornagon approved these changes Jul 1, 2024

View reviewed changes

electron-cation bot added api-review/approved ✅ and removed api-review/requested 🗳 labels Jul 1, 2024

nornagon reviewed Jul 1, 2024

View reviewed changes

shell/browser/api/electron_api_view.cc Outdated Show resolved Hide resolved

samuelmaddock force-pushed the feat/view-border-radius branch 2 times, most recently from 2cc50d4 to c31374f Compare July 11, 2024 20:19

samuelmaddock and others added 6 commits July 15, 2024 13:16

feat: add View#setBorderRadius

8ef03a3

test: initial setBorderRadius tests fix: robustly set border radius chore: add PAUSE_CAPTURE_TESTS for easier screencap dev feat: add view border radius support test: view border radius refactor: cleanup view code

maybe delay capture to fix tests?

c21bd9d

refactor: retry screen captures in an attempt to fix flakiness

b816f1d

refactor: ScreenCapture constructor no longer async

e7f2f42

increase screen capture timeout, feels a little short

81801f6

refactor: move rounded rect util into chromium_src

ce97b90

samuelmaddock force-pushed the feat/view-border-radius branch from c31374f to ce97b90 Compare July 15, 2024 17:16

jkleinsc requested changes Jul 16, 2024

View reviewed changes

spec/api-browser-window-spec.ts Outdated Show resolved Hide resolved

spec/api-browser-window-spec.ts Outdated Show resolved Hide resolved

electron-cation bot added api-review/requested 🗳 and removed api-review/approved ✅ labels Jul 16, 2024

skip some capture tests on mas

f31cb08

samuelmaddock mentioned this pull request Jul 16, 2024

ci: upload test artifacts #42925

Merged

5 tasks

jkleinsc approved these changes Jul 17, 2024

View reviewed changes

jkleinsc merged commit 778d309 into electron:main Jul 17, 2024

samuelmaddock deleted the feat/view-border-radius branch July 17, 2024 00:17

VerteDinde mentioned this pull request Jul 25, 2024

fix: remove InspectableWebContentsViewMac #43033

Merged

5 tasks

VerteDinde added a commit that referenced this pull request Jul 29, 2024

fix: remove custom border radius feat (#42320)

848ad1c

VerteDinde added a commit that referenced this pull request Jul 30, 2024

fix: remove custom border radius feat (#42320)

0dcaa8b


		* `radius` Integer - Border radius size in pixels.

		Note: The area cutout of the view's border still captures clicks.

feat: customize border radius of Views #42320

feat: customize border radius of Views #42320

Uh oh!

Conversation

samuelmaddock commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Release Notes

Uh oh!

erickzhao left a comment

Choose a reason for hiding this comment

Uh oh!

samuelmaddock May 30, 2024

Choose a reason for hiding this comment

Uh oh!

samuelmaddock May 30, 2024

Choose a reason for hiding this comment

Uh oh!

nornagon May 30, 2024

Choose a reason for hiding this comment

Uh oh!

samuelmaddock May 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nornagon Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

samuelmaddock Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

samuelmaddock Jun 20, 2024

Choose a reason for hiding this comment

Uh oh!

nornagon Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

codebytere left a comment

Choose a reason for hiding this comment

Uh oh!

nornagon left a comment

Choose a reason for hiding this comment

Uh oh!

nornagon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkleinsc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

release-clerk bot commented Jul 17, 2024

Uh oh!

liuyike98 commented Oct 23, 2024

Uh oh!

samuelmaddock commented Oct 23, 2024

Uh oh!

Uh oh!

samuelmaddock commented May 30, 2024 •

edited

Loading

samuelmaddock May 31, 2024 •

edited

Loading