Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

feat: customize border radius of Views #42320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 17, 2024
Merged

feat: customize border radius of Views #42320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8ef03a3
feat: add View#setBorderRadius
samuelmaddock May 27, 2024
c21bd9d
maybe delay capture to fix tests?
samuelmaddock May 31, 2024
b816f1d
refactor: retry screen captures in an attempt to fix flakiness
samuelmaddock May 31, 2024
e7f2f42
refactor: ScreenCapture constructor no longer async
samuelmaddock Jun 3, 2024
81801f6
increase screen capture timeout, feels a little short
samuelmaddock Jun 3, 2024
ce97b90
refactor: move rounded rect util into chromium_src
samuelmaddock Jul 9, 2024
f31cb08
skip some capture tests on mas
samuelmaddock Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 2 chromium_src/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import("//third_party/widevine/cdm/widevine.gni")
static_library("chrome") {
visibility = [ "//electron:electron_lib" ]
sources = [
"//ash/style/rounded_rect_cutout_path_builder.cc",
"//ash/style/rounded_rect_cutout_path_builder.h",
"//chrome/browser/app_mode/app_mode_utils.cc",
"//chrome/browser/app_mode/app_mode_utils.h",
"//chrome/browser/browser_features.cc",
Expand Down
6 changes: 6 additions & 0 deletions 6 docs/api/view.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ Examples of valid `color` values:

**Note:** Hex format with alpha takes `AARRGGBB` or `ARGB`, _not_ `RRGGBBAA` or `RGB`.

#### `view.setBorderRadius(radius)`

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

**Note:** The area cutout of the view's border still captures clicks.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


#### `view.setVisible(visible)`

* `visible` boolean - If false, the view will be hidden from display.
Expand Down
35 changes: 35 additions & 0 deletions 35 shell/browser/api/electron_api_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>
#include <utility>

#include "ash/style/rounded_rect_cutout_path_builder.h"
#include "gin/data_object_builder.h"
#include "gin/wrappable.h"
#include "shell/browser/javascript_environment.h"
Expand Down Expand Up @@ -338,13 +339,46 @@ void View::SetBackgroundColor(std::optional<WrappedSkColor> color) {
view_->SetBackground(color ? views::CreateSolidBackground(*color) : nullptr);
}

void View::SetBorderRadius(int radius) {
border_radius_ = radius;
ApplyBorderRadius();
}

void View::ApplyBorderRadius() {
if (!border_radius_.has_value() || !view_)
return;

auto size = view_->bounds().size();

// Restrict border radius to the constraints set in the path builder class.
// If the constraints are exceeded, the builder will crash.
int radius;
{
float r = border_radius_.value() * 1.f;
r = std::min(r, size.width() / 2.f);
r = std::min(r, size.height() / 2.f);
r = std::max(r, 0.f);
radius = std::floor(r);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// RoundedRectCutoutPathBuilder has a minimum size of 32 x 32.
if (radius > 0 && size.width() >= 32 && size.height() >= 32) {
auto builder = ash::RoundedRectCutoutPathBuilder(gfx::SizeF(size));
builder.CornerRadius(radius);
view_->SetClipPath(builder.Build());
} else {
view_->SetClipPath(SkPath());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@samuelmaddock samuelmaddock May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough! I'll unblock this.

}
}

void View::SetVisible(bool visible) {
if (!view_)
return;
view_->SetVisible(visible);
}

void View::OnViewBoundsChanged(views::View* observed_view) {
ApplyBorderRadius();
Emit("bounds-changed");
}

Expand Down Expand Up @@ -393,6 +427,7 @@ void View::BuildPrototype(v8::Isolate* isolate,
.SetMethod("setBounds", &View::SetBounds)
.SetMethod("getBounds", &View::GetBounds)
.SetMethod("setBackgroundColor", &View::SetBackgroundColor)
.SetMethod("setBorderRadius", &View::SetBorderRadius)
.SetMethod("setLayout", &View::SetLayout)
.SetMethod("setVisible", &View::SetVisible);
}
Expand Down
4 changes: 4 additions & 0 deletions 4 shell/browser/api/electron_api_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ class View : public gin_helper::EventEmitter<View>,
void SetLayout(v8::Isolate* isolate, v8::Local<v8::Object> value);
std::vector<v8::Local<v8::Value>> GetChildren();
void SetBackgroundColor(std::optional<WrappedSkColor> color);
void SetBorderRadius(int radius);
void SetVisible(bool visible);

// views::ViewObserver
void OnViewBoundsChanged(views::View* observed_view) override;
void OnViewIsDeleting(views::View* observed_view) override;

views::View* view() const { return view_; }
std::optional<int> border_radius() const { return border_radius_; }

// disable copy
View(const View&) = delete;
Expand All @@ -58,9 +60,11 @@ class View : public gin_helper::EventEmitter<View>,
void set_delete_view(bool should) { delete_view_ = should; }

private:
void ApplyBorderRadius();
void ReorderChildView(gin::Handle<View> child, size_t index);

std::vector<v8::Global<v8::Object>> child_views_;
std::optional<int> border_radius_;

bool delete_view_ = true;
raw_ptr<views::View> view_ = nullptr;
Expand Down
23 changes: 23 additions & 0 deletions 23 shell/browser/api/electron_api_web_contents_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "shell/common/options_switches.h"
#include "third_party/skia/include/core/SkRegion.h"
#include "ui/base/hit_test.h"
#include "ui/gfx/geometry/rounded_corners_f.h"
#include "ui/views/controls/webview/webview.h"
#include "ui/views/layout/flex_layout_types.h"
#include "ui/views/view_class_properties.h"
#include "ui/views/widget/widget.h"
Expand Down Expand Up @@ -65,6 +67,25 @@ void WebContentsView::SetBackgroundColor(std::optional<WrappedSkColor> color) {
}
}

void WebContentsView::SetBorderRadius(int radius) {
View::SetBorderRadius(radius);
ApplyBorderRadius();
}

void WebContentsView::ApplyBorderRadius() {
if (border_radius().has_value() && api_web_contents_ && view()->GetWidget()) {
auto* web_view = api_web_contents_->inspectable_web_contents()
->GetView()
->contents_web_view();

// WebView won't exist for offscreen rendering.
if (web_view) {
web_view->holder()->SetCornerRadii(
gfx::RoundedCornersF(border_radius().value()));
}
}
}

int WebContentsView::NonClientHitTest(const gfx::Point& point) {
if (api_web_contents_) {
gfx::Point local_point(point);
Expand Down Expand Up @@ -93,6 +114,7 @@ void WebContentsView::OnViewAddedToWidget(views::View* observed_view) {
// because that's handled in the WebContents dtor called prior.
api_web_contents_->SetOwnerWindow(native_window);
native_window->AddDraggableRegionProvider(this);
ApplyBorderRadius();
}

void WebContentsView::OnViewRemovedFromWidget(views::View* observed_view) {
Expand Down Expand Up @@ -198,6 +220,7 @@ void WebContentsView::BuildPrototype(
prototype->SetClassName(gin::StringToV8(isolate, "WebContentsView"));
gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
.SetMethod("setBackgroundColor", &WebContentsView::SetBackgroundColor)
.SetMethod("setBorderRadius", &WebContentsView::SetBorderRadius)
.SetProperty("webContents", &WebContentsView::GetWebContents);
}

Expand Down
3 changes: 3 additions & 0 deletions 3 shell/browser/api/electron_api_web_contents_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class WebContentsView : public View,
// Public APIs.
gin::Handle<WebContents> GetWebContents(v8::Isolate* isolate);
void SetBackgroundColor(std::optional<WrappedSkColor> color);
void SetBorderRadius(int radius);

int NonClientHitTest(const gfx::Point& point) override;

Expand All @@ -57,6 +58,8 @@ class WebContentsView : public View,
private:
static gin_helper::WrappableBase* New(gin_helper::Arguments* args);

void ApplyBorderRadius();

// Keep a reference to v8 wrapper.
v8::Global<v8::Value> web_contents_;
raw_ptr<api::WebContents> api_web_contents_;
Expand Down
10 changes: 5 additions & 5 deletions 10 shell/browser/ui/inspectable_web_contents_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ InspectableWebContentsView::InspectableWebContentsView(
auto* contents_web_view = new views::WebView(nullptr);
contents_web_view->SetWebContents(
inspectable_web_contents_->GetWebContents());
contents_web_view_ = contents_web_view;
contents_view_ = contents_web_view_ = contents_web_view;
} else {
contents_web_view_ = new views::Label(u"No content under offscreen mode");
contents_view_ = new views::Label(u"No content under offscreen mode");
}

devtools_web_view_->SetVisible(false);
AddChildView(devtools_web_view_.get());
AddChildView(contents_web_view_.get());
AddChildView(contents_view_.get());
}

InspectableWebContentsView::~InspectableWebContentsView() {
Expand Down Expand Up @@ -209,7 +209,7 @@ const std::u16string InspectableWebContentsView::GetTitle() {

void InspectableWebContentsView::Layout(PassKey) {
if (!devtools_web_view_->GetVisible()) {
contents_web_view_->SetBoundsRect(GetContentsBounds());
contents_view_->SetBoundsRect(GetContentsBounds());
// Propagate layout call to all children, for example browser views.
LayoutSuperclass<View>(this);
return;
Expand All @@ -227,7 +227,7 @@ void InspectableWebContentsView::Layout(PassKey) {
new_contents_bounds.set_x(GetMirroredXForRect(new_contents_bounds));

devtools_web_view_->SetBoundsRect(new_devtools_bounds);
contents_web_view_->SetBoundsRect(new_contents_bounds);
contents_view_->SetBoundsRect(new_contents_bounds);

// Propagate layout call to all children, for example browser views.
LayoutSuperclass<View>(this);
Expand Down
5 changes: 4 additions & 1 deletion 5 shell/browser/ui/inspectable_web_contents_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class InspectableWebContentsView : public views::View {
return inspectable_web_contents_;
}

views::WebView* contents_web_view() const { return contents_web_view_; }

// The delegate manages its own life.
void SetDelegate(InspectableWebContentsViewDelegate* delegate) {
delegate_ = delegate;
Expand Down Expand Up @@ -67,8 +69,9 @@ class InspectableWebContentsView : public views::View {

std::unique_ptr<views::Widget> devtools_window_;
raw_ptr<views::WebView> devtools_window_web_view_ = nullptr;
raw_ptr<views::View> contents_web_view_ = nullptr;
raw_ptr<views::WebView> devtools_web_view_ = nullptr;
raw_ptr<views::WebView> contents_web_view_ = nullptr;
raw_ptr<views::View> contents_view_ = nullptr;

DevToolsContentsResizingStrategy strategy_;
bool devtools_visible_ = false;
Expand Down
12 changes: 5 additions & 7 deletions 12 spec/api-browser-view-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as path from 'node:path';
import { BrowserView, BrowserWindow, screen, webContents } from 'electron/main';
import { closeWindow } from './lib/window-helpers';
import { defer, ifit, startRemoteControlApp } from './lib/spec-helpers';
import { ScreenCapture } from './lib/screen-helpers';
import { ScreenCapture, hasCapturableScreen } from './lib/screen-helpers';
import { once } from 'node:events';

describe('BrowserView module', () => {
Expand Down Expand Up @@ -75,8 +75,7 @@ describe('BrowserView module', () => {
}).not.to.throw();
});

// Linux and arm64 platforms (WOA and macOS) do not return any capture sources
ifit(process.platform === 'darwin' && process.arch === 'x64')('sets the background color to transparent if none is set', async () => {
ifit(hasCapturableScreen())('sets the background color to transparent if none is set', async () => {
const display = screen.getPrimaryDisplay();
const WINDOW_BACKGROUND_COLOR = '#55ccbb';

Expand All @@ -90,12 +89,11 @@ describe('BrowserView module', () => {
w.setBrowserView(view);
await view.webContents.loadURL('data:text/html,hello there');

const screenCapture = await ScreenCapture.createForDisplay(display);
const screenCapture = new ScreenCapture(display);
await screenCapture.expectColorAtCenterMatches(WINDOW_BACKGROUND_COLOR);
});

// Linux and arm64 platforms (WOA and macOS) do not return any capture sources
ifit(process.platform === 'darwin' && process.arch === 'x64')('successfully applies the background color', async () => {
ifit(hasCapturableScreen())('successfully applies the background color', async () => {
const WINDOW_BACKGROUND_COLOR = '#55ccbb';
const VIEW_BACKGROUND_COLOR = '#ff00ff';
const display = screen.getPrimaryDisplay();
Expand All @@ -111,7 +109,7 @@ describe('BrowserView module', () => {
w.setBackgroundColor(VIEW_BACKGROUND_COLOR);
await view.webContents.loadURL('data:text/html,hello there');

const screenCapture = await ScreenCapture.createForDisplay(display);
const screenCapture = new ScreenCapture(display);
await screenCapture.expectColorAtCenterMatches(VIEW_BACKGROUND_COLOR);
});
});
Expand Down
29 changes: 10 additions & 19 deletions 29 spec/api-browser-window-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6490,8 +6490,8 @@ describe('BrowserWindow module', () => {
expect(w.getBounds()).to.deep.equal(newBounds);
});

// FIXME(codebytere): figure out why these are failing on macOS arm64.
ifit(process.platform === 'darwin' && process.arch !== 'arm64')('should not display a visible background', async () => {
// FIXME(codebytere): figure out why these are failing on MAS arm64.
ifit(hasCapturableScreen() && !(process.mas && process.arch === 'arm64'))('should not display a visible background', async () => {
const display = screen.getPrimaryDisplay();

const backgroundWindow = new BrowserWindow({
Expand All @@ -6514,9 +6514,7 @@ describe('BrowserWindow module', () => {
const colorFile = path.join(__dirname, 'fixtures', 'pages', 'half-background-color.html');
await foregroundWindow.loadFile(colorFile);

await setTimeout(1000);

const screenCapture = await ScreenCapture.createForDisplay(display);
const screenCapture = new ScreenCapture(display);
await screenCapture.expectColorAtPointOnDisplayMatches(
HexColors.GREEN,
(size) => ({
Expand All @@ -6533,8 +6531,8 @@ describe('BrowserWindow module', () => {
);
});

// FIXME(codebytere): figure out why these are failing on macOS arm64.
ifit(process.platform === 'darwin' && process.arch !== 'arm64')('Allows setting a transparent window via CSS', async () => {
// FIXME(codebytere): figure out why these are failing on MAS arm64.
ifit(hasCapturableScreen() && !(process.mas && process.arch === 'arm64'))('Allows setting a transparent window via CSS', async () => {
const display = screen.getPrimaryDisplay();

const backgroundWindow = new BrowserWindow({
Expand All @@ -6560,14 +6558,11 @@ describe('BrowserWindow module', () => {
foregroundWindow.loadFile(path.join(__dirname, 'fixtures', 'pages', 'css-transparent.html'));
await once(ipcMain, 'set-transparent');

await setTimeout(1000);

const screenCapture = await ScreenCapture.createForDisplay(display);
const screenCapture = new ScreenCapture(display);
await screenCapture.expectColorAtCenterMatches(HexColors.PURPLE);
});

// Linux and arm64 platforms (WOA and macOS) do not return any capture sources
ifit(process.platform === 'darwin' && process.arch === 'x64')('should not make background transparent if falsy', async () => {
ifit(hasCapturableScreen())('should not make background transparent if falsy', async () => {
const display = screen.getPrimaryDisplay();

for (const transparent of [false, undefined]) {
Expand All @@ -6579,8 +6574,7 @@ describe('BrowserWindow module', () => {
await once(window, 'show');
await window.webContents.loadURL('data:text/html,<head><meta name="color-scheme" content="dark"></head>');

await setTimeout(1000);
const screenCapture = await ScreenCapture.createForDisplay(display);
const screenCapture = new ScreenCapture(display);
// color-scheme is set to dark so background should not be white
await screenCapture.expectColorAtCenterDoesNotMatch(HexColors.WHITE);

Expand All @@ -6592,8 +6586,7 @@ describe('BrowserWindow module', () => {
describe('"backgroundColor" option', () => {
afterEach(closeAllWindows);

// Linux/WOA doesn't return any capture sources.
ifit(process.platform === 'darwin')('should display the set color', async () => {
ifit(hasCapturableScreen())('should display the set color', async () => {
const display = screen.getPrimaryDisplay();

const w = new BrowserWindow({
Expand All @@ -6605,9 +6598,7 @@ describe('BrowserWindow module', () => {
w.loadURL('about:blank');
await once(w, 'ready-to-show');

await setTimeout(1000);

const screenCapture = await ScreenCapture.createForDisplay(display);
const screenCapture = new ScreenCapture(display);
await screenCapture.expectColorAtCenterMatches(HexColors.BLUE);
});
});
Expand Down
11 changes: 11 additions & 0 deletions 11 spec/api-view-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,15 @@ describe('View', () => {
w.contentView.addChildView(v2);
expect(w.contentView.children).to.deep.equal([v3, v1, v2]);
});

it('allows setting various border radius values', () => {
w = new BaseWindow({ show: false });
const v = new View();
w.setContentView(v);
v.setBorderRadius(10);
v.setBorderRadius(0);
v.setBorderRadius(-10);
v.setBorderRadius(9999999);
v.setBorderRadius(-9999999);
});
});
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.