From ea082d65d013b1558aff656e74b535dcde7060a4 Mon Sep 17 00:00:00 2001 From: Eemeli Aro Date: Thu, 25 May 2023 19:20:54 +0300 Subject: [PATCH 1/4] refactor: Add EditFieldHandle and EditorResult as interfaces for the editor value --- translate/.eslintrc.js | 1 + translate/src/context/Editor.test.js | 179 ++++++---- translate/src/context/Editor.tsx | 316 +++++++++--------- translate/src/context/UnsavedChanges.tsx | 24 +- .../modules/editor/components/Editor.test.js | 19 +- .../modules/editor/components/FtlSwitch.tsx | 8 +- .../components/MachinerySourceIndicator.tsx | 8 +- .../components/TranslationLength.test.js | 5 +- .../editor/components/TranslationLength.tsx | 8 +- .../useExistingTranslationGetter.test.js | 50 +-- .../hooks/useExistingTranslationGetter.ts | 9 +- .../components/OriginalString.tsx | 2 +- .../originalstring/components/RichString.tsx | 8 +- .../components/EditAccesskey.tsx | 144 ++++---- .../translationform/components/EditField.tsx | 138 +++++--- .../TranslationForm-multiple.test.js | 18 +- .../components/TranslationForm-single.test.js | 21 +- .../components/TranslationForm.tsx | 119 +++---- .../src/utils/message/buildMessageEntry.tsx | 26 +- .../src/utils/message/editMessageEntry.tsx | 34 +- .../message/extractAccessKeyCandidates.ts | 14 +- translate/src/utils/message/index.ts | 2 +- 22 files changed, 646 insertions(+), 507 deletions(-) diff --git a/translate/.eslintrc.js b/translate/.eslintrc.js index c53b62c4b8..946aa041c5 100644 --- a/translate/.eslintrc.js +++ b/translate/.eslintrc.js @@ -19,6 +19,7 @@ module.exports = { '@typescript-eslint/no-empty-function': 0, '@typescript-eslint/no-explicit-any': 0, '@typescript-eslint/no-inferrable-types': 0, + '@typescript-eslint/no-unused-vars': ['error', { varsIgnorePattern: '^_' }], '@typescript-eslint/prefer-as-const': 0, 'import/no-default-export': 'error', }, diff --git a/translate/src/context/Editor.test.js b/translate/src/context/Editor.test.js index b3f418f0f8..b355b7c548 100644 --- a/translate/src/context/Editor.test.js +++ b/translate/src/context/Editor.test.js @@ -6,7 +6,12 @@ import { act } from 'react-dom/test-utils'; import { createReduxStore, mountComponentWithStore } from '~/test/store'; import { editMessageEntry, parseEntry } from '~/utils/message'; -import { EditorActions, EditorData, EditorProvider } from './Editor'; +import { + EditorActions, + EditorData, + EditorProvider, + EditorResult, +} from './Editor'; import { EntityView, EntityViewProvider } from './EntityView'; import { Locale } from './Locale'; import { Location, LocationProvider } from './Location'; @@ -68,37 +73,64 @@ function mountSpy(Spy, format, translation) { describe('', () => { it('provides a simple non-Fluent value', () => { - let editor; + let editor, result; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); return null; }; mountSpy(Spy, 'simple', 'message'); expect(editor).toMatchObject({ sourceView: false, - initial: [{ id: '', keys: [], labels: [], name: '', value: 'message' }], - value: [{ id: '', keys: [], labels: [], name: '', value: 'message' }], + initial: { + id: 'key', + value: { pattern: { body: [{ type: 'text', value: 'message' }] } }, + }, + fields: [ + { + id: '', + keys: [], + labels: [], + name: '', + handle: { current: { value: 'message' } }, + }, + ], }); + expect(result).toMatchObject([{ name: '', keys: [], value: 'message' }]); }); it('provides a simple Fluent value', () => { - let editor; + let editor, result; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); return null; }; mountSpy(Spy, 'ftl', 'key = message'); expect(editor).toMatchObject({ sourceView: false, - initial: [{ id: '', keys: [], labels: [], name: '', value: 'message' }], - value: [{ id: '', keys: [], labels: [], name: '', value: 'message' }], + initial: { + id: 'key', + value: { pattern: { body: [{ type: 'text', value: 'message' }] } }, + }, + fields: [ + { + id: '', + keys: [], + labels: [], + name: '', + handle: { current: { value: 'message' } }, + }, + ], }); + expect(result).toMatchObject([{ name: '', keys: [], value: 'message' }]); }); it('provides a rich Fluent value', () => { - let editor; + let editor, result; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); return null; }; const source = ftl` @@ -111,14 +143,23 @@ describe('', () => { `; mountSpy(Spy, 'ftl', source); - const value = editMessageEntry(parseEntry(source)); - expect(editor).toMatchObject({ sourceView: false, initial: value, value }); + const entry = parseEntry(source); + const fields = editMessageEntry(parseEntry(source)).map((field) => ({ + ...field, + handle: { current: { value: field.handle.current.value } }, + })); + expect(editor).toMatchObject({ sourceView: false, initial: entry, fields }); + expect(result).toMatchObject([ + { name: '', keys: [{ type: 'nmtoken', value: 'one' }], value: 'ONE' }, + { name: '', keys: [{ type: '*', value: 'other' }], value: 'OTHER' }, + ]); }); it('provides a forced source Fluent value', () => { - let editor; + let editor, result; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); return null; }; const source = '## comment\n'; @@ -126,19 +167,28 @@ describe('', () => { expect(editor).toMatchObject({ sourceView: true, - initial: [ - { id: '', keys: [], labels: [], name: '', value: '## comment' }, + initial: { + id: 'key', + value: { pattern: { body: [{ type: 'text', value: '## comment\n' }] } }, + }, + fields: [ + { + handle: { current: { value: '## comment' } }, + id: '', + keys: [], + labels: [], + name: '', + }, ], - value: [{ id: '', keys: [], labels: [], name: '', value: '## comment' }], }); + expect(result).toMatchObject([{ name: '', keys: [], value: '## comment' }]); }); it('updates state on entity and plural form changes', () => { - let editor; - let location; - let entity; + let editor, result, location, entity; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); location = useContext(Location); entity = useContext(EntityView); return null; @@ -149,26 +199,30 @@ describe('', () => { wrapper.update(); expect(editor).toMatchObject({ - sourceView: false, - initial: [{ id: '', keys: [], labels: [], name: '', value: 'one' }], - value: [{ id: '', keys: [], labels: [], name: '', value: 'one' }], + initial: { + value: { pattern: { body: [{ type: 'text', value: 'one' }] } }, + }, + fields: [{ handle: { current: { value: 'one' } } }], }); + expect(result).toMatchObject([{ value: 'one' }]); act(() => entity.setPluralForm(1)); wrapper.update(); expect(editor).toMatchObject({ - sourceView: false, - initial: [{ id: '', keys: [], labels: [], name: '', value: 'other' }], - value: [{ id: '', keys: [], labels: [], name: '', value: 'other' }], + initial: { + value: { pattern: { body: [{ type: 'text', value: 'other' }] } }, + }, + fields: [{ handle: { current: { value: 'other' } } }], }); + expect(result).toMatchObject([{ value: 'other' }]); }); it('clears a rich Fluent value', () => { - let editor; - let actions; + let editor, result, actions; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); actions = useContext(EditorActions); return null; }; @@ -184,40 +238,41 @@ describe('', () => { wrapper.update(); expect(editor).toMatchObject({ - fields: [{ current: null }, { current: null }], sourceView: false, - value: [ + fields: [ { + handle: { current: { value: '' } }, + id: '|one', keys: [{ type: 'nmtoken', value: 'one' }], labels: [{ label: 'one', plural: true }], name: '', - value: '', }, { + handle: { current: { value: '' } }, + id: '|other', keys: [{ type: '*', value: 'other' }], labels: [{ label: 'other', plural: true }], name: '', - value: '', }, ], }); + expect(result).toMatchObject([ + { name: '', keys: [{ type: 'nmtoken', value: 'one' }], value: '' }, + { name: '', keys: [{ type: '*', value: 'other' }], value: '' }, + ]); }); it('sets editor from history', () => { - let editor; - let actions; + let editor, result, actions; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); actions = useContext(EditorActions); return null; }; const wrapper = mountSpy(Spy, 'ftl', `key = VALUE\n`); - expect(editor).toMatchObject({ - fields: [{ current: null }], - sourceView: false, - value: [{ keys: [], labels: [], name: '', value: 'VALUE' }], - }); + expect(result).toMatchObject([{ name: '', keys: [], value: 'VALUE' }]); const source = ftl` key = @@ -230,30 +285,35 @@ describe('', () => { wrapper.update(); expect(editor).toMatchObject({ - fields: [{ current: null }, { current: null }], sourceView: false, - value: [ + fields: [ { + handle: { current: { value: 'ONE' } }, + id: '|one', keys: [{ type: 'nmtoken', value: 'one' }], labels: [{ label: 'one', plural: true }], name: '', - value: 'ONE', }, { + handle: { current: { value: 'OTHER' } }, + id: '|other', keys: [{ type: '*', value: 'other' }], labels: [{ label: 'other', plural: true }], name: '', - value: 'OTHER', }, ], }); + expect(result).toMatchObject([ + { name: '', keys: [{ type: 'nmtoken', value: 'one' }], value: 'ONE' }, + { name: '', keys: [{ type: '*', value: 'other' }], value: 'OTHER' }, + ]); }); it('toggles Fluent source view', () => { - let editor; - let actions; + let editor, result, actions; const Spy = () => { editor = useContext(EditorData); + result = useContext(EditorResult); actions = useContext(EditorActions); return null; }; @@ -269,31 +329,26 @@ describe('', () => { wrapper.update(); expect(editor).toMatchObject({ - fields: [{ current: null }], sourceView: true, - value: [{ keys: [], labels: [], name: '', value: source }], - }); - - act(() => actions.toggleSourceView()); - wrapper.update(); - - expect(editor).toMatchObject({ - fields: [{ current: null }, { current: null }], - sourceView: false, - value: [ + fields: [ { - keys: [{ type: 'nmtoken', value: 'one' }], - labels: [{ label: 'one', plural: true }], + handle: { current: { value: source } }, + id: '', + keys: [], + labels: [], name: '', - value: 'ONE', - }, - { - keys: [{ type: '*', value: 'other' }], - labels: [{ label: 'other', plural: true }], - name: '', - value: 'OTHER', }, ], }); + expect(result).toMatchObject([{ name: '', keys: [], value: source }]); + + act(() => actions.toggleSourceView()); + wrapper.update(); + + expect(editor).toMatchObject({ fields: [{}, {}], sourceView: false }); + expect(result).toMatchObject([ + { name: '', keys: [{ type: 'nmtoken', value: 'one' }], value: 'ONE' }, + { name: '', keys: [{ type: '*', value: 'other' }], value: 'OTHER' }, + ]); }); }); diff --git a/translate/src/context/Editor.tsx b/translate/src/context/Editor.tsx index 28a423d36d..47ff961123 100644 --- a/translate/src/context/Editor.tsx +++ b/translate/src/context/Editor.tsx @@ -8,11 +8,12 @@ import React, { } from 'react'; import type { SourceType } from '~/api/machinery'; -import { useTranslationStatus } from '~/modules/entities/useTranslationStatus'; import { useReadonlyEditor } from '~/hooks/useReadonlyEditor'; +import { useTranslationStatus } from '~/modules/entities/useTranslationStatus'; import { buildMessageEntry, editMessageEntry, + editSource, requiresSourceView, getEmptyMessageEntry, MessageEntry, @@ -25,9 +26,16 @@ import { EntityView, useActiveTranslation } from './EntityView'; import { FailedChecksData } from './FailedChecksData'; import { Locale } from './Locale'; import { MachineryTranslations } from './MachineryTranslations'; -import { UnsavedActions, UnsavedChanges } from './UnsavedChanges'; +import { UnsavedActions } from './UnsavedChanges'; + +export type EditFieldHandle = { + get value(): string; + set value(next: string); + focus(): void; + setSelection(content: string): void; +}; -export type EditorMessage = Array<{ +export type EditorField = { /** An identifier for this field */ id: string; @@ -39,61 +47,8 @@ export type EditorMessage = Array<{ labels: Array<{ label: string; plural: boolean }>; - /** - * A flattened representation of a single message pattern, - * which may contain syntactic representations of placeholders. - */ - value: string; -}>; - -function editSource(source: string | MessageEntry) { - const value = - typeof source === 'string' ? source : serializeEntry('ftl', source); - return [{ id: '', name: '', keys: [], labels: [], value: value.trim() }]; -} - -/** - * Creates a copy of `base` with an entry matching `id` updated to `value`. - * - * @param id If empty, matches first entry of `base`. - * If set, a path split by `|` characters. - */ -function setEditorMessage( - base: EditorMessage, - id: string | null | undefined, - value: string, -): EditorMessage { - let set = false; - return base.map((field) => { - if (!set && (!id || field.id === id)) { - set = true; - return { ...field, value }; - } else { - return field; - } - }); -} - -function parseEntryFromFluentSource(base: MessageEntry, source: string) { - const entry = parseEntry(source); - if (entry) { - entry.id = base.id; - } - return entry; -} - -/** - * Create a new MessageEntry with a simple string pattern `value`, - * using `id` as its identifier. - */ -const createSimpleMessageEntry = (id: string, value: string): MessageEntry => ({ - id, - value: { - type: 'message', - declarations: [], - pattern: { body: [{ type: 'text', value }] }, - }, -}); + handle: React.MutableRefObject; +}; export type EditorData = Readonly<{ /** Is a request to send a new translation running? */ @@ -102,19 +57,17 @@ export type EditorData = Readonly<{ /** Used to reconstruct edited messages */ entry: MessageEntry; - /** Editor input components */ - fields: Array< - React.MutableRefObject - >; + /** Input fields for the value being edited */ + fields: EditorField[]; /** - * Index in `fields` of the current or most recent field with focus; + * The current or most recent field with focus; * used as the target of machinery replacements. */ - focusField: React.MutableRefObject; + focusField: React.MutableRefObject; /** Used for detecting unsaved changes */ - initial: EditorMessage; + initial: MessageEntry; machinery: { manual: boolean; @@ -123,9 +76,20 @@ export type EditorData = Readonly<{ } | null; sourceView: boolean; +}>; + +export type EditorResult = Array<{ + /** Attribute name, or empty for the value */ + name: string; + + /** Selector keys, or empty array for single-pattern messages */ + keys: Variant['keys']; - /** The current value being edited */ - value: EditorMessage; + /** + * A flattened representation of a single message pattern, + * which may contain syntactic representations of placeholders. + */ + value: string; }>; export type EditorActions = { @@ -136,8 +100,8 @@ export type EditorActions = { /** If `format: 'ftl'`, must be called with the source of a full entry */ setEditorFromHistory(value: string): void; - /** For `view: 'rich'`, if `value` is a string, sets the value of the active input */ - setEditorFromInput(value: string | EditorMessage): void; + /** Set the result value of the active input */ + setEditorFromInput(idx: number, value: string): void; /** @param manual Set `true` when value set due to direct user action */ setEditorFromHelpers( @@ -151,15 +115,35 @@ export type EditorActions = { toggleSourceView(): void; }; +function parseEntryFromFluentSource(base: MessageEntry, source: string) { + const entry = parseEntry(source); + if (entry) { + entry.id = base.id; + } + return entry; +} + +/** + * Create a new MessageEntry with a simple string pattern `value`, + * using `id` as its identifier. + */ +const createSimpleMessageEntry = (id: string, value: string): MessageEntry => ({ + id, + value: { + type: 'message', + declarations: [], + pattern: { body: [{ type: 'text', value }] }, + }, +}); + const initEditorData: EditorData = { busy: false, entry: { id: '', value: null, attributes: new Map() }, - fields: [], - focusField: { current: 0 }, - initial: [], + focusField: { current: null }, + initial: { id: '', value: null, attributes: new Map() }, machinery: null, + fields: [], sourceView: false, - value: [], }; const initEditorActions: EditorActions = { @@ -173,8 +157,16 @@ const initEditorActions: EditorActions = { }; export const EditorData = createContext(initEditorData); +export const EditorResult = createContext([]); export const EditorActions = createContext(initEditorActions); +const buildResult = (message: EditorField[]): EditorResult => + message.map(({ handle, keys, name }) => ({ + name, + keys, + value: handle.current.value, + })); + export function EditorProvider({ children }: { children: React.ReactElement }) { const locale = useContext(Locale); const { entity } = useContext(EntityView); @@ -182,37 +174,49 @@ export function EditorProvider({ children }: { children: React.ReactElement }) { const readonly = useReadonlyEditor(); const machinery = useContext(MachineryTranslations); const { setUnsavedChanges } = useContext(UnsavedActions); - const { exist } = useContext(UnsavedChanges); - const { resetFailedChecks } = useContext(FailedChecksData); + const { resetFailedChecks, source: failedChecksSource } = + useContext(FailedChecksData); - const [state, setState] = useState(initEditorData); + const [state, setState] = useState(initEditorData); + const [result, setResult] = useState([]); const actions = useMemo(() => { if (readonly) { return initEditorActions; } return { - clearEditor: () => - setState((prev) => { - const empty = prev.value.map((field) => ({ ...field, value: '' })); - return { ...prev, value: empty }; - }), + clearEditor() { + setState((state) => { + for (const field of state.fields) { + field.handle.current.value = ''; + } + return state; + }); + setResult((result) => + result.map(({ name, keys }) => ({ name, keys, value: '' })), + ); + }, setEditorBusy: (busy) => setState((prev) => (busy === prev.busy ? prev : { ...prev, busy })), setEditorFromHelpers: (str, sources, manual) => setState((prev) => { - const { fields, focusField, sourceView, value } = prev; - const input = fields[focusField.current]?.current; - let next = setEditorMessage(value, input?.id, str); + const { fields, focusField, sourceView } = prev; + const field = focusField.current ?? fields[0]; + field.handle.current.value = str; + let next = fields.slice(); + let result = buildResult(next); if (sourceView) { - next = editSource(buildMessageEntry(prev.entry, next)); + next = editSource(buildMessageEntry(prev.entry, result)); + focusField.current = next[0]; + result = buildResult(next); } + setResult(result); return { ...prev, machinery: { manual, translation: str, sources }, - value: next, + fields: next, }; }), @@ -225,80 +229,59 @@ export function EditorProvider({ children }: { children: React.ReactElement }) { next.entry = entry; } if (entry && !requiresSourceView(entry)) { - next.value = prev.sourceView + next.fields = prev.sourceView ? editSource(entry) : editMessageEntry(entry); } else { - next.value = editSource(str); + next.fields = editSource(str); next.sourceView = true; } - next.fields = next.value.map(() => ({ current: null })); } else { - next.value = setEditorMessage(prev.initial, null, str); + next.fields = editMessageEntry(prev.initial); + next.fields[0].handle.current.value = str; } + next.focusField.current = next.fields[0]; + setResult(buildResult(next.fields)); return next; }), - setEditorFromInput: (input) => - setState((prev) => { - if (typeof input === 'string') { - const { fields, focusField, value } = prev; - const field = fields[focusField.current]?.current; - const next = setEditorMessage(value, field?.id, input); - return { ...prev, value: next }; - } else { - return { ...prev, value: input }; - } + setEditorFromInput: (idx, value) => + setResult((prev) => { + const res = prev.slice(); + res[idx] = { ...res[idx], value }; + return res; }), setEditorSelection: (content) => - setState((prev) => { - const { fields, focusField, sourceView, value } = prev; - let next: EditorMessage; - const input = fields[focusField.current]?.current; - if (input) { - input.setRangeText( - content, - input.selectionStart ?? 0, // never actually null for or