(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

@@ -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(
+export interface ColorButtonOptions {
-  styleOptions: StyleOptions,
+  styleOptions: StyleOptions;
-  onSave: () => Promise<void>): Element {
+  colorPickerDomArgs?: DomElementArg[];
+  onSave(): Promise<void>;
+  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,
+interface ColorPickerOptions {
-  {
+  styleOptions: StyleOptions;
-    textColor,
+  onSave(): Promise<void>;
-    fillColor,
+  onRevert?(): void;
-    fontBold,
+  onClose?(): void;
-    fontUnderline,
+}
-    fontItalic,
+
-    fontStrikethrough
+function buildColorPicker(
-  }: StyleOptions,
+  ctl: IOpenController,
-  onSave: () => Promise<void>,
+  options: ColorPickerOptions,
-  onRevert?: () => void,
+  ...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,
   );
 }
 

app/client/widgets/ChoiceListEntry.ts

@@ -91,6 +91,9 @@ export class ChoiceListEntry extends Disposable {
   private _isEditing: Observable<boolean> = Observable.create(this, false);
   private _tokenFieldHolder: Holder<TokenField<ChoiceItem>> = Holder.create(this);
 
+  private _editorContainer: HTMLElement | null = null;
+  private _editorSaveButtons: HTMLElement | null = null;
+
   constructor(
     private _values: Observable<string[]>,
     private _choiceOptionsByName: Observable<ChoiceOptionsByName>,
@@ -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({Escape: () => this._cancel()}),
-          dom.onKeyDown({Enter$: () => this._save()}),
+          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({
+      colorButton(
-          textColor: new ColorOption({color: textColorObs, defaultColor: '#000000'}),
+        {
-          fillColor: new ColorOption(
+          styleOptions: {
-            {color: fillColorObs, allowsNone: true, noneText: 'none', defaultColor: '#FFFFFF'}),
+            textColor: new ColorOption({color: textColorObs, defaultColor: '#000000'}),
-          fontBold: fontBoldObs,
+            fillColor: new ColorOption(
-          fontItalic: fontItalicObs,
+              {color: fillColorObs, allowsNone: true, noneText: 'none', defaultColor: '#FFFFFF'}),
-          fontUnderline: fontUnderlineObs,
+            fontBold: fontBoldObs,
-          fontStrikethrough: fontStrikethroughObs
+            fontItalic: fontItalicObs,
-        },
+            fontUnderline: fontUnderlineObs,
-        async () => {
+            fontStrikethrough: fontStrikethroughObs
-          const tokenField = this._tokenFieldHolder.get();
+          },
-          if (!tokenField) { return; }
+          onSave: async () => {
+            const tokenField = this._tokenFieldHolder.get();
+            if (!tokenField) { return; }
 
-          const fillColor = fillColorObs.get();
+            const fillColor = fillColorObs.get();
-          const textColor = textColorObs.get();
+            const textColor = textColorObs.get();
-          const fontBold = fontBoldObs.get();
+            const fontBold = fontBoldObs.get();
-          const fontItalic = fontItalicObs.get();
+            const fontItalic = fontItalicObs.get();
-          const fontUnderline = fontUnderlineObs.get();
+            const fontUnderline = fontUnderlineObs.get();
-          const fontStrikethrough = fontStrikethroughObs.get();
+            const fontStrikethrough = fontStrikethroughObs.get();
-          tokenField.replaceToken(token.label, ChoiceItem.from(token).changeStyle({
+            tokenField.replaceToken(token.label, ChoiceItem.from(token).changeStyle({
-            fillColor,
+              fillColor,
-            textColor,
+              textColor,
-            fontBold,
+              fontBold,
-            fontItalic,
+              fontItalic,
-            fontUnderline,
+              fontUnderline,
-            fontStrikethrough,
+              fontStrikethrough,
-          }));
+            }));
-        }
+          },
+          onClose: () => this._editorContainer?.focus(),
+          colorPickerDomArgs: [
+            dom.cls('token-color-picker'),
+          ],
+        },
       ),
       editableLabel(choiceText, {
         save: rename,

test/nbrowser/ChoiceList.ts

@@ -666,18 +666,26 @@ describe('ChoiceList', function() {
          strikethrough, underline, bold}
       ]
     );
+  });
 
-    // Open the editor again to make another change.
+  it('should discard changes on cancel', async function() {
-    await driver.find('.test-choice-list-entry').click();
+    for (const method of ['button', 'shortcut']) {
-    await gu.waitAppFocus(false);
+      // Open the editor.
+      await driver.find('.test-choice-list-entry').click();
+      await gu.waitAppFocus(false);
 
-    // Delete 'Apricot', then cancel the change by pressing Escape.
+      // Delete 'Apricot', then cancel the change.
-    await gu.sendKeys(Key.BACK_SPACE);
+      await gu.sendKeys(Key.BACK_SPACE);
-    assert.deepEqual(await getEditModeChoiceLabels(), ['Green', 'Blue', 'Black']);
+      assert.deepEqual(await getEditModeChoiceLabels(), ['Green', 'Blue', 'Black']);
-    await gu.sendKeys(Key.ESCAPE);
+      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.
+      // Check that 'Apricot' is still there and the change wasn't saved.
-    assert.deepEqual(await getChoiceLabels(), ['Green', 'Blue', 'Black', 'Apricot']);
+      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"));