diff --git a/frontend/viewer/package.json b/frontend/viewer/package.json
index 479396ef48..dd3a0c22d4 100644
--- a/frontend/viewer/package.json
+++ b/frontend/viewer/package.json
@@ -30,12 +30,13 @@
"test:watch": "vitest",
"test:storybook": "vitest --project=storybook",
"test:unit": "vitest --project=unit",
+ "test:browser": "vitest --project=browser",
"check": "svelte-check",
"lint": "eslint .",
"lint:report": "eslint . --output-file eslint_report.json --format json",
"i18n:extract": "lingui extract --clean && lingui extract",
"generate-icon-types": "node ./generate-icon-types.js",
- "storybook": "storybook dev -p 6006",
+ "storybook": "storybook dev -p 6006 --host 0.0.0.0",
"build-storybook": "storybook build"
},
"devDependencies": {
diff --git a/frontend/viewer/src/lib/components/lcm-rich-text-editor/editor-schema.ts b/frontend/viewer/src/lib/components/lcm-rich-text-editor/editor-schema.ts
index c8f4e671d9..6a9856070a 100644
--- a/frontend/viewer/src/lib/components/lcm-rich-text-editor/editor-schema.ts
+++ b/frontend/viewer/src/lib/components/lcm-rich-text-editor/editor-schema.ts
@@ -1,10 +1,14 @@
import {Schema} from 'prosemirror-model';
import {gt} from 'svelte-i18n-lingui';
import {cn} from '$lib/utils';
-
+/*
+ whitespace: pre tells prose-mirror how to parse the dom, not how to render it, that's our job
+ */
export const textSchema = new Schema({
nodes: {
- text: {},
+ text: {
+ whitespace: 'pre',
+ },
/**
* Note: it seems that our spans likely should have been modeled as "marks" rather than "nodes".
* Conceptually our users "mark" up a text.
@@ -13,7 +17,9 @@ export const textSchema = new Schema({
* */
span: {
selectable: false,
- content: 'text*',
+ content: 'text* br?',
+ // If we remove this Backspace + Delete start removing whole spans
+ inline: true,
whitespace: 'pre',
toDOM: (node) => {
return ['span', {
@@ -28,6 +34,20 @@ export const textSchema = new Schema({
//this allows us to update the text property without having to map all the span properties into the schema
attrs: {richSpan: {default: {}}, className: {default: ''}}
},
- doc: {content: 'span*', attrs: {}}
+ br: {
+ inline: true,
+ group: 'inline',
+ selectable: false,
+ linebreakReplacement: true,
+ toDOM: () => ['br'],
+ parseDOM: [{tag: 'br'}]
+ },
+ doc: {
+ whitespace: 'pre',
+ // if we remove this Shift + Enter creates new spans and then Backspace starts removing whole spans
+ inline: true,
+ content: 'span*',
+ attrs: {}
+ }
}
});
diff --git a/frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte b/frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte
index 77d0519d55..b6d8bf38f9 100644
--- a/frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte
+++ b/frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte
@@ -13,7 +13,7 @@
import type {IRichString} from '$lib/dotnet-types/generated-types/MiniLcm/Models/IRichString';
import {Label} from '$lib/components/ui/label';
import {EditorView} from 'prosemirror-view';
- import {AllSelection, EditorState} from 'prosemirror-state';
+ import {AllSelection, EditorState, Selection} from 'prosemirror-state';
import {keymap} from 'prosemirror-keymap';
import {baseKeymap} from 'prosemirror-commands';
import {undo, redo, history} from 'prosemirror-history';
@@ -22,8 +22,10 @@
import type {HTMLAttributes} from 'svelte/elements';
import {IsUsingKeyboard} from 'bits-ui';
import type {IRichSpan} from '$lib/dotnet-types/generated-types/MiniLcm/Models/IRichSpan';
- import {inputVariants} from '../ui/input/input.svelte';
import {on} from 'svelte/events';
+ import {IsMobile} from '$lib/hooks/is-mobile.svelte';
+ import {findNextTabbable} from '$lib/utils/tabbable';
+ import {inputVariants} from '../ui/input/input.svelte';
let {
value = $bindable(),
@@ -36,6 +38,8 @@
readonly = false,
onchange = () => {},
autofocus,
+ class: className,
+ enterkeyhint = 'next',
...rest
}:
{
@@ -54,6 +58,8 @@
let editor: EditorView | null = null;
const isUsingKeyboard = new IsUsingKeyboard();
+ // isUsingKeyboard.current isn't entirely reliable on mobile due to virtual keyboards
+ let pointerDown = false;
onMount(() => {
// docs https://prosemirror.net/docs/
@@ -72,7 +78,7 @@
editor.updateState(newState);
},
attributes: {
- class: inputVariants({class: 'min-h-10 h-auto block'}),
+ class: inputVariants({class: 'min-h-10 h-auto pb-1.5'}),
// todo: the distribution of props between the editor and the elementRef is not good
// there should probably be a wrapper component that provides the elementRef to this one
...(id ? {id} : {}),
@@ -81,13 +87,25 @@
role: 'textbox',
...(autocapitalize ? {autocapitalize} : {}),
spellcheck: 'false',
+ ...(enterkeyhint ? {enterkeyhint} : {}),
},
editable() {
return !readonly;
},
handleDOMEvents: {
- 'focus': onfocus,
+ pointerdown() {
+ pointerDown = true;
+ setTimeout(() => pointerDown = false, 100); // yes, apparently we need a decently high timeout value
+ },
+ 'focus'(editor) {
+ onfocus(editor, !pointerDown);
+ },
'blur': onblur,
+ keydown(_view, event) {
+ if (event.key === 'Enter') {
+ onEnterKey();
+ }
+ },
}
});
editor.dom.setAttribute('tabindex', '0');
@@ -97,9 +115,22 @@
if (relatedLabel) return on(relatedLabel, 'click', onFocusTargetClick);
});
- function onfocus(editor: EditorView) {
- if (isUsingKeyboard.current) { // tabbed in
- selectAll(editor);
+ let prevSelection: Selection | undefined;
+ function onfocus(editor: EditorView, viaKeyboard: boolean) {
+ const usingKeyboard = isUsingKeyboard.current ||
+ IsMobile.value && viaKeyboard;
+ if (usingKeyboard) { // tabbed in
+ if (IsMobile.value) {
+ if (prevSelection) {
+ const prevSelectionForCurrentDoc = Selection.fromJSON(editor.state.doc, prevSelection.toJSON());
+ setSelection(prevSelectionForCurrentDoc);
+ prevSelection = undefined;
+ } else {
+ setSelection(Selection.atEnd(editor.state.doc));
+ }
+ } else {
+ selectAll(editor);
+ }
}
}
@@ -108,6 +139,7 @@
onchange(value);
dirty = false;
}
+ prevSelection = editor.state.selection;
clearSelection(editor);
}
@@ -139,6 +171,15 @@
keymap({
'Mod-z': undo,
'Mod-y': redo,
+ // eslint-disable-next-line @typescript-eslint/naming-convention
+ 'Enter': () => {
+ // This handler is not triggered reliably on mobile, so we're using handleDOMEvents.keydown instead
+ // if (IsMobile.value) onEnterKey();
+
+ // and we never want to do anything on desktop
+ // (partially because it causes range errors - on mobile too)
+ return true;
+ },
'Shift-Enter': (state, dispatch) => {
if (dispatch) dispatch(state.tr.insertText(newLine));
return true;
@@ -149,19 +190,32 @@
});
}
+ function onEnterKey(): void {
+ if (IsMobile.value && enterkeyhint === 'next' && editor?.dom) {
+ // mimic 'next' behaviour
+ const nextTabbable = findNextTabbable(editor.dom);
+ nextTabbable?.focus();
+ }
+ }
+
function valueToDoc(): Node {
return textSchema.node('doc', {}, value?.spans
.filter(s => s.text)
- .map(s => richSpanToNode(s)) ?? []);
+ .map((s, i, all) => richSpanToNode(s, i === all.length - 1)) ?? []);
}
- function richSpanToNode(s: IRichSpan) {
+ function richSpanToNode(s: IRichSpan, isLast: boolean): Node {
//we must pull text out of what is stored on the node attrs
//ProseMirror will keep the text up to date itself, if we store it on the richSpan attr then it will become out of date
let {text, ...rest} = s;
//if the ws doesn't match expected, or there's more than just the ws key in props
const isCustomized = (!!s.ws && !!normalWs && normalWs !== s.ws) || Object.keys(rest).length > 1;
- return textSchema.node('span', {richSpan: rest, className: cn(isCustomized && 'customized')}, [textSchema.text(replaceLineSeparatorWithNewLine(text))]);
+ return textSchema.node('span', {richSpan: rest, className: cn(isCustomized && 'customized')}, [
+ textSchema.text(replaceLineSeparatorWithNewLine(text)),
+ // a seems to be the only thing that will cause a trailing \n to be rendered
+ // this is what Prose-Mirror does if inline: false, which we can't use
+ ...(isLast ? [textSchema.node('br')] : [])
+ ]);
}
function richSpanFromNode(node: Node) {
@@ -169,7 +223,16 @@
}
function selectAll(editor: EditorView) {
- editor.dispatch(editor.state.tr.setSelection(new AllSelection(editor.state.doc)));
+ setSelection(new AllSelection(editor.state.doc));
+
+ if (!readonly) return; // happens automatically if not readonly
+
+ const selection = window.getSelection();
+ if (!selection) return;
+ const range = document.createRange();
+ range.selectNodeContents(editor.dom);
+ selection.removeAllRanges();
+ selection.addRange(range);
}
function clearSelection(editor: EditorView) {
@@ -181,6 +244,19 @@
}
}
+ function setSelection(selection: Selection): void {
+ if (!editor) return;
+ editor.dispatch(editor.state.tr.setSelection(selection));
+ setTimeout(() => {
+ if (!editor) return;
+ if (editor.state.selection.eq(selection) || selection.$anchor.doc !== editor.state.doc) return;
+ // when tabbing back and forth between 2 rich text editors, the previous setSelection is not sufficient 🤷
+ // (It doesn't have anything to do with the clearSelection() below)
+ // The first call is kept, because it avoid a visible delay when possible
+ editor.dispatch(editor.state.tr.setSelection(selection));
+ }, 1); // 0 is not enough on Firefox
+ }
+
//lcm expects line separators, but html does not render them, so we replace them with \n
function replaceNewLineWithLineSeparator(text: string) {
return text.replaceAll(newLine, lineSeparator);
@@ -190,17 +266,24 @@
}
function onFocusTargetClick(event: MouseEvent) {
- if (!editor) return;
- if (event.target === editor?.dom) return; // the editor will handle focus itself
- editor.focus();
+ if (!editor || !event.target) return;
+ if (event.target === editor.dom || event.target instanceof globalThis.Node && editor.dom.contains(event.target))
+ return; // the editor will handle focus itself
+ editor?.focus();
+ // Minorly dissatisfying is that when you click on the padding to the right of prose-mirror,
+ // the caret is not intelligently placed at the end of the text (and on the correct line if multi-line)
+ // Everything else seems good
}
-
{#if label}
-
-
+
+
{:else}
-
+
{/if}
diff --git a/frontend/viewer/src/lib/components/ui/input/input-shell.svelte b/frontend/viewer/src/lib/components/ui/input/input-shell.svelte
index 5ed7731285..1d9f84570b 100644
--- a/frontend/viewer/src/lib/components/ui/input/input-shell.svelte
+++ b/frontend/viewer/src/lib/components/ui/input/input-shell.svelte
@@ -8,7 +8,7 @@
focusRingClass?: string;
}
- const anyChildHasFocusRing = 'has-[:focus-visible]:ring-ring has-[:focus-visible]:outline-none has-[:focus-visible]:ring-2 has-[:focus-visible]:ring-offset-2';
+ const anyChildHasFocusRing = 'ring-ring outline-none ring-offset-2 has-[:focus-visible]:ring-2';
let {
ref = $bindable(null),
diff --git a/frontend/viewer/src/lib/utils/tabbable.test.ts b/frontend/viewer/src/lib/utils/tabbable.test.ts
new file mode 100644
index 0000000000..a26871a423
--- /dev/null
+++ b/frontend/viewer/src/lib/utils/tabbable.test.ts
@@ -0,0 +1,207 @@
+import {afterEach, beforeEach, describe, expect, it} from 'vitest';
+import {findFirstTabbable, findNextTabbable} from './tabbable';
+
+describe('tabbable utilities', () => {
+ let container: HTMLDivElement;
+
+ beforeEach(() => {
+ // Create a fresh container for each test
+ container = document.createElement('div');
+ document.body.appendChild(container);
+ });
+
+ afterEach(() => {
+ // Clean up after each test
+ document.body.removeChild(container);
+ });
+
+ describe('findFirstTabbable', () => {
+ it('should return undefined for null container', () => {
+ expect(findFirstTabbable(null)).toBeUndefined();
+ });
+
+ it('should return undefined for undefined container', () => {
+ expect(findFirstTabbable(undefined)).toBeUndefined();
+ });
+
+ it('should return null when no tabbable elements exist', () => {
+ container.innerHTML = '
+ `;
+ const result = findFirstTabbable(container);
+ expect(result?.id).toBe('first');
+ });
+
+ it('should find tabbable element with explicit tabindex', () => {
+ container.innerHTML = `
+
+ Tabbable span
+
+
+ `;
+ const result = findFirstTabbable(container);
+ expect(result?.id).toBe('tabbable-span');
+ });
+ });
+
+ describe('findNextTabbable', () => {
+ it('should return undefined when current is null', () => {
+ expect(findNextTabbable(null)).toBeUndefined();
+ });
+
+ it('should return undefined when current is undefined', () => {
+ expect(findNextTabbable(undefined)).toBeUndefined();
+ });
+
+ it('should return current element when it has no parent', () => {
+ const isolated = document.createElement('button');
+ expect(findNextTabbable(isolated)).toBe(isolated);
+ });
+
+ it('should throw error when current element is not tabbable', () => {
+ container.innerHTML = '
Not tabbable
';
+ const notTabbable = container.querySelector('#not-tabbable') as HTMLElement;
+
+ expect(() => findNextTabbable(notTabbable)).toThrow(
+ 'Current element is not tabbable, so can\'t find relative tabbable element'
+ );
+ });
+
+ it('should find the next tabbable element in same container', () => {
+ container.innerHTML = `
+
+
+
+
+
+ `;
+ const first = container.querySelector('#first') as HTMLElement;
+ const result = findNextTabbable(first);
+ expect(result?.id).toBe('second');
+ });
+
+ it('should wrap to first element when at the end', () => {
+ container.innerHTML = `
+
+
+
+
+
+ `;
+ const third = container.querySelector('#third') as HTMLElement;
+ const result = findNextTabbable(third);
+ expect(result?.id).toBe('first');
+ });
+
+ it('should traverse up the DOM when no next element in current container', () => {
+ container.innerHTML = `
+