From 063df752047efd0d7e3f025be9abf31ded36d844 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Sat, 13 Jul 2024 01:27:54 -0400 Subject: [PATCH] (core) Forms improvements: mouse selection in firefox, focus, and styling Summary: - Remove unused Form file (Label.ts) - Fix Firefox-specific bug in Forms, where mouse selection wasn't working in textarea. - Focus and set cursor in textarea on click. - Save on blur but only when focus stays within the Grist app, as for editing cells. - Make paragraph margins of rendered form match those in the form editor. Test Plan: Tested manually on Firefox and Chrome; relying on existing tests that nothing broke. Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D4281 --- app/client/components/Forms/Editor.ts | 34 ++++------ app/client/components/Forms/Label.ts | 85 ------------------------ app/client/components/Forms/Paragraph.ts | 28 ++++---- app/client/components/Forms/elements.ts | 1 - app/client/components/Forms/styles.ts | 15 +++-- app/client/ui/FormPage.ts | 2 +- 6 files changed, 40 insertions(+), 125 deletions(-) delete mode 100644 app/client/components/Forms/Label.ts diff --git a/app/client/components/Forms/Editor.ts b/app/client/components/Forms/Editor.ts index 9170995f..e59733d5 100644 --- a/app/client/components/Forms/Editor.ts +++ b/app/client/components/Forms/Editor.ts @@ -22,10 +22,6 @@ interface Props { * Actual element to put into the editor. This is the main content of the editor. */ content: DomContents, - /** - * Click handler. If not provided, then clicking on the editor will select it. - */ - click?: (ev: MouseEvent, box: BoxModel) => void, /** * Whether to show the remove button. Defaults to true. */ @@ -75,22 +71,6 @@ export function buildEditor(props: Props, ...args: IDomArgs) { style.cssRemoveButton.cls('-right', props.removePosition === 'right'), ); - const onClick = (ev: MouseEvent) => { - // Only if the click was in this element. - const target = ev.target as HTMLElement; - if (!target.closest) { return; } - // Make sure that the closest editor is this one. - const closest = target.closest(`.${style.cssFieldEditor.className}`); - if (closest !== element) { return; } - - ev.stopPropagation(); - ev.preventDefault(); - props.click?.(ev, props.box); - - // Mark this box as selected. - box.view.selectedBox.set(box); - }; - const dragAbove = Observable.create(owner, false); const dragBelow = Observable.create(owner, false); const dragging = Observable.create(owner, false); @@ -111,7 +91,10 @@ export function buildEditor(props: Props, ...args: IDomArgs) { testId('field-editor-selected', box.selected), // Select on click. - dom.on('click', onClick), + dom.on('click', (ev) => { + stopEvent(ev); + box.view.selectedBox.set(box); + }), // Attach context menu. buildMenu({ @@ -122,6 +105,15 @@ export function buildEditor(props: Props, ...args: IDomArgs) { // And now drag and drop support. {draggable: "true"}, + // In Firefox, 'draggable' interferes with mouse selection in child input elements. Workaround + // is to turn off 'draggable' temporarily (see https://stackoverflow.com/q/21680363/328565). + dom.on('mousedown', (ev, elem) => { + const isInput = ["INPUT", "TEXTAREA"].includes((ev.target as Element)?.tagName); + // Turn off 'draggable' for inputs only, to support selection there; keep it on elsewhere. + elem.draggable = !isInput; + }), + dom.on('mouseup', (ev, elem) => { elem.draggable = true; }), + // When started, we just put the box into the dataTransfer as a plain text. // TODO: this might be very sofisticated in the future. dom.on('dragstart', (ev) => { diff --git a/app/client/components/Forms/Label.ts b/app/client/components/Forms/Label.ts deleted file mode 100644 index 3233346c..00000000 --- a/app/client/components/Forms/Label.ts +++ /dev/null @@ -1,85 +0,0 @@ -import * as css from './styles'; -import {buildEditor} from 'app/client/components/Forms/Editor'; -import {BoxModel} from 'app/client/components/Forms/Model'; -import {stopEvent} from 'app/client/lib/domUtils'; -import {not} from 'app/common/gutil'; -import {Computed, dom, Observable} from 'grainjs'; - -export class LabelModel extends BoxModel { - public edit = Observable.create(this, false); - - protected defaultValue = ''; - - public render(): HTMLElement { - let element: HTMLTextAreaElement; - const text = this.prop('text', this.defaultValue) as Observable; - const cssClass = this.prop('cssClass', '') as Observable; - const editableText = Observable.create(this, text.get() || ''); - const overlay = Computed.create(this, use => !use(this.edit)); - - this.autoDispose(text.addListener((v) => editableText.set(v || ''))); - - const save = (ok: boolean) => { - if (ok) { - text.set(editableText.get()); - void this.parent?.save().catch(reportError); - } else { - editableText.set(text.get() || ''); - } - }; - - const mode = (edit: boolean) => { - if (this.isDisposed() || this.edit.isDisposed()) { return; } - if (this.edit.get() === edit) { return; } - this.edit.set(edit); - }; - - return buildEditor( - { - box: this, - editMode: this.edit, - overlay, - click: (ev) => { - stopEvent(ev); - // If selected, then edit. - if (!this.selected.get()) { return; } - if (document.activeElement === element) { return; } - editableText.set(text.get() || ''); - this.edit.set(true); - setTimeout(() => { - element.focus(); - element.select(); - }, 10); - }, - content: element = css.cssEditableLabel( - editableText, - {onInput: true, autoGrow: true}, - {placeholder: `Empty label`}, - dom.on('click', ev => { - stopEvent(ev); - }), - // Styles saved (for titles and such) - css.cssEditableLabel.cls(use => `-${use(cssClass)}`), - // Disable editing if not in edit mode. - dom.boolAttr('readonly', not(this.edit)), - // Pass edit to css. - css.cssEditableLabel.cls('-edit', this.edit), - // Attach default save controls (Enter, Esc) and so on. - css.saveControls(this.edit, save), - // Turn off resizable for textarea. - dom.style('resize', 'none'), - ), - }, - dom.onKeyDown({Enter$: (ev) => { - // If no in edit mode, change it. - if (!this.edit.get()) { - mode(true); - ev.stopPropagation(); - ev.stopImmediatePropagation(); - ev.preventDefault(); - return; - } - }}) - ); - } -} diff --git a/app/client/components/Forms/Paragraph.ts b/app/client/components/Forms/Paragraph.ts index f62eb65d..44ac1321 100644 --- a/app/client/components/Forms/Paragraph.ts +++ b/app/client/components/Forms/Paragraph.ts @@ -19,7 +19,6 @@ export class ParagraphModel extends BoxModel { public override render(): HTMLElement { const box = this; const editMode = box.edit; - let element: HTMLElement; const text = this.prop('text', this.defaultValue) as Observable; // There is a spacial hack here. We might be created as a separator component, but the rendering @@ -44,18 +43,21 @@ export class ParagraphModel extends BoxModel { this.cssClass ? dom.cls(this.cssClass, not(editMode)) : null, dom.maybe(editMode, () => { const draft = Observable.create(null, text.get() || ''); - setTimeout(() => element?.focus(), 10); - return [ - element = cssTextArea(draft, {autoGrow: true, onInput: true}, - cssTextArea.cls('-edit', editMode), - css.saveControls(editMode, (ok) => { - if (ok && editMode.get()) { - text.set(draft.get()); - this.save().catch(reportError); - } - }) - ), - ]; + return cssTextArea(draft, {autoGrow: true, onInput: true}, + cssTextArea.cls('-edit', editMode), + (elem) => { + setTimeout(() => { + elem.focus(); + elem.setSelectionRange(elem.value.length, elem.value.length); + }, 10); + }, + css.saveControls(editMode, (ok) => { + if (ok && editMode.get()) { + text.set(draft.get()); + this.save().catch(reportError); + } + }) + ); }), ) }); diff --git a/app/client/components/Forms/elements.ts b/app/client/components/Forms/elements.ts index c979e084..ede2b03c 100644 --- a/app/client/components/Forms/elements.ts +++ b/app/client/components/Forms/elements.ts @@ -13,7 +13,6 @@ export * from "./Section"; export * from './Field'; export * from './Columns'; export * from './Submit'; -export * from './Label'; export function defaultElement(type: FormLayoutNodeType): FormLayoutNode { switch(type) { diff --git a/app/client/components/Forms/styles.ts b/app/client/components/Forms/styles.ts index 1288d090..935269d2 100644 --- a/app/client/components/Forms/styles.ts +++ b/app/client/components/Forms/styles.ts @@ -1,3 +1,4 @@ +import type {App} from 'app/client/ui/App'; import {textarea} from 'app/client/ui/inputs'; import {sanitizeHTML} from 'app/client/ui/sanitizeHTML'; import {basicButton, basicButtonLink, primaryButtonLink, textButton} from 'app/client/ui2018/buttons'; @@ -759,11 +760,17 @@ export function saveControls(editMode: Observable, save: (ok: boolean) } } }), - dom.on('blur', (ev) => { - if (!editMode.isDisposed() && editMode.get()) { - save(true); - editMode.set(false); + dom.create((owner) => { + // Whenever focus returns to the Clipboard component, close the editor by saving the value. + function saveEdit() { + if (!editMode.isDisposed() && editMode.get()) { + save(true); + editMode.set(false); + } } + const app = (window as any).gristApp as App; + app.on('clipboard_focus', saveEdit); + owner.onDispose(() => app.off('clipboard_focus', saveEdit)); }), ]; } diff --git a/app/client/ui/FormPage.ts b/app/client/ui/FormPage.ts index ead02922..53d8de36 100644 --- a/app/client/ui/FormPage.ts +++ b/app/client/ui/FormPage.ts @@ -165,7 +165,7 @@ const cssFormContent = styled('form', ` font-size: 10px; } & p { - margin: 0px; + margin: 0 0 10px 0; } & strong { font-weight: 600;