A swath of UI bug fixes and small improvements

[myieye]

I can't think of any known UI bugs that this doesn't cover.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Adds backend error-handling for CRDT HTTP sync using ApiResponse and a new CrdtSyncException. Introduces a root-location service and centralizes routing into AppRoutes. Enhances rich-text editor paste/selection and commit behaviors. Adjusts sync UI components and sizing. Tweaks project resource initialization timing and switches stats to debounce. Minor styling updates.

Changes

Cohort / File(s)	Summary
Editor/IDE config .vscode/settings.json	Adds search.exclude to ignore all dist directories.
Backend CRDT sync error handling backend/FwLite/LcmCrdt/RemoteSync/CrdtHttpSyncService.cs, backend/FwLite/LcmCrdt/RemoteSync/CrdtSyncException.cs	GetChanges now returns ApiResponse and checks for errors; throws CrdtSyncException (new type with Step enum) on failures.
Routing and root-location service frontend/viewer/src/App.svelte, frontend/viewer/src/AppRoutes.svelte, frontend/viewer/src/lib/services/root-location-service.ts, frontend/viewer/.storybook/decorators/FWLiteDecorator.svelte	Centralizes routes into AppRoutes; introduces RootLocation context with init/use helpers; initializes in app and Storybook decorator.
Rich-text editor behavior and stomp-safe wrapper frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte, frontend/viewer/src/lib/components/stomp/stomp-safe-lcm-rich-text-editor.svelte, frontend/viewer/src/project/tasks/SubjectPopup.svelte	Adds robust paste cleanup, selection restore, empty backspace handling; introduces commitAnyChanges with idle/location hooks; ensures blur/tick before subject type switch.
Project resource loading and stats timing frontend/viewer/src/lib/project-context.svelte.ts, frontend/viewer/src/lib/project-stats.ts	Ensures initial load when throttled via watchOnce; changes onAdd timing from throttle(3000) to debounce(500).
Sync UI components frontend/viewer/src/project/sync/SyncStatusPrimitive.svelte, frontend/viewer/src/project/sync/SyncArrow.svelte, frontend/viewer/src/project/sync/FwLiteToFwMergeDetails.svelte	Refactors status layout, adds “Last sync,” consolidates controls; reworks SVG sizing with headSize scaling; adjusts arrow size usage (1.5 → 1.25).
Tasks styling frontend/viewer/src/project/tasks/DoneView.svelte	Adds text-primary-foreground class to subject span.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Update Sync dialog to be easier to understand #1971 — Touches the same sync UI components (SyncStatusPrimitive, SyncArrow, merge details).
• Rich-text fixes (Mobile, Enter, Range errors, alignment etc.) #1802 — Edits the same rich-text editor with overlapping behavior fixes.
• Show total entry count in sidebar and filter #1721 — Modifies project-context and project-stats in similar areas.

Suggested reviewers

• hahn-kev
• rmunn

Poem

In routes I hop, from root to leaf,
I sniff out errors—throw relief.
Paste made clean, selections keep,
Arrows shrink and metrics sleep.
Syncs now speak of last-time glows—
Thump-thump goes my reviewer nose. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The title “A swath of UI bug fixes and small improvements” is overly generic and does not capture the primary changes in this pull request, such as the addition of explicit CRDT sync error handling, the new CrdtSyncException type, and the refactored AppRoutes component. By relying on vague terms like “swath” and “small improvements,” it fails to convey key backend and routing modifications. A more specific title would help reviewers quickly understand the main functional impact of this changeset.	Consider revising the title to succinctly reflect the most significant changes—for example, “Add explicit CRDT sync error handling and centralize Svelte routing”—or split distinct concerns into separate pull requests to improve clarity and review focus.
Description Check	❓ Inconclusive	The description “I can’t think of any known UI bugs that this doesn’t cover.” is an off-hand remark that does not summarize the actual changes, as it fails to mention the backend API adjustments, new exception handling, configuration updates, routing refactor, or rich-text editor enhancements included in this pull request. It is too vague to provide meaningful context for reviewers. Without a clear overview, contributors may miss important implementation details and the rationale behind the changes.	Please update the description with a concise summary of the main objectives and changes, listing key features such as CRDT sync error handling, AppRoutes restructuring, and editor fixes, along with any important implementation notes to guide reviewers.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

UI unit Tests

  1 files  ±0   45 suites  ±0   29s ⏱️ -1s
111 tests ±0  111 ✅ ±0  0 💤 ±0  0 ❌ ±0 
160 runs  ±0  160 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 405355a. ± Comparison against base commit 4c03705.

♻️ This comment has been updated with latest results.

[github-actions]

C# Unit Tests

130 tests   130 ✅  19s ⏱️
 20 suites    0 💤
  1 files      0 ❌

Results for commit 405355a.

♻️ This comment has been updated with latest results.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

frontend/viewer/src/project/tasks/SubjectPopup.svelte (1)

67-74: Defensive fix for rich-text handler timing.

The blur/tick pattern ensures change handlers complete before subject processing. This prevents errors from handlers firing too late (e.g., during onDestroy).

Optional refinement: Consider narrowing the blur condition to specifically target rich-text editors:

-    if (document.activeElement instanceof HTMLElement) {
+    if (document.activeElement instanceof HTMLElement && 
+        (document.activeElement.classList.contains('ProseMirror') || 
+         document.activeElement.contentEditable === 'true')) {

This would avoid blurring non-rich-text form elements, though the current broad approach is safer for ensuring all handlers complete.

frontend/viewer/src/AppRoutes.svelte (1)

55-57: Use onMount+tick for fallback redirects
Imperative redirects in svelte-routing should live in a component’s onMount hook – wrap navigate('/', { replace: true }) inside onMount (with await tick() or requestAnimationFrame) rather than inline setTimeout.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c03705 and a5d05d2.

📒 Files selected for processing (16)

• .vscode/settings.json (1 hunks)
• backend/FwLite/LcmCrdt/RemoteSync/CrdtHttpSyncService.cs (2 hunks)
• backend/FwLite/LcmCrdt/RemoteSync/CrdtSyncException.cs (1 hunks)
• frontend/viewer/.storybook/decorators/FWLiteDecorator.svelte (2 hunks)
• frontend/viewer/src/App.svelte (2 hunks)
• frontend/viewer/src/AppRoutes.svelte (1 hunks)
• frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte (6 hunks)
• frontend/viewer/src/lib/components/stomp/stomp-safe-lcm-rich-text-editor.svelte (1 hunks)
• frontend/viewer/src/lib/project-context.svelte.ts (2 hunks)
• frontend/viewer/src/lib/project-stats.ts (1 hunks)
• frontend/viewer/src/lib/services/root-location-service.ts (1 hunks)
• frontend/viewer/src/project/sync/FwLiteToFwMergeDetails.svelte (2 hunks)
• frontend/viewer/src/project/sync/SyncArrow.svelte (1 hunks)
• frontend/viewer/src/project/sync/SyncStatusPrimitive.svelte (1 hunks)
• frontend/viewer/src/project/tasks/DoneView.svelte (1 hunks)
• frontend/viewer/src/project/tasks/SubjectPopup.svelte (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

frontend/viewer/src/lib/project-context.svelte.ts (1)

frontend/viewer/src/lib/dotnet-types/generated-types/FwLiteShared/Services/IMiniLcmJsInvokable.ts (1)

• IMiniLcmJsInvokable (24-69)

backend/FwLite/LcmCrdt/RemoteSync/CrdtHttpSyncService.cs (1)

backend/FwLite/LcmCrdt/RemoteSync/CrdtSyncException.cs (3)

• CrdtSyncException (3-17)
• CrdtSyncException (9-12)
• CrdtSyncException (13-16)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Build API / publish-api
• GitHub Check: Build UI / publish-ui
• GitHub Check: Build FwHeadless / publish-fw-headless
• GitHub Check: check-and-lint
• GitHub Check: frontend-component-unit-tests
• GitHub Check: Build FW Lite and run tests
• GitHub Check: Analyze (csharp)

🔇 Additional comments (21)

.vscode/settings.json (1)

47-49: Appreciate the search noise reduction.

Excluding dist directories from search keeps editor results focused on source files. Looks good.

frontend/viewer/src/project/tasks/DoneView.svelte (1)

53-53: LGTM! Proper contrast pairing added.

Adding text-primary-foreground ensures proper text contrast against the bg-primary background, fixing a potential readability/accessibility issue.

frontend/viewer/src/lib/project-stats.ts (1)

18-18: Good improvement to stats update behavior.

Switching from throttle: 3000 to debounce: 500 improves responsiveness—stats now update 500ms after user activity stops rather than at fixed 3-second intervals. This is more efficient for burst updates and provides faster feedback after editing entries.

frontend/viewer/src/project/tasks/SubjectPopup.svelte (1)

18-18: LGTM!

The tick import is correctly added to support the new async blur/tick pattern in the onNext function.

frontend/viewer/src/lib/components/lcm-rich-text-editor/lcm-rich-text-editor.svelte (5)

2-2: LGTM!

The additional imports (Fragment and Slice) are correctly added to support the new paste handling logic.

---

57-58: LGTM!

Converting dirty to a plain variable avoids timing issues when blur occurs during navigation, as explained in the onblur comment. The logic correctly tracks whether onchange should be called.

---

157-166: LGTM!

The defensive checks and error handling properly address edge cases where the field is cleared or selection restoration fails on older devices. The fallback behavior is sound.

---

178-180: LGTM!

The comment clearly explains the timing constraint that prevents state updates during navigation-triggered blur events. This complements the dirty variable change.

---

230-234: LGTM!

The Backspace handler correctly prevents errors when the document is empty by returning true to indicate the keypress was handled. This is good defensive programming for older devices.

frontend/viewer/src/project/sync/FwLiteToFwMergeDetails.svelte (1)

72-72: LGTM: Size adjustment aligns with SyncArrow refactor.

The reduction from 1.5 to 1.25 is a straightforward visual tuning that follows the updated scaling logic introduced in SyncArrow.svelte.

Also applies to: 81-81

frontend/viewer/.storybook/decorators/FWLiteDecorator.svelte (1)

32-33: LGTM: Storybook integration with root-location service.

Using readable() as a stub RootLocation is appropriate for Storybook, where real routing is not available.

Also applies to: 44-44

frontend/viewer/src/lib/project-context.svelte.ts (1)

133-149: LGTM: Explicit initialization for throttled resources.

The logic ensures that throttled resources load when the API becomes available, avoiding the swallowed-refetch issue described in the comment. The approach handles both immediate and deferred API availability.

Note: Error handling via .catch(console.error) is minimal but aligns with existing patterns in this module.

frontend/viewer/src/project/sync/SyncArrow.svelte (1)

12-19: LGTM: Sizing refactor improves clarity and correctness.

Introducing the headSize constant and explicit scaling logic makes the component more maintainable and removes previous hardcoded adjustments. The SVG preserveAspectRatio attribute ensures correct rendering across scale factors.

Also applies to: 26-26

frontend/viewer/src/App.svelte (1)

5-5: LGTM: Routing centralized into AppRoutes.

Moving routing logic out of App.svelte into a dedicated AppRoutes component improves separation of concerns and maintainability.

Also applies to: 7-7, 17-19

frontend/viewer/src/lib/components/stomp/stomp-safe-lcm-rich-text-editor.svelte (3)

27-30: LGTM: Idle-time commit using untrack.

The use of untrack(commitAnyChanges) correctly invokes the commit function when the user is idle, without creating reactive dependencies inside the effect.

---

31-40: LGTM: Navigation-aware commit handling.

The onDestroy hook ensures pending changes are committed before unmounting and subscribes to location changes to handle navigation-related blur timing issues. The returned unsubscribe function is correctly used for cleanup.

---

44-46: LGTM: Centralized commit logic in onchange handler.

The editor's onchange callback now invokes commitAnyChanges, which consolidates the dirty check and commit logic.

frontend/viewer/src/lib/services/root-location-service.ts (2)

8-18: LGTM: Robust initialization with double-init protection.

The function correctly prevents double initialization by throwing in development and logging a warning in production, ensuring the service is initialized exactly once per context.

---

20-23: LGTM: Fail-fast retrieval of RootLocation.

The function correctly throws if the service is not initialized, ensuring consumers are aware of missing setup.

frontend/viewer/src/AppRoutes.svelte (2)

12-13: LGTM: Global initialization at app startup.

Setting up error handlers and initializing the root location service at the top level of AppRoutes ensures these services are available before any routes are rendered.

---

16-54: LGTM: Route structure with remounting logic.

Using {#key} blocks ensures components are remounted when route parameters change, preventing stale state. The nested Router pattern is appropriate for the routing setup.

[argos-ci]

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	Oct 3, 2025, 3:38 PM