Skip to content

Commit ef484ee

Browse files
myieyemyieye
authored
A swath of UI bug fixes and small improvements (#2018)
* Use a button component for Lexbox link This gets us a consistent focus effect * Unify heights of local and lexbox rows * Fix: whitespace on one side of svg sync-arrow * Put Last Sync at top of tab where it has more space on mobile * SyncStatusPrimitive: put sync/login buttons in middle So not lopsided on mobile * Fix throttled api resources might never load * Use debounce for refreshing entry count instead of throttle Debounce ensures we get the latest value * Map crdt deserialization/download exceptions to user-readable exceptions * Fix: navigation while focussed in a dirty rich-text-field fails to save with error * Improve task done-view items contrast * Fix error and ws not persisted during task * Remove dist from vs-code search results * Supply root-location in storybook * Patch various rich-text errors * i18n:extract
1 parent 3dc65b5 commit ef484ee

23 files changed

Lines changed: 356 additions & 125 deletions

‎.vscode/settings.json‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,8 @@
4343
],
4444
"iconify.preview.exclude": [
4545
"**/icon-class.ts"
46-
]
46+
],
47+
"search.exclude": {
48+
"**/dist": true
49+
}
4750
}

‎backend/FwLite/LcmCrdt/RemoteSync/CrdtHttpSyncService.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,17 @@ async Task ISyncable.AddRangeFromSync(IEnumerable<Commit> commits)
8888

8989
async Task<ChangesResult<Commit>> ISyncable.GetChanges(SyncState otherHeads)
9090
{
91-
var changes = await restSyncClient.GetChanges(projectId, otherHeads);
91+
var changesResponse = await restSyncClient.GetChanges(projectId, otherHeads);
92+
if (changesResponse.Error is not null)
93+
{
94+
// The inner exception is almost certainly more interesting than the wrapping ApiException
95+
// (e.g. a JsonException if trying to download a change object that is too new for this version of FieldWorks Lite)
96+
var error = changesResponse.Error?.InnerException ?? changesResponse.Error!;
97+
throw new CrdtSyncException("FieldWorks Lite is likely out of date. Failed to download dictionary changes.",
98+
CrdtSyncException.CrdtSyncStep.Download, error);
99+
}
100+
101+
var changes = changesResponse.Content;
92102
ArgumentNullException.ThrowIfNull(changes);
93103
foreach (var commit in changes.MissingFromClient)
94104
{
@@ -128,5 +138,5 @@ public interface ISyncHttp
128138
internal Task<SyncState> GetSyncState(Guid id);
129139

130140
[Post("/api/crdt/{id}/changes")]
131-
internal Task<ChangesResult<Commit>> GetChanges(Guid id, SyncState otherHeads);
141+
internal Task<ApiResponse<ChangesResult<Commit>>> GetChanges(Guid id, SyncState otherHeads);
132142
}

‎backend/FwLite/LcmCrdt/RemoteSync/CrdtSyncException.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
namespace LcmCrdt.RemoteSync;
2+
3+
public class CrdtSyncException : Exception
4+
{
5+
public enum CrdtSyncStep { Upload, Download }
6+
7+
public CrdtSyncStep Step { get; init; }
8+
9+
public CrdtSyncException(string message, CrdtSyncStep step) : base(message)
10+
{
11+
Step = step;
12+
}
13+
public CrdtSyncException(string message, CrdtSyncStep step, Exception innerException) : base(message, innerException)
14+
{
15+
Step = step;
16+
}
17+
}

‎frontend/viewer/.storybook/decorators/FWLiteDecorator.svelte‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import {extract} from 'runed';
3030
import {setupGlobalErrorHandlers} from '$lib/errors/global-errors';
3131
import {TooltipProvider} from '$lib/components/ui/tooltip';
32+
import {initRootLocation} from '$lib/services/root-location-service';
33+
import {readable} from 'svelte/store';
3234
3335
3436
let { children }: { children: Snippet } = $props();
@@ -39,6 +41,7 @@
3941
initView();
4042
const storyContext = useSvelteStoryContext();
4143
setupGlobalErrorHandlers();
44+
initRootLocation(readable());
4245
4346
const {
4447
themePicker = true,

‎frontend/viewer/src/App.svelte‎

Lines changed: 4 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@
22
import {TooltipProvider} from '$lib/components/ui/tooltip';
33
import {setupGlobalErrorHandlers} from '$lib/errors/global-errors';
44
import {ModeWatcher} from 'mode-watcher';
5-
import {navigate, Route, Router} from 'svelte-routing';
5+
import {Router} from 'svelte-routing';
66
import NotificationOutlet from './lib/notifications/NotificationOutlet.svelte';
7-
import Sandbox from '$lib/sandbox/Sandbox.svelte';
8-
import DotnetProjectView from './DotnetProjectView.svelte';
9-
import HomeView from './home/HomeView.svelte';
10-
import TestProjectView from './TestProjectView.svelte';
11-
12-
let url = '';
7+
import AppRoutes from './AppRoutes.svelte';
138
149
setupGlobalErrorHandlers();
1510
</script>
@@ -19,49 +14,8 @@
1914

2015
<TooltipProvider delayDuration={300}>
2116
<div class="app">
22-
<Router {url}>
23-
<Route path="/project/:code/*" let:params>
24-
<Router {url}>
25-
{#key params.code}
26-
<DotnetProjectView code={params.code} type="crdt" />
27-
{/key}
28-
</Router>
29-
</Route>
30-
<Route path="/fwdata/:name/*" let:params>
31-
<Router {url}>
32-
{#key params.name}
33-
<DotnetProjectView code={params.name} type="fwdata" />
34-
{/key}
35-
</Router>
36-
</Route>
37-
<Route path="/paratext/project/:code/*" let:params>
38-
<Router {url}>
39-
{#key params.code}
40-
<DotnetProjectView code={params.code} type="crdt" paratext />
41-
{/key}
42-
</Router>
43-
</Route>
44-
<Route path="/paratext/fwdata/:name/*" let:params>
45-
<Router {url}>
46-
{#key params.name}
47-
<DotnetProjectView code={params.name} type="fwdata" paratext />
48-
{/key}
49-
</Router>
50-
</Route>
51-
<Route path="/testing/project-view/*">
52-
<Router {url}>
53-
<TestProjectView />
54-
</Router>
55-
</Route>
56-
<Route path="/">
57-
<HomeView />
58-
</Route>
59-
<Route path="/sandbox">
60-
<Sandbox />
61-
</Route>
62-
<Route path="/*">
63-
{setTimeout(() => navigate('/', { replace: true }))}
64-
</Route>
17+
<Router>
18+
<AppRoutes />
6519
</Router>
6620
<NotificationOutlet/>
6721
</div>

‎frontend/viewer/src/AppRoutes.svelte‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<script lang="ts">
2+
import { setupGlobalErrorHandlers } from '$lib/errors/global-errors';
3+
import { navigate, Route, Router, useLocation } from 'svelte-routing';
4+
import Sandbox from '$lib/sandbox/Sandbox.svelte';
5+
import DotnetProjectView from './DotnetProjectView.svelte';
6+
import HomeView from './home/HomeView.svelte';
7+
import TestProjectView from './TestProjectView.svelte';
8+
import { initRootLocation } from '$lib/services/root-location-service';
9+
10+
let url = '';
11+
12+
setupGlobalErrorHandlers();
13+
initRootLocation(useLocation());
14+
</script>
15+
16+
<Route path="/project/:code/*" let:params>
17+
<Router {url}>
18+
{#key params.code}
19+
<DotnetProjectView code={params.code} type="crdt" />
20+
{/key}
21+
</Router>
22+
</Route>
23+
<Route path="/fwdata/:name/*" let:params>
24+
<Router {url}>
25+
{#key params.name}
26+
<DotnetProjectView code={params.name} type="fwdata" />
27+
{/key}
28+
</Router>
29+
</Route>
30+
<Route path="/paratext/project/:code/*" let:params>
31+
<Router {url}>
32+
{#key params.code}
33+
<DotnetProjectView code={params.code} type="crdt" paratext />
34+
{/key}
35+
</Router>
36+
</Route>
37+
<Route path="/paratext/fwdata/:name/*" let:params>
38+
<Router {url}>
39+
{#key params.name}
40+
<DotnetProjectView code={params.name} type="fwdata" paratext />
41+
{/key}
42+
</Router>
43+
</Route>
44+
<Route path="/testing/project-view/*">
45+
<Router {url}>
46+
<TestProjectView />
47+
</Router>
48+
</Route>
49+
<Route path="/">
50+
<HomeView />
51+
</Route>
52+
<Route path="/sandbox">
53+
<Sandbox />
54+
</Route>
55+
<Route path="/*">
56+
{setTimeout(() => navigate('/', { replace: true }))}
57+
</Route>

‎frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte‎

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script lang="ts" module>
2-
import {type Node} from 'prosemirror-model';
2+
import {Fragment, Slice, type Node} from 'prosemirror-model';
33
import {cn} from '$lib/utils';
44
import {textSchema} from './editor-schema';
55
@@ -54,7 +54,8 @@
5454
} & HTMLAttributes<HTMLDivElement> = $props();
5555
5656
let elementRef: HTMLElement | null = $state(null);
57-
let dirty = $state(false);
57+
// should not be state unless we catch errors. See onBlur.
58+
let dirty = false;
5859
let editor: EditorView | null = null;
5960
6061
const isUsingKeyboard = new IsUsingKeyboard();
@@ -92,6 +93,37 @@
9293
editable() {
9394
return !readonly;
9495
},
96+
handlePaste(view, _event, slice) {
97+
if (view.state.doc.content.size === 0) {
98+
// if the field is cleared, the resulting selection breaks paste (on older devices at least)
99+
selectAll(view);
100+
}
101+
102+
// When a user copies a whole field. It includes the trailing <br>.
103+
// Pasting it often causes errors, so we remove them.
104+
function withoutBrs(nodes: readonly Node[]): Node[] {
105+
return nodes
106+
.filter(node => node.type.name !== textSchema.nodes.br.name)
107+
.map(node => {
108+
if (node.isText) return node;
109+
return node.copy(Fragment.fromArray(withoutBrs(node.content.content)));
110+
});
111+
}
112+
const cleanFragment = Fragment.fromArray(withoutBrs(slice.content.content));
113+
const cleanSlice = new Slice(cleanFragment, slice.openStart, slice.openEnd);
114+
115+
// Below code is copied from prosemirror's doPaste
116+
// https://github.com/ProseMirror/prosemirror-view/blob/381c163b0abde96cabd609a8c4fc72ed2891b0e1/src/input.ts#L624
117+
function sliceSingleNode(slice: Slice): Node | null {
118+
return slice.openStart == 0 && slice.openEnd == 0 && slice.content.childCount == 1 ? slice.content.firstChild : null;
119+
}
120+
let singleNode = sliceSingleNode(cleanSlice);
121+
let tr = singleNode
122+
? view.state.tr.replaceSelectionWith(singleNode, false)
123+
: view.state.tr.replaceSelection(cleanSlice);
124+
view.dispatch(tr.scrollIntoView().setMeta('paste', true).setMeta('uiEvent', 'paste'));
125+
return true;
126+
},
95127
handleDOMEvents: {
96128
pointerdown() {
97129
pointerDown = true;
@@ -122,8 +154,16 @@
122154
if (usingKeyboard) { // tabbed in
123155
if (IsMobile.value) {
124156
if (prevSelection) {
125-
const prevSelectionForCurrentDoc = Selection.fromJSON(editor.state.doc, prevSelection.toJSON());
126-
setSelection(prevSelectionForCurrentDoc);
157+
// We can land here when the field gets cleared for some reason.
158+
// In that case fromJSON doesn't like the prevSelection (on older devices at least)
159+
if (editor.state.doc.content.size) {
160+
try {
161+
const prevSelectionForCurrentDoc = Selection.fromJSON(editor.state.doc, prevSelection.toJSON());
162+
setSelection(prevSelectionForCurrentDoc);
163+
} catch {
164+
console.warn('Could not restore previous selection', prevSelection.toJSON(), editor.state.doc);
165+
}
166+
}
127167
prevSelection = undefined;
128168
} else {
129169
setSelection(Selection.atEnd(editor.state.doc));
@@ -135,6 +175,9 @@
135175
}
136176
137177
function onblur(editor: EditorView) {
178+
// we cannot set and $state variables here, because if this is called due to navigation
179+
// it's too late to update state and doing so will throw.
180+
// See stomp-safe-lcm-rich-text-editor for how we handle that case.
138181
if (dirty && value) {
139182
onchange(value);
140183
dirty = false;
@@ -184,6 +227,11 @@
184227
if (dispatch) dispatch(state.tr.insertText(newLine));
185228
return true;
186229
},
230+
// eslint-disable-next-line @typescript-eslint/naming-convention
231+
'Backspace': (state) => {
232+
// If the field is empty, backspace results in an error (on older devices at least)
233+
return state.doc.content.size === 0;
234+
},
187235
}),
188236
keymap(baseKeymap)
189237
]

‎frontend/viewer/src/lib/components/stomp/stomp-safe-lcm-rich-text-editor.svelte‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,47 @@
11
<script lang="ts">
2-
import {untrack, type ComponentProps} from 'svelte';
2+
import {onDestroy, untrack, type ComponentProps} from 'svelte';
33
import {StompGuard} from './stomp-guard.svelte';
44
import {mergeProps} from 'bits-ui';
55
import LcmRichTextEditor from '../lcm-rich-text-editor/lcm-rich-text-editor.svelte';
66
import {useIdleService} from '$lib/services/idle-service';
7+
import {useRootLocation} from '$lib/services/root-location-service';
78
89
type Props = ComponentProps<typeof LcmRichTextEditor> & { onchange: () => void };
910
1011
let { value = $bindable(), onchange, ...rest}: Props = $props();
1112
13+
const locationService = useRootLocation();
14+
1215
const guard = new StompGuard(
1316
() => value,
1417
(newValue) => value = newValue
1518
);
1619
17-
let idleService = useIdleService();
18-
function onIdle() {
20+
function commitAnyChanges() {
1921
if (guard.isDirty) {
2022
guard.commitAndUnlock();
2123
onchange();
2224
}
2325
}
2426
27+
let idleService = useIdleService();
2528
$effect(() => {
26-
if (idleService.isIdle) untrack(onIdle);
29+
if (idleService.isIdle) untrack(commitAnyChanges);
30+
});
31+
onDestroy(() => {
32+
// This is just a precaution. I'm not aware of a scenario where we actually need to commit at this point.
33+
commitAnyChanges();
34+
35+
return locationService.subscribe(() => {
36+
// This handler is required, because the contenteditable blur event is too late when blurring due to navigation.
37+
// Calling subscribe seems to be the only reliable way to get the callback triggered in that case.
38+
commitAnyChanges();
39+
});
2740
});
2841
</script>
2942

3043
<LcmRichTextEditor bind:value={guard.value} {...mergeProps({
3144
onchange: () => {
32-
guard.commitAndUnlock();
45+
commitAnyChanges();
3346
}
34-
}, { onchange }, rest)} />
47+
}, rest)} />

‎frontend/viewer/src/lib/project-context.svelte.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type {
66
import type {
77
ISyncServiceJsInvokable
88
} from '$lib/dotnet-types/generated-types/FwLiteShared/Services/ISyncServiceJsInvokable';
9-
import {resource, type ResourceOptions, type ResourceReturn} from 'runed';
9+
import {resource, watchOnce, type ResourceOptions, type ResourceReturn} from 'runed';
1010
import {SvelteMap} from 'svelte/reactivity';
1111
import type {IProjectData} from '$lib/dotnet-types/generated-types/LcmCrdt/IProjectData';
1212

@@ -130,6 +130,23 @@ export class ProjectContext {
130130
if (!api) return Promise.resolve(initialValue);
131131
return factory(api);
132132
}), {initialValue, ...options});
133+
134+
// If throttling and the api is not yet defined, the resource will likely throttle/swallow the refetch
135+
// call triggered by the api becoming defined. As a result it will never be loaded. So, we explicitly ensure an initial load.
136+
if (options?.throttle && !this.#api) {
137+
function initialLoad(api: IMiniLcmJsInvokable) {
138+
factory(api).then(res.mutate).catch(console.error);
139+
}
140+
141+
if (this.#api) {
142+
initialLoad(this.#api);
143+
} else {
144+
watchOnce(() => this.#api, () => {
145+
if (this.#api) initialLoad(this.#api);
146+
else console.warn('apiResource: initialLoad expected api to be defined after watchOnce');
147+
});
148+
}
149+
}
133150
return res;
134151
}
135152

‎frontend/viewer/src/lib/project-stats.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export function useProjectStats() {
1515
totalEntryCount,
1616
}
1717
}, {
18-
throttle: 3000,
18+
debounce: 500,
1919
onAdd: (resource) => {
2020
projectEventBus.onEntryDeleted(() => void resource.refetch());
2121
projectEventBus.onEntryUpdated(() => void resource.refetch());

0 commit comments

Comments
 (0)