[infra-sk] Move the fast filter on query-sk.
I got a bug report that fast filter should work on both params and values, when
it already does that. So it appears that it isn't clear that is what filtering
does.
This CL moves the filter to below both params and values and adds placeholder
text that indicates what the control does.
Bug: skia:10327
Change-Id: I4c14e2a7e00b4db9ceced1df1db82b6bb26b4800
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/301842
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@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 46883eb..82d6ca1 100644
--- a/infra-sk/modules/query-sk/query-sk-demo.html
+++ b/infra-sk/modules/query-sk/query-sk-demo.html
@@ -17,6 +17,10 @@
<div class="body-sk darkmode">
<query-sk></query-sk>
</div>
+ <h2>With filtering</h2>
+ <div class="body-sk darkmode">
+ <query-sk id=activeFilter></query-sk>
+ </div>
<h2>Actions</h2>
<div>
<button id="swap">Swap Paramset</button>
diff --git a/infra-sk/modules/query-sk/query-sk-demo.ts b/infra-sk/modules/query-sk/query-sk-demo.ts
index cc75308..abbd83f 100644
--- a/infra-sk/modules/query-sk/query-sk-demo.ts
+++ b/infra-sk/modules/query-sk/query-sk-demo.ts
@@ -55,3 +55,10 @@
ele.paramset = [paramset, paramset2][n];
});
});
+
+// Set filtering on one of the demo elements.
+const activeFilter = document.querySelector<HTMLInputElement>(
+ '#activeFilter #fast'
+)!;
+activeFilter.value = 'one';
+activeFilter.dispatchEvent(new InputEvent('input'));
diff --git a/infra-sk/modules/query-sk/query-sk.scss b/infra-sk/modules/query-sk/query-sk.scss
index 374012d..22769a5 100644
--- a/infra-sk/modules/query-sk/query-sk.scss
+++ b/infra-sk/modules/query-sk/query-sk.scss
@@ -42,5 +42,21 @@
color: var(--on-surface);
background: var(--surface-1dp);
border: solid 1px var(--on-surface);
+ padding: 4px;
+ height: 20px;
+ }
+
+ .clear_filters {
+ border: none;
+ min-width: 0;
+ margin: 0 0 0 -20px;
+ background: none;
+ cursor: pointer;
+ height: 20px;
+ padding: 0;
+ }
+
+ .filtering {
+ margin-bottom: 8px;
}
}
diff --git a/infra-sk/modules/query-sk/query-sk.ts b/infra-sk/modules/query-sk/query-sk.ts
index 9401f06..aadab29 100644
--- a/infra-sk/modules/query-sk/query-sk.ts
+++ b/infra-sk/modules/query-sk/query-sk.ts
@@ -22,43 +22,80 @@
import { ParamSet, toParamSet, fromParamSet } from 'common-sk/modules/query';
import { ElementSk } from '../ElementSk';
import { SelectSk } from 'elements-sk/select-sk/select-sk';
-import { QueryValuesSk, QueryValuesSkQueryValuesChangedEventDetail } from '../query-values-sk/query-values-sk';
+import {
+ QueryValuesSk,
+ QueryValuesSkQueryValuesChangedEventDetail,
+} from '../query-values-sk/query-values-sk';
import '../query-values-sk';
import 'elements-sk/select-sk';
import 'elements-sk/styles/buttons';
+import { QuerySkPO } from './query-sk_po';
// The delay in ms before sending a delayed query-change event.
const DELAY_MS = 500;
export interface QuerySkQueryChangeEventDetail {
readonly q: string;
-};
+}
export class QuerySk extends ElementSk {
-
private static template = (ele: QuerySk) => html`
- <div>
- <label>Filter <input id=fast @input=${ele._fastFilter}></label>
- <button @click=${ele._clearFilter} class=clear_filters>Clear Filter</button>
+ <div class="filtering">
+ <input
+ id="fast"
+ @input=${ele._fastFilter}
+ placeholder="Filter Parameters and Values"
+ />
+ ${QuerySk.clearFilterButton(ele)}
</div>
- <div class=bottom>
- <div class=selection>
+ <div class="bottom">
+ <div class="selection">
<select-sk @selection-changed=${ele._keyChange}>
${QuerySk.keysTemplate(ele)}
</select-sk>
- <button @click=${ele._clear} class=clear_selections>Clear Selections</button>
+ <button @click=${ele._clear} class="clear_selections">
+ Clear Selections
+ </button>
</div>
- <query-values-sk id=values @query-values-changed=${ele._valuesChanged}
- ?hide_invert=${ele.hide_invert} ?hide_regex=${ele.hide_regex}></query-values-sk>
+ <query-values-sk
+ id="values"
+ @query-values-changed=${ele._valuesChanged}
+ ?hide_invert=${ele.hide_invert}
+ ?hide_regex=${ele.hide_regex}
+ ></query-values-sk>
</div>
`;
- private static keysTemplate = (ele: QuerySk) => ele._keys.map((k) => html`<div>${k}</div>`);
+ private static clearFilterButton(ele: QuerySk) {
+ if (!ele._filtering) {
+ return html``;
+ }
+ return html`
+ <button
+ @click=${ele._clearFilter}
+ class="clear_filters"
+ title="Clear filter"
+ >
+ ✗
+ </button>
+ `;
+ }
+
+ private static keysTemplate = (ele: QuerySk) =>
+ ele._keys.map(
+ (k) =>
+ html`
+ <div>${k}</div>
+ `
+ );
private _paramset: ParamSet = {};
private _originalParamset: ParamSet = {};
+ // True if there is text in the fitler input.
+ private _filtering: boolean = false;
+
// We keep the current_query as an object.
private _query: ParamSet = {};
@@ -91,7 +128,9 @@
this._fast = this.querySelector('#fast');
}
- private _valuesChanged(e: CustomEvent<QueryValuesSkQueryValuesChangedEventDetail>) {
+ private _valuesChanged(
+ e: CustomEvent<QueryValuesSkQueryValuesChangedEventDetail>
+ ) {
const key = this._keys[this._keySelect!.selection as number];
if (this._fast!.value.trim() !== '') {
// Things get complicated if the user has entered a filter. The user may
@@ -143,16 +182,23 @@
const prev_query = this.current_query;
this._rationalizeQuery();
if (prev_query !== this.current_query) {
- this.dispatchEvent(new CustomEvent<QuerySkQueryChangeEventDetail>('query-change', {
- detail: { q: this.current_query },
- bubbles: true,
- }));
- window.clearTimeout(this._delayedTimeout!);
- this._delayedTimeout = window.setTimeout(() => {
- this.dispatchEvent(new CustomEvent<QuerySkQueryChangeEventDetail>('query-change-delayed', {
+ this.dispatchEvent(
+ new CustomEvent<QuerySkQueryChangeEventDetail>('query-change', {
detail: { q: this.current_query },
bubbles: true,
- }));
+ })
+ );
+ window.clearTimeout(this._delayedTimeout!);
+ this._delayedTimeout = window.setTimeout(() => {
+ this.dispatchEvent(
+ new CustomEvent<QuerySkQueryChangeEventDetail>(
+ 'query-change-delayed',
+ {
+ detail: { q: this.current_query },
+ bubbles: true,
+ }
+ )
+ );
}, DELAY_MS);
}
}
@@ -169,10 +215,12 @@
delete this._query[key];
} else {
// Filter out invalid values.
- this._query[key] =
- this._query[key].filter(val => this._originalParamset[key].includes(val) ||
+ this._query[key] = this._query[key].filter(
+ (val) =>
+ this._originalParamset[key].includes(val) ||
val.startsWith('~') ||
- val.startsWith('!'));
+ val.startsWith('!')
+ );
}
});
@@ -192,7 +240,12 @@
}
private _fastFilter() {
- const filters = this._fast!.value.trim().toLowerCase().split(/\s+/);
+ const filterString = this._fast!.value.trim();
+ const filters = filterString.toLowerCase().split(/\s+/);
+
+ if (filterString) {
+ this._filtering = true;
+ }
// Create a closure that returns true if the given label matches the filter.
const matches = (s: string) => {
@@ -229,11 +282,15 @@
private _clearFilter() {
this._fast!.value = '';
this.paramset = this._originalParamset;
+ this._filtering = false;
this._queryChanged();
+ this._render();
}
/** @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.
@@ -251,7 +308,11 @@
this._render();
// Now re-select the current key if it still exists post-filtering.
- if (this._keySelect && prevSelectKey && this._keys.indexOf(prevSelectKey) !== -1) {
+ if (
+ this._keySelect &&
+ prevSelectKey &&
+ this._keys.indexOf(prevSelectKey) !== -1
+ ) {
this._keySelect.selection = this._keys.indexOf(prevSelectKey);
this._keyChange();
}
@@ -261,7 +322,9 @@
* The keys in the order they 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;
@@ -270,7 +333,9 @@
}
/** 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) {
@@ -282,7 +347,9 @@
}
/** 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) {
@@ -294,9 +361,13 @@
}
/** Mirrors the current_query attribute. */
- get current_query() { return this.getAttribute('current_query') || ''; }
+ get current_query() {
+ return this.getAttribute('current_query') || '';
+ }
- set current_query(val: string) { this.setAttribute('current_query', val); }
+ set current_query(val: string) {
+ this.setAttribute('current_query', val);
+ }
static get observedAttributes() {
return ['current_query', 'hide_invert', 'hide_regex'];
diff --git a/infra-sk/modules/query-sk/query-sk_puppeteer_test.ts b/infra-sk/modules/query-sk/query-sk_puppeteer_test.ts
index 5171bb9..516009d 100644
--- a/infra-sk/modules/query-sk/query-sk_puppeteer_test.ts
+++ b/infra-sk/modules/query-sk/query-sk_puppeteer_test.ts
@@ -12,12 +12,12 @@
beforeEach(async () => {
await testBed.page.goto(`${testBed.baseUrl}/query-sk.html`);
- await testBed.page.setViewport({ width: 600, height: 1100 });
+ await testBed.page.setViewport({ width: 600, height: 1400 });
});
it('should render the demo page', async () => {
// Smoke test.
- expect(await testBed.page.$$('query-sk')).to.have.length(3);
+ expect(await testBed.page.$$('query-sk')).to.have.length(4);
});
describe('screenshots', () => {
