(core) Search improvemement

Summary:
This diff implements the search improvement that are mentioned here https://grist.quip.com/j1biAmfGkbzV/Search-Improvements.

CAVEATS:

I've noticed a bit of a usability caveats: the tooltips overlap with the new `search all pages` checkbox, which requires user to move cursor away for a bit in order to be able to click the button.

{F28224}

I've experimented locally with tooltips showing on both sides of the arrows, but it overlaps with the cross icon so could also be an issue. I couldn't think of any clear simple alternative, probably not too big of an issue anyway.

Test Plan: Added new test.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2818
This commit is contained in:
Cyprien P 2021-05-19 09:07:51 +02:00
parent 79497a7e79
commit db91d31416
2 changed files with 107 additions and 24 deletions

View File

@ -17,7 +17,9 @@ export interface SearchModel {
value: Observable<string>; // string in the search input value: Observable<string>; // string in the search input
isOpen: Observable<boolean>; // indicates whether the search bar is expanded to show the input isOpen: Observable<boolean>; // indicates whether the search bar is expanded to show the input
noMatch: Observable<boolean>; // indicates if there are no search matches noMatch: Observable<boolean>; // indicates if there are no search matches
isEmpty: Observable<boolean>; // indicates whether the value is empty
isRunning: Observable<boolean>; // indicates that matching is in progress isRunning: Observable<boolean>; // indicates that matching is in progress
multiPage: Observable<boolean>; // if true will search across all pages
findNext(): Promise<void>; // find next match findNext(): Promise<void>; // find next match
findPrev(): Promise<void>; // find previous match findPrev(): Promise<void>; // find previous match
@ -69,6 +71,8 @@ export class SearchModelImpl extends Disposable implements SearchModel {
public readonly isOpen = Observable.create(this, false); public readonly isOpen = Observable.create(this, false);
public readonly isRunning = Observable.create(this, false); public readonly isRunning = Observable.create(this, false);
public readonly noMatch = Observable.create(this, true); public readonly noMatch = Observable.create(this, true);
public readonly isEmpty = Observable.create(this, true);
public readonly multiPage = Observable.create(this, false);
private _searchRegexp: RegExp; private _searchRegexp: RegExp;
private _tabStepper = new Stepper<any>(); private _tabStepper = new Stepper<any>();
private _sectionStepper = new Stepper<ViewSectionRec>(); private _sectionStepper = new Stepper<ViewSectionRec>();
@ -78,6 +82,7 @@ export class SearchModelImpl extends Disposable implements SearchModel {
private _fieldFormatters: BaseFormatter[]; private _fieldFormatters: BaseFormatter[];
private _startPosition: SearchPosition; private _startPosition: SearchPosition;
private _tabsSwitched: number = 0; private _tabsSwitched: number = 0;
private _isRestartNeeded: boolean = false;
constructor(private _gristDoc: GristDoc) { constructor(private _gristDoc: GristDoc) {
super(); super();
@ -85,10 +90,21 @@ export class SearchModelImpl extends Disposable implements SearchModel {
// Listen to input value changes (debounced) to activate searching. // Listen to input value changes (debounced) to activate searching.
const findFirst = debounce((_value: string) => this._findFirst(_value), 100); const findFirst = debounce((_value: string) => this._findFirst(_value), 100);
this.autoDispose(this.value.addListener(v => { void findFirst(v); })); this.autoDispose(this.value.addListener(v => { void findFirst(v); }));
// Set this.noMatch to false when multiPage gets turned ON.
this.autoDispose(this.multiPage.addListener(v => { if (v) { this.noMatch.set(false); }}));
// Schedule a search restart when user changes pages (otherwise search would resume from the
// previous page that is not shown anymore). Also revert noMatch flag when in single page mode.
this.autoDispose(this._gristDoc.activeViewId.addListener(() => {
if (!this.multiPage.get()) { this.noMatch.set(false); }
this._isRestartNeeded = true;
}));
} }
public async findNext() { public async findNext() {
if (this.noMatch.get()) { return; } if (this.noMatch.get()) { return; }
if (this._isRestartNeeded) { return this._findFirst(this.value.get()); }
this._startPosition = this._getCurrentPosition(); this._startPosition = this._getCurrentPosition();
await this._nextField(1); await this._nextField(1);
return this._matchNext(1); return this._matchNext(1);
@ -96,12 +112,15 @@ export class SearchModelImpl extends Disposable implements SearchModel {
public async findPrev() { public async findPrev() {
if (this.noMatch.get()) { return; } if (this.noMatch.get()) { return; }
if (this._isRestartNeeded) { return this._findFirst(this.value.get()); }
this._startPosition = this._getCurrentPosition(); this._startPosition = this._getCurrentPosition();
await this._nextField(-1); await this._nextField(-1);
return this._matchNext(-1); return this._matchNext(-1);
} }
private _findFirst(value: string) { private _findFirst(value: string) {
this._isRestartNeeded = false;
this.isEmpty.set(!value);
if (!value) { this.noMatch.set(true); return; } if (!value) { this.noMatch.set(true); return; }
this._searchRegexp = makeRegexp(value); this._searchRegexp = makeRegexp(value);
const tabs: any[] = this._gristDoc.docModel.allDocPages.peek(); const tabs: any[] = this._gristDoc.docModel.allDocPages.peek();
@ -155,7 +174,7 @@ export class SearchModelImpl extends Disposable implements SearchModel {
if (p) { await p; } if (p) { await p; }
// Detect when we get back to the start position; this is where we break on no match. // Detect when we get back to the start position; this is where we break on no match.
if (this._isCurrentPosition(this._startPosition)) { if (this._isCurrentPosition(this._startPosition) && !this._matches()) {
console.log("SearchBar: reached start position without finding anything"); console.log("SearchBar: reached start position without finding anything");
this.noMatch.set(true); this.noMatch.set(true);
return; return;
@ -256,6 +275,7 @@ export class SearchModelImpl extends Disposable implements SearchModel {
} }
private async _nextTab(step: number) { private async _nextTab(step: number) {
if (!this.multiPage.get()) { return; }
await this._tabStepper.next(step, () => undefined); await this._tabStepper.next(step, () => undefined);
this._tabsSwitched++; this._tabsSwitched++;
// console.log("nextTab", this._tabStepper.index); // console.log("nextTab", this._tabStepper.index);
@ -290,7 +310,7 @@ export class SearchModelImpl extends Disposable implements SearchModel {
const section = this._sectionStepper.value; const section = this._sectionStepper.value;
if (!section.viewInstance.peek()) { if (!section.viewInstance.peek()) {
const view = this._tabStepper.value.view.peek(); const view = this._tabStepper.value.view.peek();
await this._gristDoc.openDocPage(view.getRowId()); await this._openDocPage(view.getRowId());
console.log("SearchBar: loading view %s section %s", view.getRowId(), section.getRowId()); console.log("SearchBar: loading view %s section %s", view.getRowId(), section.getRowId());
const viewInstance: any = await waitObs(section.viewInstance); const viewInstance: any = await waitObs(section.viewInstance);
await viewInstance.getLoadingDonePromise(); await viewInstance.getLoadingDonePromise();
@ -329,6 +349,12 @@ export class SearchModelImpl extends Disposable implements SearchModel {
setTimeout(() => cursor.classList.remove('search-match'), 20); setTimeout(() => cursor.classList.remove('search-match'), 20);
} }
} }
// Opens doc page without triggering a restart.
private async _openDocPage(viewId: number) {
await this._gristDoc.openDocPage(viewId);
this._isRestartNeeded = false;
}
} }
function makeRegexp(value: string) { function makeRegexp(value: string) {

View File

@ -5,8 +5,10 @@
import { createGroup } from 'app/client/components/commands'; import { createGroup } from 'app/client/components/commands';
import { reportError } from 'app/client/models/AppModel'; import { reportError } from 'app/client/models/AppModel';
import { SearchModel } from 'app/client/models/SearchModel'; import { SearchModel } from 'app/client/models/SearchModel';
import { hoverTooltip, IHoverTipOptions } from 'app/client/ui/tooltips';
import { cssHoverCircle, cssTopBarBtn } from 'app/client/ui/TopBarCss'; import { cssHoverCircle, cssTopBarBtn } from 'app/client/ui/TopBarCss';
import { colors, mediaSmall } from 'app/client/ui2018/cssVars'; import { labeledSquareCheckbox } from 'app/client/ui2018/checkbox';
import { colors, mediaSmall, vars } from 'app/client/ui2018/cssVars';
import { icon } from 'app/client/ui2018/icons'; import { icon } from 'app/client/ui2018/icons';
import { dom, input, styled } from 'grainjs'; import { dom, input, styled } from 'grainjs';
import { noTestId, TestId } from 'grainjs'; import { noTestId, TestId } from 'grainjs';
@ -27,6 +29,7 @@ const searchWrapper = styled('div', `
height: 100%; height: 100%;
max-height: 50px; max-height: 50px;
transition: width 0.4s; transition: width 0.4s;
position: relative;
&-expand { &-expand {
width: 100% !important; width: 100% !important;
border: 1px solid grey; border: 1px solid grey;
@ -71,13 +74,16 @@ const cssArrowBtn = styled('div', `
font-size: 14px; font-size: 14px;
padding: 3px; padding: 3px;
cursor: pointer; cursor: pointer;
margin: 2px; margin: 2px 4px;
visibility: hidden; visibility: hidden;
width: 24px;
&.disabled { height: 24px;
color: ${colors.darkGrey}; background-color: ${colors.mediumGrey};
cursor: default; --icon-color: ${colors.slate};
} border-radius: 3px;
text-align: center;
display: flex;
align-items: center;
.${searchWrapper.className}-expand & { .${searchWrapper.className}-expand & {
visibility: visible; visibility: visible;
@ -86,9 +92,38 @@ const cssArrowBtn = styled('div', `
const cssCloseBtn = styled(icon, ` const cssCloseBtn = styled(icon, `
cursor: pointer; cursor: pointer;
background-color: ${colors.lightGreen};
margin-left: 4px;
flex-shrink: 0;
`); `);
const cssLabel = styled('span', `
font-size: ${vars.smallFontSize};
color: ${colors.slate};
white-space: nowrap;
margin-right: 12px;
`);
const cssOptions = styled('div', `
position: absolute;
right: 0;
top: 48px;
z-index: 1;
`);
const cssShortcut = styled('span', `
color: ${colors.slate};
`);
const searchArrowBtnTooltipOptions: IHoverTipOptions = {
key: 'searchArrowBtnTooltip',
openDelay: 500,
closeDelay: 100,
};
export function searchBar(model: SearchModel, testId: TestId = noTestId) { export function searchBar(model: SearchModel, testId: TestId = noTestId) {
let keepExpanded = false;
const commandGroup = createGroup({ const commandGroup = createGroup({
find: () => { inputElem.focus(); inputElem.select(); }, find: () => { inputElem.focus(); inputElem.select(); },
// On Mac, Firefox has a default behaviour witch causes to close the search bar on Cmd+g and // On Mac, Firefox has a default behaviour witch causes to close the search bar on Cmd+g and
@ -100,9 +135,13 @@ export function searchBar(model: SearchModel, testId: TestId = noTestId) {
const toggleMenu = debounce((_value?: boolean) => { const toggleMenu = debounce((_value?: boolean) => {
model.isOpen.set(_value === undefined ? !model.isOpen.get() : _value); model.isOpen.set(_value === undefined ? !model.isOpen.get() : _value);
}, 100); }, 100);
const inputElem = searchInput(model.value, {onInput: true}, const inputElem: HTMLInputElement = searchInput(model.value, {onInput: true},
{type: 'text', placeholder: 'Search in document'}, {type: 'text', placeholder: 'Search in document'},
dom.on('blur', () => toggleMenu(false)), dom.on('blur', () => (
keepExpanded ?
setTimeout(() => inputElem.focus(), 0) :
toggleMenu(false)
)),
dom.onKeyDown({ dom.onKeyDown({
Enter: () => model.findNext(), Enter: () => model.findNext(),
Escape: () => toggleMenu(false), Escape: () => toggleMenu(false),
@ -131,21 +170,39 @@ export function searchBar(model: SearchModel, testId: TestId = noTestId) {
expandedSearch( expandedSearch(
testId('input'), testId('input'),
inputElem, inputElem,
cssArrowBtn('\u2329', dom.domComputed((use) => {
testId('prev'), const noMatch = use(model.noMatch);
// Prevent focus from being stolen from the input const isEmpty = use(model.isEmpty);
dom.on('mousedown', (event) => event.preventDefault()), if (isEmpty) { return null; }
dom.on('click', () => model.findPrev()), if (noMatch) { return cssLabel("No results"); }
dom.cls('disabled', model.noMatch)), return [
cssArrowBtn('\u232A', cssArrowBtn(
icon('Dropdown'),
testId('next'), testId('next'),
// Prevent focus from being stolen from the input // Prevent focus from being stolen from the input
dom.on('mousedown', (event) => event.preventDefault()), dom.on('mousedown', (event) => event.preventDefault()),
dom.on('click', () => model.findNext()), dom.on('click', () => model.findNext()),
dom.cls('disabled', model.noMatch)), hoverTooltip(() => ['Find Next ', cssShortcut('(Enter, ⌘G)')], searchArrowBtnTooltipOptions),
),
cssArrowBtn(
icon('DropdownUp'),
testId('prev'),
// Prevent focus from being stolen from the input
dom.on('mousedown', (event) => event.preventDefault()),
dom.on('click', () => model.findPrev()),
hoverTooltip(() => ['Find Previous ', cssShortcut('(⌘⇧G)')], searchArrowBtnTooltipOptions),
)
];
}),
cssCloseBtn('CrossSmall', cssCloseBtn('CrossSmall',
testId('close'), testId('close'),
dom.on('click', () => toggleMenu(false))) dom.on('click', () => toggleMenu(false))),
cssOptions(
labeledSquareCheckbox(model.multiPage, 'Search all pages'),
dom.on('mouseenter', () => keepExpanded = true),
dom.on('mouseleave', () => keepExpanded = false),
testId('option-all-pages'),
),
) )
); );
} }