[query-sk] Fix issues with selecting when a filter is used.
* Reformat code for eslint.
* Remove query-summary-sk from query-sk demo page since it currently
sits in Perf and not infra-sk.
Change-Id: I97fb42e8e0c8759ebef49993a79575785ddeca0b
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/279337
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
diff --git a/infra-sk/modules/query-sk/query-sk-demo.html b/infra-sk/modules/query-sk/query-sk-demo.html
index 9333127..656aa9b 100644
--- a/infra-sk/modules/query-sk/query-sk-demo.html
+++ b/infra-sk/modules/query-sk/query-sk-demo.html
@@ -1,70 +1,20 @@
<!DOCTYPE html>
<html>
-<head>
- <title>query-sk</title>
- <meta charset="utf-8" />
- <meta http-equiv="X-UA-Compatible" content="IE=edge">
- <meta name="viewport" content="width=device-width, initial-scale=1.0">
-</head>
-<body>
- <h1>query-sk</h1>
- <query-sk></query-sk>
- <h2>Actions</h2>
- <div>
- <button id=swap>Swap Paramset</button>
- </div>
+ <head>
+ <title>query-sk</title>
+ <meta charset="utf-8" />
+ <meta http-equiv="X-UA-Compatible" content="IE=edge" />
+ <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+ </head>
+ <body>
+ <h1>query-sk</h1>
+ <query-sk></query-sk>
+ <h2>Actions</h2>
+ <div>
+ <button id="swap">Swap Paramset</button>
+ </div>
- <h2>Events</h2>
- <pre id=events></pre>
- <query-summary-sk></query-summary-sk>
- <script type="text/javascript" charset="utf-8">
- const q = document.querySelector('query-sk');
- const summary = document.querySelector('query-summary-sk');
- const events = document.querySelector('#events');
- q.addEventListener('query-change', (e) => {
- events.textContent = JSON.stringify(e.detail, null, ' ');
- summary.selection = e.detail.q;
- });
-
- var n = 0;
- var paramset = {
- "config": ["565", "8888"],
- "type": ["CPU", "GPU"],
- "units": ["ms", "bytes"],
- "test": [
- "DeferredSurfaceCopy_discardable",
- "DeferredSurfaceCopy_nonDiscardable",
- "GLInstancedArraysBench_instance",
- "GLInstancedArraysBench_one_0",
- "GLInstancedArraysBench_one_1",
- "GLInstancedArraysBench_one_2",
- "GLInstancedArraysBench_one_4",
- "GLInstancedArraysBench_one_8",
- "GLInstancedArraysBench_two_0",
- "GLInstancedArraysBench_two_1",
- "GLInstancedArraysBench_two_2",
- "GLInstancedArraysBench_two_4",
- "GLInstancedArraysBench_two_8",
- "GLVec4ScalarBench_scalar_1_stage",
- "GLVec4ScalarBench_scalar_2_stage",
- ],
- };
- var paramset2 = {
- "config": ["565"],
- "type": ["CPU", "GPU"],
- "test": [
- "DeferredSurfaceCopy_discardable",
- "DeferredSurfaceCopy_nonDiscardable",
- ],
- };
- q.paramset = paramset;
- q.key_order = ['test', 'units'];
-
- document.querySelector('#swap').addEventListener('click', function(e){
- n = (n + 1) % 2;
- q.paramset = [paramset, paramset2][n];
- });
-
- </script>
-</body>
+ <h2>Events</h2>
+ <pre id="events"></pre>
+ </body>
</html>
diff --git a/infra-sk/modules/query-sk/query-sk-demo.js b/infra-sk/modules/query-sk/query-sk-demo.js
index e2655d9..b67e788 100644
--- a/infra-sk/modules/query-sk/query-sk-demo.js
+++ b/infra-sk/modules/query-sk/query-sk-demo.js
@@ -1,2 +1,46 @@
-import './index.js'
-import '../query-summary-sk'
+import './index';
+
+const q = document.querySelector('query-sk');
+const events = document.querySelector('#events');
+q.addEventListener('query-change', (e) => {
+ events.textContent = JSON.stringify(e.detail, null, ' ');
+});
+
+let n = 0;
+const paramset = {
+ config: ['565', '8888'],
+ type: ['CPU', 'GPU'],
+ units: ['ms', 'bytes'],
+ test: [
+ 'DeferredSurfaceCopy_discardable',
+ 'DeferredSurfaceCopy_nonDiscardable',
+ 'GLInstancedArraysBench_instance',
+ 'GLInstancedArraysBench_one_0',
+ 'GLInstancedArraysBench_one_1',
+ 'GLInstancedArraysBench_one_2',
+ 'GLInstancedArraysBench_one_4',
+ 'GLInstancedArraysBench_one_8',
+ 'GLInstancedArraysBench_two_0',
+ 'GLInstancedArraysBench_two_1',
+ 'GLInstancedArraysBench_two_2',
+ 'GLInstancedArraysBench_two_4',
+ 'GLInstancedArraysBench_two_8',
+ 'GLVec4ScalarBench_scalar_1_stage',
+ 'GLVec4ScalarBench_scalar_2_stage',
+ ],
+};
+const paramset2 = {
+ config: ['565'],
+ type: ['CPU', 'GPU'],
+ test: [
+ 'DeferredSurfaceCopy_discardable',
+ 'DeferredSurfaceCopy_nonDiscardable',
+ ],
+};
+q.paramset = paramset;
+q.key_order = ['test', 'units'];
+
+document.querySelector('#swap').addEventListener('click', () => {
+ n = (n + 1) % 2;
+ q.paramset = [paramset, paramset2][n];
+});
diff --git a/infra-sk/modules/query-sk/query-sk.js b/infra-sk/modules/query-sk/query-sk.js
index a381c70..ebfc713 100644
--- a/infra-sk/modules/query-sk/query-sk.js
+++ b/infra-sk/modules/query-sk/query-sk.js
@@ -17,18 +17,16 @@
* @attr {boolean} hide_regex - If the option to include regex in the query should be made
* available to the user.
*/
-import { define } from 'elements-sk/define'
-import { ElementSk } from '../../../infra-sk/modules/ElementSk'
-import { html } from 'lit-html'
-import { toParamSet, fromParamSet } from 'common-sk/modules/query'
+import { define } from 'elements-sk/define';
+import { html } from 'lit-html';
+import { toParamSet, fromParamSet } from 'common-sk/modules/query';
+import { ElementSk } from '../ElementSk';
-import '../query-values-sk'
-import 'elements-sk/select-sk'
-import 'elements-sk/styles/buttons'
+import '../query-values-sk';
+import 'elements-sk/select-sk';
+import 'elements-sk/styles/buttons';
-const _keys = (ele) => {
- return ele._keys.map((k) => html`<div>${k}</div>`);
-};
+const _keys = (ele) => ele._keys.map((k) => html`<div>${k}</div>`);
const template = (ele) => html`
<div>
@@ -82,13 +80,36 @@
_valuesChanged(e) {
const key = this._keys[this._keySelect.selection];
- this._query[key] = e.detail;
+ if (this._fast.value.trim() !== '') {
+ // Things get complicated if the user has entered a filter. The user may
+ // have selections in this._query[key] which don't appear in e.detail
+ // because they have been filtered out, so we should only add/remove
+ // values from this._query[key] that appear in this._paramset[key].
+
+ // Make everything into Sets to make our lives easier.
+ const valuesDisplayed = new Set(this._paramset[key]);
+ const currentQueryForKey = new Set(this._query[key]);
+ const selectionsFromEvent = new Set(e.detail);
+ // Loop over valuesDisplayed, if the value appears in selectionsFromEvent
+ // then add it to currentQueryForKey, otherwise remove it from
+ // currentQueryForKey.
+ valuesDisplayed.forEach((value) => {
+ if (selectionsFromEvent.has(value)) {
+ currentQueryForKey.add(value);
+ } else {
+ currentQueryForKey.delete(value);
+ }
+ });
+ this._query[key] = [...currentQueryForKey];
+ } else {
+ this._query[key] = e.detail;
+ }
this._queryChanged();
}
_keyChange() {
if (this._keySelect.selection === -1) {
- return
+ return;
}
const key = this._keys[this._keySelect.selection];
this._values.options = this._paramset[key] || [];
@@ -97,11 +118,11 @@
}
_recalcKeys() {
- let keys = Object.keys(this._paramset);
+ const keys = Object.keys(this._paramset);
keys.sort();
// Pull out all the keys that appear in _key_order to be pushed to the front of the list.
- let pre = this._key_order.filter(ordered => keys.indexOf(ordered) > -1);
- let post = keys.filter(key => pre.indexOf(key) === -1);
+ const pre = this._key_order.filter((ordered) => keys.indexOf(ordered) > -1);
+ const post = keys.filter((key) => pre.indexOf(key) === -1);
this._keys = pre.concat(post);
}
@@ -118,13 +139,13 @@
this.current_query = fromParamSet(this._query);
if (prev_query !== this.current_query) {
this.dispatchEvent(new CustomEvent('query-change', {
- detail: {q: this.current_query},
+ detail: { q: this.current_query },
bubbles: true,
}));
clearTimeout(this._delayedTimeout);
this._delayedTimeout = setTimeout(() => {
this.dispatchEvent(new CustomEvent('query-change-delayed', {
- detail: {q: this.current_query},
+ detail: { q: this.current_query },
bubbles: true,
}));
}, DELAY_MS);
@@ -145,7 +166,7 @@
// Create a closure that returns true if the given label matches the filter.
const matches = (s) => {
s = s.toLowerCase();
- return filters.filter(f => s.indexOf(f) > -1).length > 0;
+ return filters.filter((f) => s.indexOf(f) > -1).length > 0;
};
// Loop over this._originalParamset.
@@ -175,13 +196,14 @@
}
_clearFilter() {
- this._fast.value = "";
+ this._fast.value = '';
this.paramset = this._originalParamset;
this._queryChanged();
}
/** @prop paramset {Object} A serialized paramtools.ParamSet. */
- get paramset() { return this._paramset }
+ get paramset() { return this._paramset; }
+
set paramset(val) {
// Record the current key so we can restore it later.
let prevSelectKey = '';
@@ -208,7 +230,8 @@
* should appear. All keys not in the key order will be present after and in
* alphabetical order.
*/
- get key_order() { return this._key_order }
+ get key_order() { return this._key_order; }
+
set key_order(val) {
this._key_order = val;
this._recalcKeys();
@@ -216,7 +239,8 @@
}
/** @prop hide_invert {boolean} Mirrors the hide_invert attribute. */
- get hide_invert() { return this.hasAttribute('hide_invert'); }
+ get hide_invert() { return this.hasAttribute('hide_invert'); }
+
set hide_invert(val) {
if (val) {
this.setAttribute('hide_invert', '');
@@ -227,7 +251,8 @@
}
/** @prop hide_regex {boolean} Mirrors the hide_regex attribute. */
- get hide_regex() { return this.hasAttribute('hide_regex'); }
+ get hide_regex() { return this.hasAttribute('hide_regex'); }
+
set hide_regex(val) {
if (val) {
this.setAttribute('hide_regex', '');
@@ -243,6 +268,7 @@
/** @prop current_query {string} Mirrors the current_query attribute. */
get current_query() { return this.getAttribute('current_query'); }
+
set current_query(val) { this.setAttribute('current_query', val); }
attributeChangedCallback(name, oldValue, newValue) {
diff --git a/infra-sk/modules/query-sk/query-sk_test.js b/infra-sk/modules/query-sk/query-sk_test.js
index c9cbbbc..f22a8b7 100644
--- a/infra-sk/modules/query-sk/query-sk_test.js
+++ b/infra-sk/modules/query-sk/query-sk_test.js
@@ -1,38 +1,39 @@
-import './index.js'
-import { $ } from 'common-sk/modules/dom'
+import './index.js';
+import { $, $$ } from 'common-sk/modules/dom';
+import { toParamSet } from 'common-sk/modules/query';
-let container = document.createElement('div');
+const container = document.createElement('div');
document.body.appendChild(container);
-afterEach(function() {
+afterEach(() => {
container.innerHTML = '';
});
const paramset = {
- 'arch': [
+ arch: [
'WASM',
'arm',
'arm64',
'asmjs',
'wasm',
'x86',
- 'x86_64'
+ 'x86_64',
],
- 'bench_type': [
+ bench_type: [
'deserial',
'micro',
'playback',
'recording',
'skandroidcodec',
'skcodec',
- 'tracing'
+ 'tracing',
],
- 'compiler': [
+ compiler: [
'Clang',
'EMCC',
'GCC',
],
- 'config': [
+ config: [
'8888',
'f16',
'gl',
@@ -41,43 +42,71 @@
};
describe('query-sk', () => {
- it('obeys key_order', () => {
- return window.customElements.whenDefined('query-sk').then(() => {
- container.innerHTML = `<query-sk></query-sk>`;
- const q = container.firstElementChild;
- q.paramset = paramset;
- assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
+ it('obeys key_order', () => window.customElements.whenDefined('query-sk').then(() => {
+ container.innerHTML = '<query-sk></query-sk>';
+ const q = container.firstElementChild;
+ q.paramset = paramset;
+ assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
- // Setting key_order will change the key order.
- q.key_order = ['config'];
- assert.deepEqual(['config', 'arch', 'bench_type', 'compiler'], $('select-sk div', q).map((ele) => ele.textContent));
+ // Setting key_order will change the key order.
+ q.key_order = ['config'];
+ assert.deepEqual(['config', 'arch', 'bench_type', 'compiler'], $('select-sk div', q).map((ele) => ele.textContent));
- // Setting key_order to empty will go back to alphabetical order.
- q.key_order = [];
- assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
- });
- });
+ // Setting key_order to empty will go back to alphabetical order.
+ q.key_order = [];
+ assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
+ }));
- it('obeys filter', () => {
- return window.customElements.whenDefined('query-sk').then(() => {
- container.innerHTML = `<query-sk></query-sk>`;
- const q = container.firstElementChild;
- q.paramset = paramset;
- assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
+ it('obeys filter', () => window.customElements.whenDefined('query-sk').then(() => {
+ container.innerHTML = '<query-sk></query-sk>';
+ const q = container.firstElementChild;
+ q.paramset = paramset;
+ assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
- // Setting the filter will change the keys displayed.
- const fast = q.querySelector('#fast');
- fast.value = 'cro'; // Only 'micro' in 'bench_type' should match.
- fast.dispatchEvent(new Event('input')); // Emulate user input.
+ // Setting the filter will change the keys displayed.
+ const fast = q.querySelector('#fast');
+ fast.value = 'cro'; // Only 'micro' in 'bench_type' should match.
+ fast.dispatchEvent(new Event('input')); // Emulate user input.
- // Only key should be bench_type.
- assert.deepEqual(['bench_type'], $('select-sk div', q).map((ele) => ele.textContent));
+ // Only key should be bench_type.
+ assert.deepEqual(['bench_type'], $('select-sk div', q).map((ele) => ele.textContent));
- // Clearing the filter will restore all options.
- fast.value = '';
- fast.dispatchEvent(new Event('input')); // Emulate user input.
+ // Clearing the filter will restore all options.
+ fast.value = '';
+ fast.dispatchEvent(new Event('input')); // Emulate user input.
- assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
- });
- });
+ assert.deepEqual(['arch', 'bench_type', 'compiler', 'config'], $('select-sk div', q).map((ele) => ele.textContent));
+ }));
+
+ it('only edits displayed values when filter is used.', () => window.customElements.whenDefined('query-sk').then(() => {
+ container.innerHTML = '<query-sk></query-sk>';
+ const q = container.firstElementChild;
+ q.paramset = paramset;
+
+ // Make a selection.
+ q.current_query = 'arch=x86';
+
+ // Setting the filter will change the keys displayed.
+ const fast = $$('#fast', q);
+ fast.value = '64'; // Only 'arm64' and 'x86_64' in 'arch' should match.
+ fast.dispatchEvent(new Event('input')); // Emulate user input.
+
+ // Only key should be arch.
+ assert.deepEqual(['arch'], $('select-sk div', q).map((ele) => ele.textContent));
+
+ // Click on 'arch'.
+ $$('select-sk', q).firstElementChild.click();
+
+ // Click on the value 'arm64' to add it to the query.
+ $$('multi-select-sk', q).firstElementChild.click();
+
+ // Confirm it gets added.
+ assert.deepEqual(toParamSet('arch=x86&arch=arm64'), toParamSet(q.current_query));
+
+ // Click on the value 'arm64' a second time to remove it from the query.
+ $$('multi-select-sk', q).firstElementChild.click();
+
+ // Confirm it gets removed.
+ assert.deepEqual(toParamSet('arch=x86'), toParamSet(q.current_query));
+ }));
});
