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>