Skip to content

Commit b2111e5

Browse files
ergunshDevtools-frontend LUCI CQ
authored andcommitted
[GdpIntegration] Fix Badge Notification appearing behind the Settings screen
Previously, to make sure that badge notification is rendered in DevTools bounds, we were rendering it under inspector view. However, Settings screen is rendered as a dialog under `document.body`. Because of this reason, badge notification had no chance of being rendered on top of the settings screen (which we need). This CL: * Renders `BadgeNotification` under `document.body` (similar to GlassPane). * Positions it based on the height & the inspector view bounds. Fixed: 444428219 Change-Id: Ie9e6de6066c9cd72aaab23c9d0f9589fcf273fbe Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6939233 Auto-Submit: Ergün Erdoğmuş <ergunsh@chromium.org> Reviewed-by: Wolfgang Beyer <wolfi@chromium.org> Commit-Queue: Ergün Erdoğmuş <ergunsh@chromium.org>
1 parent 15a0b96 commit b2111e5

File tree

4 files changed

+32
-5
lines changed

4 files changed

+32
-5
lines changed

front_end/panels/common/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ devtools_module("common") {
3737
"../../core/platform:bundle",
3838
"../../models/badges:bundle",
3939
"../../models/geometry:bundle",
40+
"../../services/window_bounds:bundle",
4041
"../../ui/components/buttons:bundle",
4142
"../../ui/components/snackbars:bundle",
4243
"../../ui/components/spinners:bundle",

front_end/panels/common/BadgeNotification.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ describeWithEnvironment('BadgeNotification', () => {
5959
widget.message = properties?.message ?? 'Test message';
6060
widget.imageUri = properties?.imageUri ?? 'test.png';
6161
widget.actions = properties?.actions ?? [];
62-
widget.markAsRoot();
6362
renderElementIntoDOM(widget, {allowMultipleChildren: true});
6463
widget.requestUpdate();
6564
await view.nextInput;
@@ -78,10 +77,12 @@ describeWithEnvironment('BadgeNotification', () => {
7877

7978
it('invokes action callback on click', async () => {
8079
const action1Spy = sinon.spy();
81-
const {view} = await createWidget({actions: [{label: 'Action 1', onClick: action1Spy}]});
80+
const {view, widget} = await createWidget({actions: [{label: 'Action 1', onClick: action1Spy}]});
8281

8382
view.input.actions[0].onClick();
8483
sinon.assert.calledOnce(action1Spy);
84+
85+
widget.detach();
8586
});
8687

8788
it('is removed on close click', async () => {
@@ -91,6 +92,8 @@ describeWithEnvironment('BadgeNotification', () => {
9192

9293
view.input.onCloseClick();
9394
assert.isFalse(inspectorViewRootElementStub.contains(widget.element));
95+
96+
widget.detach();
9497
});
9598

9699
it('presents an activity-based badge', async () => {
@@ -105,6 +108,8 @@ describeWithEnvironment('BadgeNotification', () => {
105108
assert.strictEqual(input.actions[0].label, 'Badge settings');
106109
assert.strictEqual(input.actions[1].label, 'View profile');
107110
assertMessageIncludes(input.message, 'It has been added to your Developer Profile.');
111+
112+
widget.detach();
108113
});
109114

110115
it('presents a starter badge as an activity-based badge if the user has a profile and has enabled badges',
@@ -124,6 +129,8 @@ describeWithEnvironment('BadgeNotification', () => {
124129
assert.strictEqual(input.actions[0].label, 'Badge settings');
125130
assert.strictEqual(input.actions[1].label, 'View profile');
126131
assertMessageIncludes(input.message, 'It has been added to your Developer Profile.');
132+
133+
widget.detach();
127134
});
128135

129136
it('presents a starter badge with an opt-in message if the user has a profile but has disabled badges', async () => {
@@ -141,6 +148,8 @@ describeWithEnvironment('BadgeNotification', () => {
141148
assert.strictEqual(input.actions[0].label, 'Remind me later');
142149
assert.strictEqual(input.actions[1].label, 'Receive badges');
143150
assertMessageIncludes(input.message, 'Turn on badges to claim it.');
151+
152+
widget.detach();
144153
});
145154

146155
it('presents a starter badge with a create profile message if the user does not have a profile', async () => {
@@ -157,5 +166,7 @@ describeWithEnvironment('BadgeNotification', () => {
157166
assert.strictEqual(input.actions[0].label, 'Remind me later');
158167
assert.strictEqual(input.actions[1].label, 'Create profile');
159168
assertMessageIncludes(input.message, 'Create a profile to claim your badge.');
169+
170+
widget.detach();
160171
});
161172
});

front_end/panels/common/BadgeNotification.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as Common from '../../core/common/common.js';
66
import * as Host from '../../core/host/host.js';
77
import * as i18n from '../../core/i18n/i18n.js';
88
import * as Badges from '../../models/badges/badges.js';
9+
import * as WindowBoundsService from '../../services/window_bounds/window_bounds.js';
910
import * as Buttons from '../../ui/components/buttons/buttons.js';
1011
import * as UI from '../../ui/legacy/legacy.js';
1112
import * as Lit from '../../ui/lit/lit.js';
@@ -66,6 +67,8 @@ const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
6667
const i18nFormatString = i18n.i18n.getFormatLocalizedString.bind(undefined, str_);
6768
const lockedString = i18n.i18n.lockedString;
6869

70+
const LEFT_OFFSET = 5;
71+
const BOTTOM_OFFSET = 5;
6972
export interface BadgeNotificationAction {
7073
label: string;
7174
jslogContext?: string;
@@ -141,6 +144,8 @@ export class BadgeNotification extends UI.Widget.Widget {
141144
constructor(element?: HTMLElement, view: View = DEFAULT_VIEW) {
142145
super(element);
143146
this.#view = view;
147+
148+
this.markAsRoot();
144149
}
145150

146151
async present(badge: Badges.Badge): Promise<void> {
@@ -151,12 +156,24 @@ export class BadgeNotification extends UI.Widget.Widget {
151156
}
152157
}
153158

159+
#positionNotification(): void {
160+
const boundingRect = this.contentElement.getBoundingClientRect();
161+
const container =
162+
WindowBoundsService.WindowBoundsService.WindowBoundsServiceImpl.instance().getDevToolsBoundingElement();
163+
this.contentElement.positionAt(
164+
LEFT_OFFSET, container.clientHeight - boundingRect.height - BOTTOM_OFFSET, container);
165+
}
166+
154167
#show(properties: BadgeNotificationProperties): void {
155168
this.message = properties.message;
156169
this.imageUri = properties.imageUri;
157170
this.actions = properties.actions;
158171
this.requestUpdate();
159-
this.show(UI.InspectorView.InspectorView.instance().element);
172+
this.show(document.body);
173+
174+
void this.updateComplete.then(() => {
175+
this.#positionNotification();
176+
});
160177
}
161178

162179
async #presentStarterBadge(badge: Badges.Badge): Promise<void> {

front_end/panels/common/badgeNotification.css

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
@scope to (devtools-widget > *) {
88
:scope {
99
position: fixed;
10-
bottom: var(--sys-size-5);
11-
left: var(--sys-size-5);
1210
z-index: 9999;
1311
/* subtract var(--sys-size-5) * 2 so that there is equal space on the left and on the right in small screens */
1412
max-width: calc(100% - 2 * var(--sys-size-5));

0 commit comments

Comments
 (0)