Skip to content

Commit 710acd1

Browse files
paulirishDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Status dialog: Hide stop button and make responsive
The stop button is only relevant to finish the recording. It cannot stop loading or processing the trace (without significant refactors). The previous arrangeDialog() solution to narrow widths could trigger dramatic layout thrashing under certain (inconsistent repro) circumstances. It's solved with container queries and a simpler layout. Demo: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6164069 Bug: 41470724 Change-Id: I681491c1959d4ace5c7d1c70238c64cad5c1e884 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6164069 Auto-Submit: Paul Irish <[email protected]> Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Connor Clark <[email protected]>
1 parent 71be07d commit 710acd1

File tree

4 files changed

+36
-41
lines changed

4 files changed

+36
-41
lines changed

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,9 @@ const UIStrings = {
214214
*/
215215
close: 'Close',
216216
/**
217-
*@description Text to download the raw trace files after an error
217+
*@description Text to download the trace file after an error
218218
*/
219-
downloadAfterError: 'Download raw trace events',
219+
downloadAfterError: 'Download trace',
220220
/**
221221
*@description Status text to indicate the recording has failed in the Performance panel
222222
*/
@@ -1763,7 +1763,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
17631763
{
17641764
description: error,
17651765
buttonText: i18nString(UIStrings.close),
1766-
buttonDisabled: false,
1766+
hideStopButton: true,
17671767
showProgress: undefined,
17681768
showTimer: undefined,
17691769
},
@@ -2264,7 +2264,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
22642264
{
22652265
showProgress: true,
22662266
showTimer: undefined,
2267-
buttonDisabled: undefined,
2267+
hideStopButton: true,
22682268
buttonText: undefined,
22692269
description: undefined,
22702270
},
@@ -2444,7 +2444,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
24442444
{
24452445
showTimer: true,
24462446
showProgress: true,
2447-
buttonDisabled: true,
2447+
hideStopButton: false,
24482448
description: undefined,
24492449
buttonText: undefined,
24502450
},
@@ -2669,11 +2669,11 @@ export class StatusPane extends UI.Widget.VBox {
26692669

26702670
constructor(
26712671
options: {
2672+
hideStopButton: boolean,
26722673
showTimer?: boolean,
26732674
showProgress?: boolean,
26742675
description?: string,
26752676
buttonText?: string,
2676-
buttonDisabled?: boolean,
26772677
},
26782678
buttonCallback: () => (Promise<void>| void)) {
26792679
super(true);
@@ -2712,22 +2712,22 @@ export class StatusPane extends UI.Widget.VBox {
27122712
}, {jslogContext: 'timeline.download-after-error'});
27132713

27142714
this.downloadTraceButton.disabled = true;
2715-
this.downloadTraceButton.style.visibility = 'hidden';
2715+
this.downloadTraceButton.classList.add('hidden');
27162716

27172717
const buttonText = options.buttonText || i18nString(UIStrings.stop);
27182718
this.button = UI.UIUtils.createTextButton(buttonText, buttonCallback, {
27192719
jslogContext: 'timeline.stop-recording',
27202720
});
27212721
// Profiling can't be stopped during initialization.
2722-
this.button.disabled = !options.buttonDisabled === false;
2722+
this.button.classList.toggle('hidden', options.hideStopButton);
27232723

27242724
buttonContainer.append(this.downloadTraceButton);
27252725
buttonContainer.append(this.button);
27262726
}
27272727

27282728
finish(): void {
27292729
this.stopTimer();
2730-
this.button.disabled = true;
2730+
this.button.classList.add('hidden');
27312731
}
27322732

27332733
async #downloadRawTraceAfterError(): Promise<void> {
@@ -2746,26 +2746,22 @@ export class StatusPane extends UI.Widget.VBox {
27462746
enableDownloadOfEvents(rawEvents: Trace.Types.Events.Event[]): void {
27472747
this.#rawEvents = rawEvents;
27482748
this.downloadTraceButton.disabled = false;
2749-
this.downloadTraceButton.style.visibility = 'visible';
2749+
this.downloadTraceButton.classList.remove('hidden');
27502750
}
27512751

27522752
remove(): void {
2753-
if (this.element.parentNode) {
2754-
(this.element.parentNode as HTMLElement).classList.remove('tinted');
2755-
this.arrangeDialog((this.element.parentNode as HTMLElement));
2756-
}
2753+
(this.element.parentNode as HTMLElement)?.classList.remove('tinted');
27572754
this.stopTimer();
27582755
this.element.remove();
27592756
}
27602757

27612758
showPane(parent: Element): void {
2762-
this.arrangeDialog(parent);
27632759
this.show(parent);
27642760
parent.classList.add('tinted');
27652761
}
27662762

27672763
enableAndFocusButton(): void {
2768-
this.button.disabled = false;
2764+
this.button.classList.remove('hidden');
27692765
this.button.focus();
27702766
}
27712767

@@ -2796,7 +2792,6 @@ export class StatusPane extends UI.Widget.VBox {
27962792
}
27972793

27982794
private updateTimer(): void {
2799-
this.arrangeDialog((this.element.parentNode as HTMLElement));
28002795
if (!this.timeUpdateTimer || !this.time) {
28012796
return;
28022797
}
@@ -2805,11 +2800,6 @@ export class StatusPane extends UI.Widget.VBox {
28052800
this.time.textContent = i18n.TimeUtilities.preciseSecondsToString(seconds, 1);
28062801
}
28072802

2808-
private arrangeDialog(parent: Element): void {
2809-
const isSmallDialog = parent.clientWidth < 325;
2810-
this.element.classList.toggle('small-dialog', isSmallDialog);
2811-
this.contentElement.classList.toggle('small-dialog', isSmallDialog);
2812-
}
28132803
override wasShown(): void {
28142804
super.wasShown();
28152805
this.registerCSSFiles([timelineStatusDialogStyles]);

front_end/panels/timeline/timelinePanel.css

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -596,11 +596,6 @@
596596
flex: none;
597597
}
598598

599-
.status-pane-container > .small-dialog {
600-
width: 100%;
601-
height: 100%;
602-
}
603-
604599
devtools-feedback-button {
605600
float: right;
606601
}

front_end/panels/timeline/timelineStatusDialog.css

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,6 @@
5555
border-radius: 12px;
5656
}
5757

58-
.timeline-status-dialog.small-dialog {
59-
width: inherit;
60-
justify-content: center;
61-
}
62-
63-
.small-dialog > .stop-button {
64-
align-self: center;
65-
margin-top: 20px;
66-
height: initial;
67-
}
68-
6958
@media (forced-colors: active) {
7059
.timeline-status-dialog {
7160
border: 1px solid canvastext;
@@ -81,3 +70,26 @@
8170
background-color: ButtonText;
8271
}
8372
}
73+
74+
:host {
75+
container-type: inline-size;
76+
}
77+
78+
/* 326 is the widths above (200 + 80) + a bunch of padding. calc() can't be used here sadly */
79+
@container (max-width: 326px) {
80+
.timeline-status-dialog {
81+
box-shadow: none;
82+
83+
.stop-button {
84+
align-self: center;
85+
}
86+
}
87+
88+
.status-dialog-line {
89+
flex-direction: column;
90+
91+
.label {
92+
display: none;
93+
}
94+
}
95+
}

test/e2e/helpers/performance-helpers.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,5 @@ export function veImpressionForPerformancePanel() {
382382
}
383383

384384
function veImpressionForStatusDialog() {
385-
return veImpression(
386-
'Dialog', 'timeline-status',
387-
[veImpression('Action', 'timeline.download-after-error'), veImpression('Action', 'timeline.stop-recording')]);
385+
return veImpression('Dialog', 'timeline-status');
388386
}

0 commit comments

Comments
 (0)