remove a bunch of dead stuff#1729
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change removes a large set of Svelte components, TypeScript modules, and stylesheets from the frontend viewer application. It eliminates collaborative editing, SignalR integration, entry editing, project view, and various UI components, while refactoring some type definitions to be local. Several import statements and UI elements are updated or deleted accordingly. Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
frontend/viewer/src/App.svelte(0 hunks)frontend/viewer/src/CrdtProjectView.svelte(0 hunks)frontend/viewer/src/FwDataProjectView.svelte(0 hunks)frontend/viewer/src/FwDataWebComponent.svelte(0 hunks)frontend/viewer/src/ProjectLoader.svelte(2 hunks)frontend/viewer/src/SvelteUxProjectView.svelte(0 hunks)frontend/viewer/src/app.postcss(0 hunks)frontend/viewer/src/lib/Editor.svelte(0 hunks)frontend/viewer/src/lib/HomeButton.svelte(0 hunks)frontend/viewer/src/lib/RightToolbar.svelte(0 hunks)frontend/viewer/src/lib/commands.ts(0 hunks)frontend/viewer/src/lib/config-data.ts(0 hunks)frontend/viewer/src/lib/config-types.ts(0 hunks)frontend/viewer/src/lib/debug.ts(0 hunks)frontend/viewer/src/lib/entry-editor/EntryOrSensePicker.postcss(0 hunks)frontend/viewer/src/lib/entry-editor/FieldHelpIcon.svelte(2 hunks)frontend/viewer/src/lib/entry-editor/FieldTitle.svelte(0 hunks)frontend/viewer/src/lib/entry-editor/field-editors/FieldEditor.svelte(0 hunks)frontend/viewer/src/lib/entry-editor/field.postcss(0 hunks)frontend/viewer/src/lib/entry-editor/inputs/CrdtField.svelte(0 hunks)frontend/viewer/src/lib/entry-editor/inputs/CrdtTextField.svelte(0 hunks)frontend/viewer/src/lib/entry-editor/object-editors/EntityEditor.svelte(0 hunks)frontend/viewer/src/lib/generated-signalr-client/Lexbox.ClientServer.Hubs.ts(0 hunks)frontend/viewer/src/lib/generated-signalr-client/TypedSignalR.Client/Lexbox.ClientServer.Hubs.ts(0 hunks)frontend/viewer/src/lib/generated-signalr-client/TypedSignalR.Client/index.ts(0 hunks)frontend/viewer/src/lib/history/HistoryView.svelte(1 hunks)frontend/viewer/src/lib/i18n.ts(1 hunks)frontend/viewer/src/lib/layout/AppBarMenu.svelte(0 hunks)frontend/viewer/src/lib/layout/DictionaryEntryViewer.svelte(0 hunks)frontend/viewer/src/lib/layout/EntryList.svelte(0 hunks)frontend/viewer/src/lib/layout/FieldListDialog.svelte(0 hunks)frontend/viewer/src/lib/layout/IndexCharacters.svelte(0 hunks)frontend/viewer/src/lib/layout/Scotty.svelte(0 hunks)frontend/viewer/src/lib/layout/ShowEmptyFieldsSwitch.svelte(0 hunks)frontend/viewer/src/lib/layout/Toc.svelte(0 hunks)frontend/viewer/src/lib/layout/ViewOptionsDrawer.svelte(0 hunks)frontend/viewer/src/lib/search-bar/SearchBar.svelte(0 hunks)frontend/viewer/src/lib/services/event-bus.ts(2 hunks)frontend/viewer/src/lib/services/lexbox-api.ts(0 hunks)frontend/viewer/src/lib/services/selected-entry-service.ts(0 hunks)frontend/viewer/src/lib/services/service-provider-signalr.ts(0 hunks)frontend/viewer/src/lib/status/SaveStatus.svelte(0 hunks)frontend/viewer/src/lib/writing-system-service.svelte.ts(1 hunks)frontend/viewer/src/lib/writing-system/WritingSystemEditor.svelte(2 hunks)frontend/viewer/src/web-component.ts(0 hunks)
💤 Files with no reviewable changes (38)
- frontend/viewer/src/web-component.ts
- frontend/viewer/src/app.postcss
- frontend/viewer/src/lib/HomeButton.svelte
- frontend/viewer/src/App.svelte
- frontend/viewer/src/lib/services/selected-entry-service.ts
- frontend/viewer/src/lib/generated-signalr-client/Lexbox.ClientServer.Hubs.ts
- frontend/viewer/src/lib/layout/ShowEmptyFieldsSwitch.svelte
- frontend/viewer/src/CrdtProjectView.svelte
- frontend/viewer/src/lib/debug.ts
- frontend/viewer/src/lib/layout/IndexCharacters.svelte
- frontend/viewer/src/lib/entry-editor/field.postcss
- frontend/viewer/src/lib/services/lexbox-api.ts
- frontend/viewer/src/lib/entry-editor/object-editors/EntityEditor.svelte
- frontend/viewer/src/lib/commands.ts
- frontend/viewer/src/lib/entry-editor/EntryOrSensePicker.postcss
- frontend/viewer/src/lib/layout/Toc.svelte
- frontend/viewer/src/lib/layout/AppBarMenu.svelte
- frontend/viewer/src/lib/entry-editor/FieldTitle.svelte
- frontend/viewer/src/lib/status/SaveStatus.svelte
- frontend/viewer/src/FwDataProjectView.svelte
- frontend/viewer/src/lib/search-bar/SearchBar.svelte
- frontend/viewer/src/lib/entry-editor/inputs/CrdtTextField.svelte
- frontend/viewer/src/lib/entry-editor/field-editors/FieldEditor.svelte
- frontend/viewer/src/lib/layout/ViewOptionsDrawer.svelte
- frontend/viewer/src/lib/services/service-provider-signalr.ts
- frontend/viewer/src/lib/generated-signalr-client/TypedSignalR.Client/Lexbox.ClientServer.Hubs.ts
- frontend/viewer/src/lib/RightToolbar.svelte
- frontend/viewer/src/lib/Editor.svelte
- frontend/viewer/src/lib/layout/EntryList.svelte
- frontend/viewer/src/lib/layout/FieldListDialog.svelte
- frontend/viewer/src/lib/config-data.ts
- frontend/viewer/src/FwDataWebComponent.svelte
- frontend/viewer/src/lib/entry-editor/inputs/CrdtField.svelte
- frontend/viewer/src/lib/config-types.ts
- frontend/viewer/src/lib/layout/Scotty.svelte
- frontend/viewer/src/SvelteUxProjectView.svelte
- frontend/viewer/src/lib/layout/DictionaryEntryViewer.svelte
- frontend/viewer/src/lib/generated-signalr-client/TypedSignalR.Client/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/viewer/src/lib/i18n.ts (4)
frontend/viewer/src/lib/entry-editor/field-data.ts (1)
FieldIds(36-36)frontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/IEntry.ts (1)
IEntry(14-27)frontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/ISense.ts (1)
ISense(13-24)frontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/IExampleSentence.ts (1)
IExampleSentence(10-18)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build API / publish-api
- GitHub Check: Build UI / publish-ui
- GitHub Check: Analyze (csharp)
- GitHub Check: check-and-lint
- GitHub Check: frontend
- GitHub Check: Build FW Lite and run tests
🔇 Additional comments (12)
frontend/viewer/src/lib/history/HistoryView.svelte (2)
88-94: LGTM: Author display logic is correct.The refactoring of the author display with proper conditional rendering looks good and maintains the same functionality.
99-99:Details
✅ Verification successful
Verify the class application changes.
The AI summary mentions that this container div "no longer conditionally applies the
hide-unusedCSS class based on this state", but I don't see evidence of such conditional class logic in the current code. Please confirm this change was implemented correctly.
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining references to showEmptyFields or hide-unused class # Expected: No references to showEmptyFields variable or hide-unused conditional class application echo "Searching for showEmptyFields references:" rg -n "showEmptyFields" echo -e "\nSearching for hide-unused class references:" rg -n "hide-unused" echo -e "\nSearching for ShowEmptyFieldsSwitch references:" rg -n "ShowEmptyFieldsSwitch"Length of output: 1263
Conditional hide-unused class removal in HistoryView confirmed
TheHistoryView.sveltefile no longer applies thehide-unusedclass or uses theshowEmptyFieldsstate—those lines (including the<ShowEmptyFieldsSwitch>) have been removed or commented out. The only remaining conditional use ofhide-unusedis inEntryView.svelte. No further changes are needed here.frontend/viewer/src/lib/services/event-bus.ts (1)
13-16: LGTM! Clean refactoring of CloseReason enum.The local definition appropriately replaces the external SignalR dependency while maintaining the same enum values and functionality. This change aligns well with the broader removal of SignalR-related code.
frontend/viewer/src/lib/i18n.ts (1)
2-2:Details
✅ Verification successful
Good refactoring approach - derive types from domain model.
This change replaces the external config dependency with a type derived directly from the domain interfaces, which is more maintainable and less prone to drift.
However, please verify that the new type definition produces the expected field IDs:
Also applies to: 10-10
🏁 Script executed:
#!/bin/bash # Verify the WellKnownFieldId type derivation produces expected results # Check if there are other files importing from config-types that might be affected # Search for any remaining imports from config-types rg "from.*config-types" --type ts # Check for WellKnownFieldId usage to ensure compatibility rg "WellKnownFieldId" --type ts -A 2 -B 2Length of output: 1135
Verification complete:
config-typesimports removed &WellKnownFieldIdcorrectly derived
- No remaining imports from
config-typesin the codebase.WellKnownFieldIdis defined as
Exclude<keyof (IEntry & ISense & IExampleSentence), 'id' | 'exampleSentences' | 'senses'>
and is used consistently indefaultI18nand thefieldNamehelper.- No compatibility issues detected with existing usages of
WellKnownFieldId.All looks good—approving these changes as-is.
frontend/viewer/src/lib/writing-system-service.svelte.ts (1)
15-20: Excellent type definitions using template literals.The local type definitions are well-structured and comprehensive. The use of template literal types for
first-${WritingSystemType}provides excellent type safety while maintaining flexibility. The types align perfectly with the switch statement logic inpickWritingSystems.frontend/viewer/src/ProjectLoader.svelte (2)
17-17: Confirm styling equivalence with the new Loading component.The switch from
<ProgressCircle />to<Loading class="size-14"/>maintains the loading indicator functionality while using local components.Ensure that the
size-14class provides visually similar sizing to the previous ProgressCircle component.
4-4:Details
✅ Verification successful
Verify local Loading component exists and functions correctly.
The migration from
svelte-uxProgressCircle to a local Loading component aligns with the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify the Loading component exists and check its implementation fd -t f "Loading.svelte" --exec cat {} \;Length of output: 413
Local Loading component verified and ready to merge.
src/lib/components/Loading.svelteexists and imports theIconfrom$lib/components/ui/icon.- It renders
<Icon icon="i-mdi-loading" class={cn('animate-spin', className)} {...restProps} />, providing the same spinning loader behavior as the originalProgressCircle.All checks pass—approving the migration.
frontend/viewer/src/lib/writing-system/WritingSystemEditor.svelte (3)
5-5: Button import migration looks good.The migration from
svelte-uxto local UI components aligns with the PR objectives.
38-38: 🛠️ Refactor suggestionEvent handler changed from Svelte to standard HTML.
The change from
on:clicktoonclickmoves from Svelte's reactive event handling to standard HTML event handling, which may affect event propagation and handling.Consider keeping Svelte's event handling pattern for consistency:
- <Button variant="outline" onclick={createNew}>Create new Writing System</Button> + <Button variant="outline" on:click={createNew}>Create new Writing System</Button>⛔ Skipped due to learnings
Learnt from: hahn-kev PR: sillsdev/languageforge-lexbox#1612 File: frontend/viewer/src/lib/entry-editor/DeleteDialog.svelte:39-52 Timestamp: 2025-04-18T10:33:51.961Z Learning: Svelte 5 uses standard HTML attribute syntax for event handlers (e.g., `onclick={handler}`) instead of the Svelte-specific directive syntax (e.g., `on:click={handler}`) used in previous versions.Learnt from: hahn-kev PR: sillsdev/languageforge-lexbox#1612 File: frontend/viewer/src/lib/entry-editor/DeleteDialog.svelte:39-52 Timestamp: 2025-04-18T10:33:51.961Z Learning: Svelte 5 uses standard HTML attribute syntax for event handlers (e.g., `onclick={handler}`) instead of the Svelte-specific directive syntax (e.g., `on:click={handler}`) used in previous versions.
32-36:Details
❓ Verification inconclusive
Form inputs are completely disabled - verify this is intentional.
All
CrdtTextFieldcomponents are commented out, which effectively disables the entire form functionality. This makes the component non-functional for editing writing system properties.Is this intentional as part of removing CRDT functionality, or should these be replaced with alternative input components?
🏁 Script executed:
#!/bin/bash # Check if CrdtTextField component still exists or has been removed fd -t f "CrdtTextField.svelte" # Check for any alternative input components that might replace CrdtTextField fd -t f "*TextField*.svelte" --exec echo "Found: {}" \;Length of output: 387
🏁 Script executed:
#!/bin/bash # Properly search for the CrdtTextField component file fd --glob 'CrdtTextField.svelte' # Search for any other TextField-like components by filename fd --glob '*TextField*.svelte' # Search for any references to CrdtTextField in the codebase rg -n 'CrdtTextField'Length of output: 832
Verify intentional disabling of writing system form inputs
All
<CrdtTextField>references in frontend/viewer/src/lib/writing-system/WritingSystemEditor.svelte (lines 32–36) are commented out, and noCrdtTextField.svelteor alternative*TextField*.sveltecomponent exists in the repo. This leaves the editor form completely non-functional.• Confirm whether removing CRDT-based inputs is intentional (e.g., pending replacement).
• If not, re-enable these inputs or replace them with the intended form components.frontend/viewer/src/lib/entry-editor/FieldHelpIcon.svelte (2)
14-14: Icon API migration looks correct.The change from
dataandsizeprops toiconstring identifier ("i-mdi-help-circle-outline") and CSS classes ("size-3 ml-1 absolute") maintains the same visual result with the new component API.Verify that the local Icon component recognizes the
"i-mdi-help-circle-outline"format and renders the same help circle icon.
2-2:Details
✅ Verification successful
Clean consolidation of icon imports.
The migration from separate
@mdi/jsandsvelte-uximports to a single local UI icon component simplifies dependencies and aligns with the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify the local Icon component exists and supports MDI icons fd -t f -e svelte -e ts "icon" | head -5 | xargs catLength of output: 3362
Verified MDI support in local Icon component
The
Iconcomponent’sIconStringtype and CSS-class implementation align with the Iconify MDI spec (https://icon-sets.iconify.design/mdi/), confirming full compatibility. No further changes needed.
|
The field help icon links got nudged up by a few pixels 😆. What an intriguing change. |
hahn-kev
added
the
💻 FW Lite
2 participants
The following still use svelte-ux and are less trivial to replace