Finalize shadcn buttons#1670
Conversation
myieye
commented
May 15, 2025
myieye
commented
|
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 update refactors button components in the home view, improves dialog state synchronization with URL query parameters, and enhances sidebar menu conditionality and developer-only content. It also updates background styling for search filters and corrects a dialog state check. No changes were made to exported or public entity declarations except for the troubleshooting dialog’s prop handling. 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 (
|
UI unit Tests12 tests 12 ✅ 0s ⏱️ Results for commit 7036cd8. |
C# Unit Tests116 tests 116 ✅ 10s ⏱️ Results for commit 7036cd8. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/viewer/src/lib/troubleshoot/TroubleshootDialog.svelte (1)
8-27: Refactored dialog state management with URL parameter synchronization.The dialog's open state is now synchronized with URL query parameters, enabling better state management and back-navigation support. This approach is consistent with the pattern used in other dialogs like DeleteDialog.
The bidirectional watchers approach addresses a technical requirement, although as noted in a previous review, this dual-watch pattern introduces some complexity. Consider documenting this pattern if it becomes a common approach across components.
🧹 Nitpick comments (4)
frontend/viewer/src/project/ProjectSidebar.svelte (1)
70-77: Developer-only UI sections.Wrapped several menu items in
DevContentcomponents to restrict visibility to developer mode, improving the user experience for regular users while maintaining access for developers.However, consider adding conditional rendering for empty groups if all their children are developer-only to avoid empty UI sections in production mode.
Also applies to: 82-114
frontend/viewer/src/home/HomeView.svelte (3)
170-191: Consider updating remaining UxButton components for consistencyWhile the main action buttons have been updated to use the shadcn Button component, several UxButton components remain in the UI (lines 170-179, 180, 190). For a consistent UI experience, consider updating these as well to the new Button component.
- <UxButton - icon={mdiDelete} - title={$t`Delete`} - class="p-2 hover:bg-primary/20" - on:click={(e) => { - e.preventDefault(); - void deleteProject(project); - }} - /> + <Button + icon="i-mdi-delete" + title={$t`Delete`} + size="icon" + variant="ghost" + onclick={(e) => { + e.preventDefault(); + void deleteProject(project); + }} + />Similarly for other UxButton instances:
- <UxButton icon={mdiChevronRight} class="p-2 pointer-events-none" /> + <Button icon="i-mdi-chevron-right" size="icon" variant="ghost" class="pointer-events-none" />
229-240: Consider updating the Import button to shadcn styleFor consistency with other updated buttons, consider migrating this Import button to the new Button component style. This would complete the transition to shadcn buttons throughout the component.
- <UxButton - loading={importing === project.name} - icon={mdiBookArrowLeftOutline} - title={$t`Import`} - disabled={!!importing} - on:click={async (e) => { - e.preventDefault(); - await importFwDataProject(project.name); - }} - class="hover:bg-primary/20" - ></UxButton> + <Button + loading={importing === project.name} + icon="i-mdi-book-arrow-left-outline" + title={$t`Import`} + disabled={!!importing} + variant="ghost" + size="icon" + onclick={async (e) => { + e.preventDefault(); + await importFwDataProject(project.name); + }} + />
29-29: Consider removing unused icon importsNow that the buttons are using string-based icon names (e.g., "i-mdi-chat-question") instead of SVG path imports, some icon imports from @mdi/js may no longer be needed. Consider reviewing and removing any unused imports to clean up the code.
Also applies to: 9-10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (5)
frontend/viewer/src/home/HomeView.svelte(1 hunks)frontend/viewer/src/lib/entry-editor/DeleteDialog.svelte(1 hunks)frontend/viewer/src/lib/troubleshoot/TroubleshootDialog.svelte(2 hunks)frontend/viewer/src/project/ProjectSidebar.svelte(4 hunks)frontend/viewer/src/project/browse/SearchFilter.svelte(1 hunks)
🔇 Additional comments (10)
frontend/viewer/src/project/browse/SearchFilter.svelte (1)
44-44: Clean styling update to use modern utility classes.The change from the custom CSS variable
bg-[hsl(var(--sidebar-background))]to the utility classbg-muted/50improves consistency with the shadcn design system. The semi-transparent background (50% opacity) provides a subtle visual distinction when filters are expanded.frontend/viewer/src/lib/troubleshoot/TroubleshootDialog.svelte (1)
40-41: Dialog binding and responsive styling update.The dialog now correctly binds to the query parameter state, and the added
sm:min-h-fitclass improves the dialog's responsiveness on small screens by allowing it to adjust its height based on content.frontend/viewer/src/project/ProjectSidebar.svelte (4)
8-8: Added imports for troubleshooting and developer features.Good organization of imports for the new troubleshooting functionality and developer-only content components.
Also applies to: 15-16
38-40: Added troubleshooting service and dialog state.Clean implementation of the troubleshooting service hook and dialog state, following the pattern used elsewhere in the application.
129-137: Conditional troubleshooting UI.Good implementation of conditional rendering for the troubleshooting feature based on service availability. The dialog is properly bound to the local state variable.
140-145: Improved Feedback link implementation.The snippet pattern properly forwards props to the anchor tag, ensuring correct handling of the external link while maintaining the sidebar menu styling. Good use of the target="_blank" attribute for external links.
frontend/viewer/src/home/HomeView.svelte (4)
113-118: Good implementation of the new shadcn Button component for the Feedback buttonThe conversion from UxButton to the new shadcn Button component is correctly implemented with appropriate properties. The icon path import has been replaced with the string-based icon name format, and the variant has been properly set to "ghost" which matches the shadcn design system.
120-127: Well-structured Troubleshoot button implementation with improved content structureThe button has been properly updated to use the shadcn design system with variant "ghost". The content structure is improved by using the button's children for the text label instead of relying on props, which is more semantically correct and aligned with shadcn's component patterns.
131-131: Clean conversion of Sandbox button to shadcn styleThe Sandbox button has been successfully migrated to use the new Button component with appropriate icon string format and variant.
149-154: Properly implemented Refresh button with correct shadcn propertiesThe Refresh button correctly uses the "icon" size property for a compact icon-only button and has the appropriate variant. The onclick handler is correctly implemented to call the refreshProjects function.
|
The Playwright test failures look like the flaky ones that were fixed on |
|
More screenshots 🤓
"Show empty fields" is now gone 😉 Bigger radio button click area on mobile (there is no space between the radio butons that is not clickable/tappable) |
|
Looks good Tim, I just pushed a change to hide the open in FLEx button when it's not a FLEx project. |
* wire up project picker * show the current project as selected in the dropdown * Playwright screenshots for fw lite ui (#1590) * setup screenshot testing using playwright * ensure UI is consistent for tests by removing date from dev version and by not changing the sync icon --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * change index.html in viewer so when you open it you're taken to /testing/project-view * Migrate dialogs to shadcn (#1612) * use shadCn dialog for NewEntryDialog * Refactor DeleteDialog to use the ShadCN dialog * use ShadCN dialog in ActivityView.svelte and HistoryView.svelte * convert About and Troubleshooting Dialog to use shadCN --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * Use runes for project services (#1593) * change writing system service to use runes and the new project context. * use project context for history service * fix proxy issues due to some rune and resource stuff * setup parts of speech service * migrate complex form types to use runes * run CI on PRs for shadcn main * fix up viewer to work with new project context * fix up viewer-specific lint errors by using project context for tests * Refactor editor classes to editor components (#1619) * Refactor editor classes to editor components * Extract editor sandbox into own component * Fix invalid binding * Introduce editor root and name editor container --------- Co-authored-by: Kevin Hahn <kevin_hahn@sil.org> * Migrate entry select dialog (#1620) * use a boundry on failing areas in the sandbox. Tweak index.html to only send to testing when the path is empty * convert EntryOrSensePicker.svelte to svelte 5 and ShadCN * use a ring to indicate selection of entry rows * move new entry dialog and delete dialog to use a new dialog service based on project context * migrate save handler * pull Add entry button out of sidebar and create SidebarPrimaryAction.svelte which will place a button in the sidebar with context of the current view. * Fix stacking dialogs * Fix selecting newly created entry --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * Shadcn Single-select (#1623) * Fix sandbox * Make Editor.Field.Body only 1 column by default * Add note to sandbox editor * Remove min-height from multi-select on desktop * Add single-select component to sandbox editor * Remove badge from empty single-select * Incorporate PR feedback * Make rich-text-editor support readonly mode (#1624) * Make rich-text-editor support readonly mode * add a checkbox to toggle readonly mode on sandbox. Wrap the rich editor with label in a div otherwise the cursor is incorrect when hovering over the editor. --------- Co-authored-by: Kevin Hahn <kevin_hahn@sil.org> * Ensure user can tab to readonly rich-text just like other inputs (#1638) * Fix entry view and filter outline overflow (#1639) * Fix entry-view scroll-area overflowing parent * Prevent new-entry-button from being covered up by sidebar menu items * Fix filter-field outline overflowing and tweak padding/margins * Make refresh button more distinct and animation less jittery * Use icon button size for entry-view icon buttons * Add gap between entry-view icon buttons * WS single and multi inputs (#1637) * Add radio button comment for our future selves * Add multi-ws-input and simple inputs to sandbox editor * Fix up readonly field-editor props * Put value preview beside sandbox editor * hook up delete buttons in new project view (#1635) * add new EntryDeletedEvent and notify on delete from sync or the notify wrapper * introduce a project event bus use the bus for entry deleted events * document how to use shadcn * add a right click menu to the entries list to enable deleting entries * add open in new to entry menu * use entry menu for entry list context menu closes #1640 --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * display a dictionary preview on the entry list (#1641) * display a dictionary preview on the entry list * make sense numbers links in dictionary preview * show dictionary preview in the entry view * allow making the preview sticky --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * update bits ui to fix issue on popup close * Tweak muted backgrounds (#1647) * 1636 editor layout (#1654) * Introduce EntryEditorPrimitive * Rename SenseEditor to SenseEditorPrimitive. That's what it is. * Rename ExampleEditor to ExampleEditorPrimitive * Replace grid-layer class with EditorSubGrid component * Add onchange events to fields * Truncate selects to first 100 filtered items with hint at end of truncated list * Introduce view-text helper/type for lite vs classic texts * Add readonly dev toggle. * Wrap editor in Editor.Root * Migrate sense fields to shadcn * Migrate entry fields to shadcn * Migrate example sentence fields to shadcn * Some clean up * Extend pickViewText api and translate editor headers * Silence warnings by binding entry objects * Fix field borders peeking around side of sticky header at some zoom-levels. * Fix matching projects (#1653) * Fix can't open fwdata projects if crdt project with same code exists * Reuse Lexbox project name for fwdata projects * Fix crdt copy not shown in project-picker if fwdata copy exists. And sort. * Fix crdt and fwdata copies always highlighted together on hover * Fix current fwdata project not highlighted in project-picker if crdt copy also exists * Fix both crdt and fwdata copies of current project highlighted in project-picker * Fix: project code not shown on synced server projects * Migrate components, complex forms and sense and example sentence actions (#1655) * Make icon and icon buttons support baseline alignment * Migrate component and complex-forms to shadcn/Svelte 5 * Prevent focus rings from being clipped and tweak preview position on mobile * Migrate rest of editor to Svelte 5 and shadcn * Remove reorderer-swapper and use svelte-context in reorderr hierarchy * Make new sense FAB look like new entry FAB * Rehide reorderer if only 1 item * Move pickIcon into the only component that now uses it * write a sandbox test for the reorderer * translate delete prompt text * Use getters instead of $state so root-props react to updates. * Use simple $derived.by instead of watch --------- Co-authored-by: Kevin Hahn <kevin_hahn@sil.org> * Url state tracking (ShadCN) (#1662) * create QueryParamState to enable reactive url params * fix bug in EntryOrSenseItemList.svelte to remove a trailing `}` from the query parameter * make the browse view `selectedEntryId` reactive based on query params * make dialogs reactive to url and the back button * use router for handling browse and tasks views * add a fallback default route in the project view --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * hide audio writing systems since we don't support them * fix linting * remove svelte ux from tailwind (#1664) * add a border to an empty server list * migrate LocalizationPicker.svelte to use Shad CN and not the Svelte UX menu * slide icons and loading indicator on change * remove svelte-ux on sandbox page --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * fix icon button styles, add a sandbox which shows button configurations * Don't run install 3 times (#1669) * cleanup lint errors * fix DictionaryEntry.svelte links * show history when clicking the history button on an entry * Finalize shadcn buttons (#1670) * Fix delete dialog close detection * Wire up sidebar and hide unimplemented content * Migrate home-page app-bar buttons to shadcn * Use function instead of prop to open troubleshooting dialog * Fix broken enter animation due to troubleshooting dialog resize after open. * Introduce responsive-menu for entry-menu * Add Open in FieldWorks button * Fix logo doesn't respond to dark-mode * Condense home-view app-bar * Make radio buttons bigger on mobile * only show Open In Flex button when supported --------- Co-authored-by: Kevin Hahn <kevin_hahn@sil.org> * fix lint error * fix more lint errors * Persist changes in EntryView.svelte (#1671) * notify entry changes when sense or example are deleted * make entry persistence service to attach to entry editor * Use project-code for detecting project events * Fix entry ID mismatch after changing entries --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org> * Setup back handler (#1673) * add QueryParamState option for using replace when a default value is set * replace dialogs using QueryParamState with `useBackHandler` * use back handler in drawer on mobile. Cleanup back handler when it's component is destroyed * handle case where we navigate from an open dialog and warn that history was not cleaned up * Make svelte-check and lint happy --------- Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org>
3 participants
