From 9438f315e967f348c250229f064c556ca3a4ba8d Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 8 May 2023 00:59:44 -0400 Subject: [PATCH] (core) Save choice config on focus loss Summary: Changes to choices are now saved whenever focus leaves the editor. Test Plan: Browser tests. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D3879 --- app/client/ui2018/ColorSelect.ts | 50 +++++++----- app/client/widgets/ChoiceListEntry.ts | 106 ++++++++++++++++++-------- test/nbrowser/ChoiceList.ts | 39 +++++++--- 3 files changed, 137 insertions(+), 58 deletions(-) diff --git a/app/client/ui2018/ColorSelect.ts b/app/client/ui2018/ColorSelect.ts index 0baa2382..4a73438c 100644 --- a/app/client/ui2018/ColorSelect.ts +++ b/app/client/ui2018/ColorSelect.ts @@ -6,7 +6,7 @@ import {IconName} from 'app/client/ui2018/IconList'; import {icon} from 'app/client/ui2018/icons'; import {cssSelectBtn} from 'app/client/ui2018/select'; import {isValidHex} from 'app/common/gutil'; -import {BindableValue, Computed, Disposable, dom, Observable, onKeyDown, styled} from 'grainjs'; +import {BindableValue, Computed, Disposable, dom, DomElementArg, Observable, onKeyDown, styled} from 'grainjs'; import {defaultMenuOptions, IOpenController, setPopupToCreateDom} from 'popweasel'; import {makeT} from 'app/client/lib/localization'; @@ -87,16 +87,24 @@ export function colorSelect( const domCreator = (ctl: IOpenController) => { onOpen?.(); - return buildColorPicker(ctl, styleOptions, onSave, onRevert); + return buildColorPicker(ctl, {styleOptions, onSave, onRevert}); }; setPopupToCreateDom(selectBtn, domCreator, {...defaultMenuOptions, placement: 'bottom-end'}); return selectBtn; } -export function colorButton( - styleOptions: StyleOptions, - onSave: () => Promise): Element { +export interface ColorButtonOptions { + styleOptions: StyleOptions; + colorPickerDomArgs?: DomElementArg[]; + onSave(): Promise; + onRevert?(): void; + onClose?(): void; +} + +export function colorButton(options: ColorButtonOptions): Element { + const { colorPickerDomArgs, ...colorPickerOptions } = options; + const { styleOptions } = colorPickerOptions; const { textColor, fillColor } = styleOptions; const iconBtn = cssIconBtn( 'T', @@ -109,24 +117,29 @@ export function colorButton( testId('color-button'), ); - const domCreator = (ctl: IOpenController) => buildColorPicker(ctl, styleOptions, onSave); + const domCreator = (ctl: IOpenController) => + buildColorPicker(ctl, colorPickerOptions, colorPickerDomArgs); setPopupToCreateDom(iconBtn, domCreator, { ...defaultMenuOptions, placement: 'bottom-end' }); return iconBtn; } -function buildColorPicker(ctl: IOpenController, - { - textColor, - fillColor, - fontBold, - fontUnderline, - fontItalic, - fontStrikethrough - }: StyleOptions, - onSave: () => Promise, - onRevert?: () => void, +interface ColorPickerOptions { + styleOptions: StyleOptions; + onSave(): Promise; + onRevert?(): void; + onClose?(): void; +} + +function buildColorPicker( + ctl: IOpenController, + options: ColorPickerOptions, + ...domArgs: DomElementArg[] ): Element { + const {styleOptions, onSave, onRevert, onClose} = options; + const { + textColor, fillColor, fontBold, fontUnderline, fontItalic, fontStrikethrough + } = styleOptions; const textColorModel = ColorModel.create(null, textColor.color); const fillColorModel = ColorModel.create(null, fillColor.color); const fontBoldModel = BooleanModel.create(null, fontBold); @@ -161,6 +174,7 @@ function buildColorPicker(ctl: IOpenController, } models.forEach(m => m.dispose()); notChanged.dispose(); + onClose?.(); }); return cssContainer( @@ -202,6 +216,8 @@ function buildColorPicker(ctl: IOpenController, // Set focus when `focusout` is bubbling from a children element. This is to allow to receive // keyboard event again after user interacted with the hex box text input. dom.on('focusout', (ev, elem) => (ev.target !== elem) && elem.focus()), + + ...domArgs, ); } diff --git a/app/client/widgets/ChoiceListEntry.ts b/app/client/widgets/ChoiceListEntry.ts index 37bbf421..e2613030 100644 --- a/app/client/widgets/ChoiceListEntry.ts +++ b/app/client/widgets/ChoiceListEntry.ts @@ -91,6 +91,9 @@ export class ChoiceListEntry extends Disposable { private _isEditing: Observable = Observable.create(this, false); private _tokenFieldHolder: Holder> = Holder.create(this); + private _editorContainer: HTMLElement | null = null; + private _editorSaveButtons: HTMLElement | null = null; + constructor( private _values: Observable, private _choiceOptionsByName: Observable, @@ -105,6 +108,12 @@ export class ChoiceListEntry extends Disposable { this.autoDispose(this._values.addListener(() => { this._cancel(); })); + + this.onDispose(() => { + if (!this._isEditing.get()) { return; } + + this._save(); + }); } // Arg maxRows indicates the number of rows to display when the editor is inactive. @@ -137,14 +146,42 @@ export class ChoiceListEntry extends Disposable { }); return cssVerticalFlex( - cssListBox( + this._editorContainer = cssListBox( + {tabIndex: '-1'}, elem => { tokenField.attach(elem); this._focusOnOpen(tokenField.getTextInput()); }, + dom.on('focusout', (ev) => { + const hasActiveElement = ( + element: Element | null, + activeElement = document.activeElement + ) => { + return element?.contains(activeElement); + }; + + // Save and close the editor when it loses focus. + setTimeout(() => { + // The editor may have already been closed via keyboard shortcut. + if (!this._isEditing.get()) { return; } + + if ( + // Don't close if focus hasn't left the editor. + hasActiveElement(this._editorContainer) || + // Or if the token color picker has focus. + hasActiveElement(document.querySelector('.token-color-picker')) || + // Or if Save or Cancel was clicked. + hasActiveElement(this._editorSaveButtons, ev.relatedTarget as Element | null) + ) { + return; + } + + this._save(); + }, 0); + }), testId('choice-list-entry') ), - cssButtonRow( + this._editorSaveButtons = cssButtonRow( primaryButton('Save', dom.on('click', () => this._save() ), testId('choice-list-entry-save') @@ -154,8 +191,8 @@ export class ChoiceListEntry extends Disposable { testId('choice-list-entry-cancel') ) ), - dom.onKeyDown({Escape$: () => this._cancel()}), - dom.onKeyDown({Enter$: () => this._save()}), + dom.onKeyDown({Escape: () => this._cancel()}), + dom.onKeyDown({Enter: () => this._save()}), ); } else { const holder = new MultiHolder(); @@ -310,34 +347,41 @@ export class ChoiceListEntry extends Disposable { dom.autoDispose(fillColorObs), dom.autoDispose(textColorObs), dom.autoDispose(choiceText), - colorButton({ - textColor: new ColorOption({color: textColorObs, defaultColor: '#000000'}), - fillColor: new ColorOption( - {color: fillColorObs, allowsNone: true, noneText: 'none', defaultColor: '#FFFFFF'}), - fontBold: fontBoldObs, - fontItalic: fontItalicObs, - fontUnderline: fontUnderlineObs, - fontStrikethrough: fontStrikethroughObs + colorButton( + { + styleOptions: { + textColor: new ColorOption({color: textColorObs, defaultColor: '#000000'}), + fillColor: new ColorOption( + {color: fillColorObs, allowsNone: true, noneText: 'none', defaultColor: '#FFFFFF'}), + fontBold: fontBoldObs, + fontItalic: fontItalicObs, + fontUnderline: fontUnderlineObs, + fontStrikethrough: fontStrikethroughObs + }, + onSave: async () => { + const tokenField = this._tokenFieldHolder.get(); + if (!tokenField) { return; } + + const fillColor = fillColorObs.get(); + const textColor = textColorObs.get(); + const fontBold = fontBoldObs.get(); + const fontItalic = fontItalicObs.get(); + const fontUnderline = fontUnderlineObs.get(); + const fontStrikethrough = fontStrikethroughObs.get(); + tokenField.replaceToken(token.label, ChoiceItem.from(token).changeStyle({ + fillColor, + textColor, + fontBold, + fontItalic, + fontUnderline, + fontStrikethrough, + })); + }, + onClose: () => this._editorContainer?.focus(), + colorPickerDomArgs: [ + dom.cls('token-color-picker'), + ], }, - async () => { - const tokenField = this._tokenFieldHolder.get(); - if (!tokenField) { return; } - - const fillColor = fillColorObs.get(); - const textColor = textColorObs.get(); - const fontBold = fontBoldObs.get(); - const fontItalic = fontItalicObs.get(); - const fontUnderline = fontUnderlineObs.get(); - const fontStrikethrough = fontStrikethroughObs.get(); - tokenField.replaceToken(token.label, ChoiceItem.from(token).changeStyle({ - fillColor, - textColor, - fontBold, - fontItalic, - fontUnderline, - fontStrikethrough, - })); - } ), editableLabel(choiceText, { save: rename, diff --git a/test/nbrowser/ChoiceList.ts b/test/nbrowser/ChoiceList.ts index 2ecfd73e..6108e2e2 100644 --- a/test/nbrowser/ChoiceList.ts +++ b/test/nbrowser/ChoiceList.ts @@ -666,18 +666,26 @@ describe('ChoiceList', function() { strikethrough, underline, bold} ] ); + }); - // Open the editor again to make another change. - await driver.find('.test-choice-list-entry').click(); - await gu.waitAppFocus(false); - - // Delete 'Apricot', then cancel the change by pressing Escape. - await gu.sendKeys(Key.BACK_SPACE); - assert.deepEqual(await getEditModeChoiceLabels(), ['Green', 'Blue', 'Black']); - await gu.sendKeys(Key.ESCAPE); + it('should discard changes on cancel', async function() { + for (const method of ['button', 'shortcut']) { + // Open the editor. + await driver.find('.test-choice-list-entry').click(); + await gu.waitAppFocus(false); + + // Delete 'Apricot', then cancel the change. + await gu.sendKeys(Key.BACK_SPACE); + assert.deepEqual(await getEditModeChoiceLabels(), ['Green', 'Blue', 'Black']); + if (method === 'button') { + await driver.find('.test-choice-list-entry-cancel').click(); + } else { + await gu.sendKeys(Key.ESCAPE); + } - // Check that 'Apricot' is still there and the change wasn't saved. - assert.deepEqual(await getChoiceLabels(), ['Green', 'Blue', 'Black', 'Apricot']); + // Check that 'Apricot' is still there and the change wasn't saved. + assert.deepEqual(await getChoiceLabels(), ['Green', 'Blue', 'Black', 'Apricot']); + } }); it('should support undo/redo shortcuts in the choice config editor', async function() { @@ -754,6 +762,17 @@ describe('ChoiceList', function() { // workflow above would copy all the choice data as well, and use it for pasting in the editor. }); + it('should save and close the choice config editor on focusout', async function() { + // Click outside of the editor. + await driver.find('.test-gristdoc').click(); + await gu.waitAppFocus(true); + + // Check that the changes were saved. + assert.deepEqual(await getChoiceLabels(), ['Choice 1', 'Choice 2', 'Choice 3']); + + await gu.undo(); + }); + it('should add a new element on a fresh ChoiceList column', async function() { await gu.addColumn("ChoiceList"); await gu.setType(gu.exactMatch("Choice List"));