Make the close functionality consistent with other close buttons
Aligned the close (other action) buttons on the bottom right and put a cross icon on the top right for:
- Tooltip
- Legacy query dialog
- Bisect Dialog
- Chart (Removed "Remove all" button as the cross icon does the job)
- New/Edit Favorite dialog
- Algo dialog on the dry run page
Note: The "Remove all" button is working behind a flag. It is by default show but if the users want to hide it then add a query param show_remove_all=false.
Bug: b/361511141
Change-Id: Ica0e6b353c8c6842fb7b00fc07e6d40a527b415e
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/895037
Commit-Queue: Vidit Chitkara <viditchitkara@google.com>
Reviewed-by: Jeff Yoon <jeffyoon@google.com>
diff --git a/perf/modules/chart-tooltip-sk/BUILD.bazel b/perf/modules/chart-tooltip-sk/BUILD.bazel
index 22b446f..09dfe82 100644
--- a/perf/modules/chart-tooltip-sk/BUILD.bazel
+++ b/perf/modules/chart-tooltip-sk/BUILD.bazel
@@ -22,6 +22,7 @@
"//perf/modules/anomaly-sk",
"//perf/modules/commit-range-sk",
"//perf/modules/ingest-file-links-sk",
+ "//elements-sk/modules/icons/close-icon-sk",
],
ts_deps = [
"//:node_modules/lit",
diff --git a/perf/modules/chart-tooltip-sk/chart-tooltip-sk.scss b/perf/modules/chart-tooltip-sk/chart-tooltip-sk.scss
index be49640..b90028a 100644
--- a/perf/modules/chart-tooltip-sk/chart-tooltip-sk.scss
+++ b/perf/modules/chart-tooltip-sk/chart-tooltip-sk.scss
@@ -14,6 +14,15 @@
overflow: auto;
background: var(--surface-2dp);
+ button#closeIcon {
+ position: absolute;
+ right: 5px;
+ top: 5px;
+ border: none;
+ cursor: pointer;
+ padding: 4px;
+ }
+
h3 {
margin: 0;
margin-bottom: 16px;
@@ -55,6 +64,10 @@
display: None;
}
+ button {
+ float: right;
+ }
+
.see-more-text {
text-decoration: underline;
font-style: italic;
diff --git a/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts b/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts
index 9452c7d..c9f66c8 100644
--- a/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts
+++ b/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts
@@ -18,6 +18,7 @@
import { CommitRangeSk } from '../commit-range-sk/commit-range-sk';
import '../window/window';
import { IngestFileLinksSk } from '../ingest-file-links-sk/ingest-file-links-sk';
+import '../../../elements-sk/modules/icons/close-icon-sk';
export class Commit {
commit_hash: string = '';
@@ -136,6 +137,12 @@
class="container"
style="display: ${ele._display ? 'block' : 'none'};
left: ${ele.left}px; top: ${ele.top}px;">
+ <button
+ id="closeIcon"
+ @click=${ele._close_button_action}
+ ?hidden=${!ele._tooltip_fixed}>
+ <close-icon-sk></close-icon-sk>
+ </button>
<h3>${ele.test_name}</h3>
<ul class="table">
<li>
diff --git a/perf/modules/cluster-lastn-page-sk/BUILD.bazel b/perf/modules/cluster-lastn-page-sk/BUILD.bazel
index 862e194..b21c5a4 100644
--- a/perf/modules/cluster-lastn-page-sk/BUILD.bazel
+++ b/perf/modules/cluster-lastn-page-sk/BUILD.bazel
@@ -13,6 +13,7 @@
"//perf/modules/commit-detail-sk",
"//perf/modules/domain-picker-sk",
"//perf/modules/triage-status-sk",
+ "//elements-sk/modules/icons/close-icon-sk",
"//elements-sk/modules/spinner-sk",
],
ts_deps = [
diff --git a/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.scss b/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.scss
index b6453cf..f04cdfc 100644
--- a/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.scss
+++ b/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.scss
@@ -10,6 +10,22 @@
background: var(--background);
color: var(--on-background);
border: solid 1px var(--on-background);
+
+ #clusterCloseIcon {
+ position: absolute;
+ right: 0;
+ top: 0;
+ margin: 0;
+ margin-top: 10px;
+ margin-right: 11px;
+ border: none;
+ padding: 5px;
+ cursor: pointer;
+ }
+
+ .buttons {
+ float: right;
+ }
}
.running {
diff --git a/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.ts b/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.ts
index d06d7d0..a631a1c 100644
--- a/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.ts
+++ b/perf/modules/cluster-lastn-page-sk/cluster-lastn-page-sk.ts
@@ -21,6 +21,7 @@
import '../triage-status-sk';
import '../alert-config-sk';
import '../domain-picker-sk';
+import '../../../elements-sk/modules/icons/close-icon-sk';
import {
ParamSet,
Alert,
@@ -97,6 +98,9 @@
.config=${ele.state}
.paramset=${ele.paramset}
.key_order=${window.perf.key_order}></alert-config-sk>
+ <button id="clusterCloseIcon" @click=${ele.alertClose}>
+ <close-icon-sk></close-icon-sk>
+ </button>
<div class="buttons">
<button @click=${ele.alertClose}>Cancel</button>
<button @click=${ele.alertAccept}>Accept</button>
diff --git a/perf/modules/explore-multi-sk/explore-multi-sk.ts b/perf/modules/explore-multi-sk/explore-multi-sk.ts
index fd6c875..e9ba565 100644
--- a/perf/modules/explore-multi-sk/explore-multi-sk.ts
+++ b/perf/modules/explore-multi-sk/explore-multi-sk.ts
@@ -73,6 +73,8 @@
enable_chart_tooltip: boolean = false;
+ show_remove_all: boolean = true;
+
use_titles: boolean = false;
}
@@ -396,6 +398,7 @@
plotSummary: this.state.plotSummary,
highlight_anomalies: this.state.highlight_anomalies,
enable_chart_tooltip: this.state.enable_chart_tooltip,
+ show_remove_all: this.state.show_remove_all,
use_titles: this.state.use_titles,
};
explore.state = newState;
diff --git a/perf/modules/explore-simple-sk/BUILD.bazel b/perf/modules/explore-simple-sk/BUILD.bazel
index 7f79504..8a5234a 100644
--- a/perf/modules/explore-simple-sk/BUILD.bazel
+++ b/perf/modules/explore-simple-sk/BUILD.bazel
@@ -25,6 +25,7 @@
"//elements-sk/modules/icons/expand-less-icon-sk",
"//elements-sk/modules/icons/expand-more-icon-sk",
"//elements-sk/modules/icons/help-icon-sk",
+ "//elements-sk/modules/icons/close-icon-sk",
"//elements-sk/modules/spinner-sk",
"//elements-sk/modules/tabs-panel-sk",
"//elements-sk/modules/tabs-sk",
diff --git a/perf/modules/explore-simple-sk/explore-simple-sk.scss b/perf/modules/explore-simple-sk/explore-simple-sk.scss
index dfb4246..cb1cddc 100644
--- a/perf/modules/explore-simple-sk/explore-simple-sk.scss
+++ b/perf/modules/explore-simple-sk/explore-simple-sk.scss
@@ -11,6 +11,18 @@
#selections {
margin: 0 1em;
+
+ #closeQueryIcon {
+ position: absolute;
+ right: 0;
+ top: 0;
+ margin: 0;
+ margin-top: 10px;
+ margin-right: 11px;
+ border: none;
+ padding: 5px;
+ cursor: pointer;
+ }
}
#commits,
@@ -132,6 +144,7 @@
display: flex;
align-items: center;
gap: 8px;
+ flex: 1;
button,
checkbox-sk {
@@ -143,6 +156,35 @@
[hidden] {
display: none;
}
+
+ #zoomPan {
+ h3 {
+ font-size: 14px;
+ margin: 0;
+ }
+
+ #btnContainer {
+ button {
+ min-height: 0;
+
+ span {
+ font-size: 16px;
+ height: auto;
+ width: auto;
+ }
+ }
+ }
+ }
+ }
+
+ button#removeAll {
+ padding: 5px;
+ border: none;
+ cursor: pointer;
+ float: right;
+ margin-left: 10px;
+ margin-bottom: 30px;
+ margin-right: 30px;
}
#traceButtons {
@@ -275,6 +317,18 @@
overflow: auto;
width: 45%;
+ #bisectCloseIcon {
+ position: absolute;
+ right: 0;
+ top: 0;
+ margin: 0;
+ margin-top: 10px;
+ margin-right: 11px;
+ border: none;
+ padding: 5px;
+ cursor: pointer;
+ }
+
paramset-sk {
display: inline-block;
diff --git a/perf/modules/explore-simple-sk/explore-simple-sk.ts b/perf/modules/explore-simple-sk/explore-simple-sk.ts
index 040919a..19a721c 100644
--- a/perf/modules/explore-simple-sk/explore-simple-sk.ts
+++ b/perf/modules/explore-simple-sk/explore-simple-sk.ts
@@ -23,6 +23,7 @@
import '../../../elements-sk/modules/icons/expand-less-icon-sk';
import '../../../elements-sk/modules/icons/expand-more-icon-sk';
import '../../../elements-sk/modules/icons/help-icon-sk';
+import '../../../elements-sk/modules/icons/close-icon-sk';
import '../../../elements-sk/modules/spinner-sk';
import '../../../elements-sk/modules/tabs-panel-sk';
import '../../../elements-sk/modules/tabs-sk';
@@ -324,6 +325,8 @@
enable_chart_tooltip: boolean = false;
+ show_remove_all: boolean = true;
+
use_titles: boolean = false;
}
@@ -556,6 +559,8 @@
private graphTitle: GraphTitleSk | null = null;
+ private showRemoveAll = true;
+
constructor(scrollable: boolean, useTestPicker?: boolean) {
super(ExploreSimpleSk.template);
this.scrollable = scrollable;
@@ -585,6 +590,7 @@
</button>
<button
+ ?hidden=${!ele.showRemoveAll}
@click=${() => ele.removeAll(false)}
title='Remove all the traces.'>
Remove All
@@ -686,38 +692,48 @@
@click=${ele.openBisect}>
Bisect
</button>
- <h3>Zoom/Pan:<h3>
- <button
- class=navigation
- @click=${ele.zoomInKey}
- title='Zoom In'>
- <span class=icon-sk>add</span>
- </button>
- <button
- class=navigation
- @click=${ele.zoomOutKey}
- title='Zoom Out'>
- <span class=icon-sk>remove</span>
- </button>
- <button
- class=navigation
- @click=${ele.zoomLeftKey}
- title='Pan Left'>
- <span class=icon-sk>arrow_back</span>
- </button>
- <button
- class=navigation
- @click=${ele.zoomRightKey}
- title='Pan Right'>
- <span class=icon-sk>arrow_forward</span>
- </button>
+ <div id="zoomPan">
+ <h3>Zoom/Pan:</h3>
+ <div id="btnContainer">
+ <button
+ class=navigation
+ @click=${ele.zoomInKey}
+ title='Zoom In'>
+ <span class=icon-sk>add</span>
+ </button>
+ <button
+ class=navigation
+ @click=${ele.zoomOutKey}
+ title='Zoom Out'>
+ <span class=icon-sk>remove</span>
+ </button>
+ <button
+ class=navigation
+ @click=${ele.zoomLeftKey}
+ title='Pan Left'>
+ <span class=icon-sk>arrow_back</span>
+ </button>
+ <button
+ class=navigation
+ @click=${ele.zoomRightKey}
+ title='Pan Right'>
+ <span class=icon-sk>arrow_forward</span>
+ </button>
+ </div>
+ </div>
</div>
+ <button
+ id="removeAll"
+ @click=${() => ele.removeAll(false)}
+ title='Remove all the traces.'>
+ <close-icon-sk></close-icon-sk>
+ </button>
</div>
</div>
<graph-title-sk id=graphTitle></graph-title-sk>
- <div id=spin-overlay @mouseleave=${ele.disableTooltip}>
+ <div id=spin-overlay @mouseleave=${ele.mouseLeave}>
<chart-tooltip-sk></chart-tooltip-sk>
<plot-simple-sk
.summary=${ele._state.summary}
@@ -761,6 +777,9 @@
> </query-sk>
<div id=selections>
<h3>Selections</h3>
+ <button id="closeQueryIcon" @click=${ele.closeQueryDialog}>
+ <close-icon-sk></close-icon-sk>
+ </button>
<paramset-sk id=summary removable_values @paramset-value-remove-click=${
ele.paramsetRemoveClick
}></paramset-sk>
@@ -832,6 +851,9 @@
<dialog id='bisect-dialog'>
<h2>Bisect</h2>
+ <button id="bisectCloseIcon" @click=${ele.closeBisectDialog}>
+ <close-icon-sk></close-icon-sk>
+ </button>
<h3>Test Path</h3>
<input id="testpath" type="text" value=${ele.testPath} readonly></input>
<h3>Bug ID</h3>
@@ -1069,6 +1091,8 @@
// with the real function once stateReflector has been setup.
// eslint-disable-next-line @typescript-eslint/no-empty-function
private _stateHasChanged = () => {
+ this.showRemoveAll = this._state.show_remove_all;
+
// If chart tooltip is enabled do not show crosshair label
this.plot!.showCrosshairLabel = !this._state.enable_chart_tooltip;
@@ -1167,6 +1191,12 @@
case 'd':
this.zoomRightKey();
break;
+ case `Escape`:
+ this.disableTooltip();
+ break;
+ case `Esc`:
+ this.disableTooltip();
+ break;
default:
break;
}
@@ -1645,10 +1675,15 @@
);
}
- /** Hides the tooltip. Generally called when mouse moves out of the graph */
- disableTooltip(): void {
+ // Triggered on mouseLeave event
+ mouseLeave(): void {
if (this.tooltipFixed) return;
+ this.disableTooltip();
+ }
+
+ /** Hides the tooltip. Generally called when mouse moves out of the graph */
+ disableTooltip(): void {
const tooltipElem = $$<ChartTooltipSk>('chart-tooltip-sk', this);
tooltipElem!.display = false;
this._render();
diff --git a/perf/modules/favorites-dialog-sk/BUILD.bazel b/perf/modules/favorites-dialog-sk/BUILD.bazel
index da8f7e8..c3b89bb 100644
--- a/perf/modules/favorites-dialog-sk/BUILD.bazel
+++ b/perf/modules/favorites-dialog-sk/BUILD.bazel
@@ -20,6 +20,7 @@
sass_srcs = ["favorites-dialog-sk.scss"],
sk_element_deps = [
"//elements-sk/modules/spinner-sk",
+ "//elements-sk/modules/icons/close-icon-sk",
],
ts_deps = [
"//:node_modules/lit",
diff --git a/perf/modules/favorites-dialog-sk/favorites-dialog-sk.scss b/perf/modules/favorites-dialog-sk/favorites-dialog-sk.scss
index dd9610f..6be4f01 100644
--- a/perf/modules/favorites-dialog-sk/favorites-dialog-sk.scss
+++ b/perf/modules/favorites-dialog-sk/favorites-dialog-sk.scss
@@ -8,6 +8,18 @@
background: var(--background);
padding: 24px;
+ #favCloseIcon {
+ position: absolute;
+ right: 0;
+ top: 0;
+ margin: 0;
+ margin-top: 10px;
+ margin-right: 11px;
+ border: none;
+ padding: 5px;
+ cursor: pointer;
+ }
+
h2 {
font-size: 2em !important;
margin-bottom: 40px !important;
@@ -30,8 +42,11 @@
padding: 6px;
}
- button {
- font-size: 1.2em !important;
+ .buttons {
+ float: right;
+ button {
+ font-size: 1.2em !important;
+ }
}
}
}
diff --git a/perf/modules/favorites-dialog-sk/favorites-dialog-sk.ts b/perf/modules/favorites-dialog-sk/favorites-dialog-sk.ts
index a912bed..e50089c 100644
--- a/perf/modules/favorites-dialog-sk/favorites-dialog-sk.ts
+++ b/perf/modules/favorites-dialog-sk/favorites-dialog-sk.ts
@@ -11,6 +11,7 @@
import { define } from '../../../elements-sk/modules/define';
import { errorMessage } from '../../../elements-sk/modules/errorMessage';
import '../../../elements-sk/modules/spinner-sk';
+import '../../../elements-sk/modules/icons/close-icon-sk';
// FavoritesDialogSk is a modal that contains a form to capture user
// input for adding/editing a new favorite.
@@ -138,6 +139,10 @@
<dialog id="favDialog">
<h2>Favorite</h2>
+ <button id="favCloseIcon" @click=${ele.dismiss}>
+ <close-icon-sk></close-icon-sk>
+ </button>
+
<div id=spinContainer>
<spinner-sk ?active=${ele.updatingFavorite}></spinner-sk>
</div>
