Remove more plot-simple references from explore-simple
Bug: 408212152
Removing most of leftover plot-simple references. Leaving
show_google_plot references for now, will remove later.
Explore-demo is in quite a bad shape. I migrated it so it doesn't break
when we remove plot-simple, but it needs some polishing. This will be
done in https://buganizer.corp.google.com/issues/450184956.
Also, Gold properly identified a difference: the missing "New Query"
button was only visible for a split-second. If we use plot-google-chart,
we open the dialog automatically, so there's no need for a user action.
Change-Id: I3371d684ddebb7d0152fe274b15c5f9256321d87
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/1068996
Commit-Queue: Marcin Mordecki <mordeckimarcin@google.com>
Reviewed-by: Sergei Rudenkov <sergeirudenkov@google.com>
diff --git a/perf/modules/explore-multi-sk/explore-multi-sk.ts b/perf/modules/explore-multi-sk/explore-multi-sk.ts
index f5f9fd1..c4d75b3 100644
--- a/perf/modules/explore-multi-sk/explore-multi-sk.ts
+++ b/perf/modules/explore-multi-sk/explore-multi-sk.ts
@@ -1357,7 +1357,6 @@
use_titles: this.state.use_titles,
useTestPicker: this.state.useTestPicker,
use_test_picker_query: false,
- show_google_plot: this.state.show_google_plot,
enable_favorites: this.canAddFav(),
hide_paramset: true,
graph_index: index,
diff --git a/perf/modules/explore-multi-sk/explore-multi-sk_test.ts b/perf/modules/explore-multi-sk/explore-multi-sk_test.ts
index 68eeb54..e32c30e 100644
--- a/perf/modules/explore-multi-sk/explore-multi-sk_test.ts
+++ b/perf/modules/explore-multi-sk/explore-multi-sk_test.ts
@@ -521,7 +521,6 @@
use_titles: false,
useTestPicker: false,
use_test_picker_query: false,
- show_google_plot: false,
enable_favorites: false,
hide_paramset: false,
horizontal_zoom: false,
diff --git a/perf/modules/explore-simple-sk/explore-simple-sk.ts b/perf/modules/explore-simple-sk/explore-simple-sk.ts
index 16a43a9..78a5b04 100644
--- a/perf/modules/explore-simple-sk/explore-simple-sk.ts
+++ b/perf/modules/explore-simple-sk/explore-simple-sk.ts
@@ -11,7 +11,7 @@
import { MdSwitch } from '@material/web/switch/switch.js';
import { define } from '../../../elements-sk/modules/define';
import { jsonOrThrow } from '../../../infra-sk/modules/jsonOrThrow';
-
+import { AnomalyData } from '../common/anomaly-data';
import { toParamSet, fromParamSet } from '../../../infra-sk/modules/query';
import { TabsSk } from '../../../elements-sk/modules/tabs-sk/tabs-sk';
import { ToastSk } from '../../../elements-sk/modules/toast-sk/toast-sk';
@@ -83,7 +83,7 @@
TraceMetadata,
TraceCommitLink,
} from '../json';
-import { PlotSimpleSk, PlotSimpleSkTraceEventDetails } from '../plot-simple-sk/plot-simple-sk';
+import { PlotSimpleSkTraceEventDetails } from '../plot-simple-sk/plot-simple-sk';
import {
ParamSetSk,
ParamSetSkCheckboxClickEventDetail,
@@ -91,7 +91,6 @@
ParamSetSkKeyCheckboxClickEventDetail,
ParamSetSkRemoveClickEventDetail,
} from '../../../infra-sk/modules/paramset-sk/paramset-sk';
-import { AnomalyData } from '../common/anomaly-data';
import {
QuerySk,
QuerySkQueryChangeEventDetail,
@@ -346,8 +345,6 @@
use_test_picker_query: boolean = false;
- show_google_plot = false;
-
// boolean indicate for enabling favorites action. set by explore-sk.
// requires user to be logged in so that the favorites can be saved for the user.
enable_favorites: boolean = false;
@@ -523,8 +520,6 @@
private percent: HTMLSpanElement | null = null;
- private plotSimple = createRef<PlotSimpleSk>();
-
private googleChartPlot = createRef<PlotGoogleChartSk>();
private plotSummary = createRef<PlotSummarySk>();
@@ -656,7 +651,7 @@
private static template = (ele: ExploreSimpleSk) => html`
<dataframe-repository-sk ${ref(ele.dfRepo)}>
<div id=explore class=${ele.displayMode}>
- <div id=buttons style="${ele._state.show_google_plot ? 'display: none' : ''}">
+ <div id=buttons style="display: none">
<button
id=open_query_dialog
?hidden=${ele.useTestPicker}
@@ -740,33 +735,22 @@
<div id=spin-overlay @mouseleave=${ele.mouseLeave}>
<div class="chart-container">
- ${
- ele._state.show_google_plot
- ? html`<plot-google-chart-sk
- .domain=${ele._state.domain}
- ${ref(ele.googleChartPlot)}
- .highlightAnomalies=${ele._state.highlight_anomalies}
- @plot-data-select=${ele.onChartSelect}
- @plot-data-mouseover=${ele.onChartOver}
- @plot-data-mousedown=${ele.onChartMouseDown}
- @selection-changing=${ele.OnSelectionRange}
- @selection-changed=${ele.OnSelectionRange}>
- <md-icon slot="untriage">help</md-icon>
- <md-icon slot="regression">report</md-icon>
- <md-icon slot="improvement">check_circle</md-icon>
- <md-icon slot="ignored">report_off</md-icon>
- <md-icon slot="issue">chat_bubble</md-icon>
- <md-text slot="xbar">|</md-text>
- </plot-google-chart-sk>`
- : html`<plot-simple-sk
- .summary=${ele._state.summary}
- ${ref(ele.plotSimple)}
- @trace_selected=${ele.traceSelected}
- @zoom=${ele.plotZoom}
- @trace_focused=${ele.plotTraceFocused}
- class="hide_on_pivot_table hide_on_query_only hide_on_spinner">
- </plot-simple-sk>`
- }
+ ${html`<plot-google-chart-sk
+ .domain=${ele._state.domain}
+ ${ref(ele.googleChartPlot)}
+ .highlightAnomalies=${ele._state.highlight_anomalies}
+ @plot-data-select=${ele.onChartSelect}
+ @plot-data-mouseover=${ele.onChartOver}
+ @plot-data-mousedown=${ele.onChartMouseDown}
+ @selection-changing=${ele.OnSelectionRange}
+ @selection-changed=${ele.OnSelectionRange}>
+ <md-icon slot="untriage">help</md-icon>
+ <md-icon slot="regression">report</md-icon>
+ <md-icon slot="improvement">check_circle</md-icon>
+ <md-icon slot="ignored">report_off</md-icon>
+ <md-icon slot="issue">chat_bubble</md-icon>
+ <md-text slot="xbar">|</md-text>
+ </plot-google-chart-sk>`}
<chart-tooltip-sk></chart-tooltip-sk>
</div>
${when(ele._state.plotSummary && ele.tracesRendered, () => ele.plotSummaryTemplate())}
@@ -1481,11 +1465,6 @@
// with the real function once stateReflector has been setup.
// eslint-disable-next-line @typescript-eslint/no-empty-function
private _stateHasChanged = () => {
- // If chart tooltip is enabled do not show crosshair label
- if (this.plotSimple.value) {
- this.plotSimple.value.showCrosshairLabel = !this._state.enable_chart_tooltip;
- }
-
this.dispatchEvent(new CustomEvent('state_changed', {}));
};
@@ -1576,10 +1555,7 @@
* }
*/
private getCurrentZoom(): ZoomWithDelta {
- let zoom = this.plotSimple.value?.zoom;
- if (!zoom) {
- zoom = [0, this._dataframe.header!.length - 1];
- }
+ const zoom = [0, this._dataframe.header!.length - 1];
let delta = zoom[1] - zoom[0];
if (delta < MIN_ZOOM_RANGE) {
const mid = (zoom[0] + zoom[1]) / 2;
@@ -1664,10 +1640,6 @@
this.rangeChangeImpl();
})
.catch(errorMessage);
- } else {
- if (this.plotSimple.value) {
- this.plotSimple.value.zoom = zoom;
- }
}
}
@@ -1766,29 +1738,6 @@
this._stateHasChanged();
}
- /** Reflect the focused trace in the paramset. */
- private plotTraceFocused({ detail }: CustomEvent<PlotSimpleSkTraceEventDetails>) {
- const header = this.dfRepo.value?.dataframe.header;
- const index = (this.selectedRange?.begin || 0) + detail.x;
- const selected = header![index]!;
- this.paramset!.highlight = fromKey(detail.name);
- this.commitTime!.textContent = new Date(selected.timestamp * 1000).toLocaleString(undefined, {
- hourCycle: 'h23',
- });
-
- if (this._state.enable_chart_tooltip && !this.tooltipSelected) {
- const commitPos = selected.offset;
- const currentCommit = this.getCommitDetails(commitPos);
- const prevCommit = this.getCommitDetails(this.getPreviousCommit(index, detail.name));
- this.enableTooltip(detail, [prevCommit, currentCommit], false);
- }
- }
-
- /** User has zoomed in on the graph. */
- private plotZoom() {
- this.render();
- }
-
/** get story name for pinpoint. */
private getLastSubtest(d: any) {
const tmp =
@@ -2164,34 +2113,17 @@
// updated in place in triage popup. This may cause the data inconsistency to manipulate
// data in several places.
// Ideally, dataframe_context should nudge anomaly data.
- const anomalyDataMap = this.plotSimple.value?.anomalyDataMap;
- let anomalyDataInPlot: AnomalyData | null = null;
- if (anomalyDataMap) {
- const traceAnomalies = anomalyDataMap[traceName];
- if (traceAnomalies) {
- for (let i = 0; i < traceAnomalies.length; i++) {
- if (pointDetails.x === traceAnomalies[i].x) {
- anomalyDataInPlot = traceAnomalies[i];
- break;
- }
- }
- }
- }
- // -- to be refactored. see above--
-
// Map an anomaly ID to a list of Nudge Entries.
// TODO(b/375678060): Reflect anomaly coordinate changes unto summary bar.
const nudgeList: NudgeEntry[] = [];
if (anomaly) {
// This is only to be backward compatible with anomaly data in simple-plot.
- const anomalyData = this.plotSimple.value
- ? anomalyDataInPlot
- : {
- anomaly: anomaly,
- x: commitPosition,
- y: pointDetails.y,
- highlight: false,
- };
+ const anomalyData: AnomalyData = {
+ anomaly: anomaly,
+ x: commitPosition,
+ y: pointDetails.y,
+ highlight: false,
+ };
const headerLength = this.dfRepo.value!.dataframe.header!.length;
for (let i = -NUDGE_RANGE; i <= NUDGE_RANGE; i++) {
@@ -2551,13 +2483,11 @@
* @param e Checkbox click event
*/
private paramsetKeyCheckboxClick(e: CustomEvent<ParamSetSkKeyCheckboxClickEventDetail>) {
- if (this._state.show_google_plot) {
- this.googleChartPlot.value?.updateChartForParam(
- e.detail.key,
- e.detail.values,
- e.detail.selected
- );
- }
+ this.googleChartPlot.value?.updateChartForParam(
+ e.detail.key,
+ e.detail.values,
+ e.detail.selected
+ );
}
/**
@@ -2565,13 +2495,11 @@
* @param e Checkbox click event
*/
private paramsetCheckboxClick(e: CustomEvent<ParamSetSkCheckboxClickEventDetail>) {
- if (this._state.show_google_plot) {
- this.googleChartPlot.value?.updateChartForParam(
- e.detail.key,
- [e.detail.value],
- e.detail.selected
- );
- }
+ this.googleChartPlot.value?.updateChartForParam(
+ e.detail.key,
+ [e.detail.value],
+ e.detail.selected
+ );
}
private AddPlotLines() {
@@ -2685,14 +2613,6 @@
}
});
- if (this.plotSimple.value) {
- // Additively highlight if the ctrl key is pressed.
- if (e.detail.ctrl) {
- this.plotSimple.value!.highlight = this.plotSimple.value!.highlight.concat(keys);
- } else {
- this.plotSimple.value!.highlight = keys;
- }
- }
this.render();
}
@@ -3279,9 +3199,6 @@
case 'use_titles':
this._state.use_titles = paramValue;
break;
- case 'show_google_plot':
- this._state.show_google_plot = paramValue;
- break;
case 'disable_filter_parent_traces':
this._state.disable_filter_parent_traces = paramValue;
break;
@@ -3303,7 +3220,6 @@
this._state.formulas = [];
this._state.queries = [];
this._state.keys = '';
- this.plotSimple.value?.removeAll();
this._dataframe.header = [];
this._dataframe.traceset = TraceSet({});
if (this.graphTitle) {
@@ -3419,7 +3335,7 @@
`/m/?begin=${this._state.begin}&end=${this._state.end}` +
`&pageSize=${chartsPerPage}&shortcut=${newShortcut}` +
`&totalGraphs=${graphConfigs.length}` +
- (this.state.show_google_plot ? `&show_google_plot=true` : ``),
+ `&show_google_plot=true`,
'_self'
);
}
diff --git a/perf/modules/explore-sk/explore-sk-demo.ts b/perf/modules/explore-sk/explore-sk-demo.ts
index cb1a9b4..93f1d88 100644
--- a/perf/modules/explore-sk/explore-sk-demo.ts
+++ b/perf/modules/explore-sk/explore-sk-demo.ts
@@ -39,6 +39,7 @@
show_bisect_btn: true,
};
+// TODO(b/450184956) Rewrite demo to plot-google-chart. This demo is unusable.
customElements.whenDefined('explore-sk').then(() => {
// Insert the element later, which should give enough time for fetchMock to be in place.
document
@@ -52,11 +53,13 @@
// Some utility functions used later.
+ // TODO(b/450184956) Rewrite demo to plot-google-chart
// Clicks inside the canvas element inside the plot-simple-sk element.
const clickOnPlot = () => {
const rect = explore!.querySelector<HTMLCanvasElement>('canvas')!.getBoundingClientRect();
// eslint-disable-next-line dot-notation
- explore!['plotSimple'].value!.dispatchEvent(
+
+ explore!['googleChartPlot'].value!.dispatchEvent(
new MouseEvent('click', {
// Pick a point in the middle of the canvas.
clientX: rect.left + rect.width / 2,
@@ -67,9 +70,11 @@
// Calls itself via timeout until the plot-simple-sk element reports that it
// has traces loaded, at which point it calls clickOnPlot.
+ // TODO(b/450184956) Rewrite demo to plot-google-chart
+
const checkIfLoaded = () => {
// eslint-disable-next-line dot-notation
- if (explore!['plotSimple'].value!.getLineNames().length > 1) {
+ if (explore!['googleChartPlot'].value!.getAllTraces().length > 1) {
clickOnPlot();
} else {
setTimeout(checkIfLoaded, 100);
diff --git a/perf/modules/explore-sk/explore-sk_puppeteer_test.ts b/perf/modules/explore-sk/explore-sk_puppeteer_test.ts
index 700a480..1a5a1b8 100644
--- a/perf/modules/explore-sk/explore-sk_puppeteer_test.ts
+++ b/perf/modules/explore-sk/explore-sk_puppeteer_test.ts
@@ -1,6 +1,6 @@
import { assert } from 'chai';
import { loadCachedTestBed, takeScreenshot, TestBed } from '../../../puppeteer-tests/util';
-
+// TODO(b/450184956) Rewrite demo to plot-google-chart
describe('explore-sk', () => {
let testBed: TestBed;
before(async () => {
