-
Notifications
You must be signed in to change notification settings - Fork 0
feat: migrate header, sidebar, maincontent inset to shadcn components #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds a large UI component set (breadcrumb, dropdown-menu, sheet, sidebar, tooltip, input/label/separator/skeleton), sidebar state/context utilities, replaces old shared Sidebar with SidebarProvider-based layout, and expands auto-import/type declaration files to expose new globals and typings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App / Layout
participant SidebarProv as SidebarProvider
participant UseSidebar as useSidebar (hook)
participant Sidebar as Sidebar (UI)
participant SidebarBtn as SidebarMenuButton / Trigger
participant Tooltip as TooltipProvider/Tooltip
App->>SidebarProv: mount (props: defaultOpen/open)
SidebarProv->>UseSidebar: provideSidebarContext(state, open, toggles)
App->>Sidebar: render (consumes context)
Sidebar->>UseSidebar: read isMobile, state, openMobile
SidebarBtn->>UseSidebar: toggleSidebar() --(user click)-->
UseSidebar-->>SidebarProv: update open/openMobile and emit update:open
SidebarProv->>App: emits update:open (v-model sync)
Note over SidebarBtn,Tooltip: If tooltip prop present and state='collapsed' and !isMobile\nTooltipTrigger shows TooltipContent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (26)
frontend/src/components/ui/input/Input.vue (1)
12-12: Use object syntax for type-safe emits.The callable emit syntax is deprecated in Vue 3. Consider using the object syntax for better type safety and future compatibility.
Apply this diff:
-const emits = defineEmits<(e: 'update:modelValue', payload: string | number) => void>() +const emits = defineEmits<{ + 'update:modelValue': [payload: string | number] +}>()frontend/src/shared/components/Header.vue (4)
15-28: Keep breadcrumb nav pure; move the Back action outside the Breadcrumb container.The Back button is inside the Breadcrumb (which is a nav). Non-crumb actions shouldn’t live inside the breadcrumb landmark.
- <Breadcrumb v-if="isProjectView" class="h-16 flex items-center justify-between p-4"> - <BreadcrumbList> - <BreadcrumbItem class="text-lg font-semibold text-gray-900"> - {{ projectName || 'Loading Project...' }} - </BreadcrumbItem> - <BreadcrumbItem class="text-sm text-gray-600">{{ totalRecords }} rows</BreadcrumbItem> - </BreadcrumbList> - <router-link :to="{ name: 'open' }"> - <Button variant="outline"> - <LucideMoveLeft /> - Back to Projects - </Button> - </router-link> - </Breadcrumb> + <div v-if="isProjectView" class="h-16 flex items-center justify-between p-4"> + <Breadcrumb> + <BreadcrumbList> + <!-- see next comment for Link/Page + tokens --> + </BreadcrumbList> + </Breadcrumb> + <router-link :to="{ name: 'open' }" class="shrink-0"> + <Button variant="outline"> + <LucideMoveLeft class="mr-2 h-4 w-4" aria-hidden="true" /> + Back to Projects + </Button> + </router-link> + </div>
16-21: Use BreadcrumbLink/BreadcrumbPage and move “rows” out of the breadcrumb; switch to semantic color tokens.
- Use BreadcrumbPage for the current page, BreadcrumbLink for ancestors.
- “rows” is metadata, not a crumb—render it outside the list.
- Prefer text-foreground/text-muted-foreground over raw grays for theme support.
- <BreadcrumbList> - <BreadcrumbItem class="text-lg font-semibold text-gray-900"> - {{ projectName || 'Loading Project...' }} - </BreadcrumbItem> - <BreadcrumbItem class="text-sm text-gray-600">{{ totalRecords }} rows</BreadcrumbItem> - </BreadcrumbList> + <BreadcrumbList> + <BreadcrumbItem> + <BreadcrumbLink :to="{ name: 'open' }">Projects</BreadcrumbLink> + </BreadcrumbItem> + <BreadcrumbSeparator /> + <BreadcrumbItem> + <BreadcrumbPage class="text-lg font-semibold text-foreground"> + <template v-if="projectName">{{ projectName }}</template> + <template v-else><Skeleton class="h-5 w-40" /></template> + </BreadcrumbPage> + </BreadcrumbItem> + </BreadcrumbList> + <span class="ml-4 text-sm text-muted-foreground">{{ totalRecords }} row{{ totalRecords === 1 ? '' : 's' }}</span>
24-25: Minor icon a11y/styling.Add sizing/margin and hide decorative icon from SR.
- <LucideMoveLeft /> + <LucideMoveLeft class="mr-2 h-4 w-4" aria-hidden="true" />
31-41: Align non-project breadcrumb with Link/Page semantics.Make “Projects” a link and “All Projects” the current page.
- <Breadcrumb v-else class="h-16 flex items-center p-4"> - <BreadcrumbList> - <BreadcrumbItem> - Projects - </BreadcrumbItem> - <BreadcrumbSeparator /> - <BreadcrumbItem> - All Projects - </BreadcrumbItem> - </BreadcrumbList> - </Breadcrumb> + <Breadcrumb v-else class="h-16 flex items-center p-4"> + <BreadcrumbList> + <BreadcrumbItem> + <BreadcrumbLink :to="{ name: 'open' }">Projects</BreadcrumbLink> + </BreadcrumbItem> + <BreadcrumbSeparator /> + <BreadcrumbItem> + <BreadcrumbPage>All Projects</BreadcrumbPage> + </BreadcrumbItem> + </BreadcrumbList> + </Breadcrumb>frontend/src/components/ui/skeleton/Skeleton.vue (1)
1-16: Prefer theme token + a11y + component name.Use bg-muted for theme resilience, hide decorative skeleton from SR, and name the component for devtools.
<script setup lang="ts"> import { cn } from '@frontend/lib/utils' import type { HTMLAttributes } from 'vue' +defineOptions({ name: 'Skeleton' }) + interface SkeletonProps { class?: HTMLAttributes['class'] } const props = defineProps<SkeletonProps>() </script> <template> <div data-slot="skeleton" - :class="cn('animate-pulse rounded-md bg-primary/10', props.class)" + aria-hidden="true" + :class="cn('animate-pulse rounded-md bg-muted', props.class)" /> </template>frontend/src/components/ui/sheet/SheetClose.vue (1)
1-15: Name the component and explicitly forward attrs.Minor hardening: add a devtools name and forward $attrs alongside props.
<script setup lang="ts"> import type { DialogCloseProps } from 'reka-ui' import { DialogClose } from 'reka-ui' +defineOptions({ name: 'SheetClose' }) + const props = defineProps<DialogCloseProps>() </script> <template> <DialogClose data-slot="sheet-close" - v-bind="props" + v-bind="props" + v-bind="$attrs" > <slot /> </DialogClose> </template>frontend/src/components/ui/sidebar/SidebarMenuBadge.vue (2)
1-8: Add a component name for devtools consistency.<script setup lang="ts"> import { cn } from '@frontend/lib/utils' import type { HTMLAttributes } from 'vue' +defineOptions({ name: 'SidebarMenuBadge' }) + const props = defineProps<{ class?: HTMLAttributes['class'] }>() </script>
15-21: Tailwind class: replace nonstandard min-w-5 with an arbitrary value.min-w-5 isn’t in Tailwind’s default scale; use min-w-[1.25rem] to avoid relying on theme extension.
- 'text-sidebar-foreground pointer-events-none absolute right-1 flex h-5 min-w-5 items-center justify-center rounded-md px-1 text-xs font-medium tabular-nums select-none', + 'text-sidebar-foreground pointer-events-none absolute right-1 flex h-5 min-w-[1.25rem] items-center justify-center rounded-md px-1 text-xs font-medium tabular-nums select-none',frontend/src/components/ui/tooltip/TooltipContent.vue (1)
30-30: Consider extracting the long class string for better readability.The class string on line 30 contains many Tailwind utility classes that are difficult to review and maintain on a single line. While functionally correct, consider extracting this to a computed property or constant with logical grouping (base styles, animations, positioning, etc.).
Example refactor:
+const tooltipClasses = computed(() => cn( + // Base styles + 'bg-primary text-primary-foreground z-50 w-fit rounded-md px-3 py-1.5 text-xs text-balance', + // Animations + 'animate-in fade-in-0 zoom-in-95', + 'data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95', + // Slide directions + 'data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2', + 'data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2', + props.class +))Then use in template:
- :class="cn('bg-primary text-primary-foreground animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 w-fit rounded-md px-3 py-1.5 text-xs text-balance', props.class)" + :class="tooltipClasses"frontend/src/components/ui/dropdown-menu/DropdownMenuShortcut.vue (1)
11-16: Hide purely visual shortcuts from screen readers.Add aria-hidden so SRs don’t announce duplicate shortcut text.
<span data-slot="dropdown-menu-shortcut" - :class="cn('text-muted-foreground ml-auto text-xs tracking-widest', props.class)" + aria-hidden="true" + :class="cn('text-muted-foreground ml-auto text-xs tracking-widest', props.class)" >frontend/src/components/ui/sheet/SheetFooter.vue (1)
9-15: Use semantic footer and tidy class binding.Minor polish: use a semantic element and remove stray newline.
- <div + <footer data-slot="sheet-footer" - :class="cn('mt-auto flex flex-col gap-2 p-4', props.class) - " + :class="cn('mt-auto flex flex-col gap-2 p-4', props.class)" > <slot /> - </div> + </footer>frontend/src/components/ui/sidebar/SidebarContent.vue (1)
11-16: Prevent scroll chaining inside the sidebar.Add overscroll containment to avoid bounce to page when reaching scroll edges.
- :class="cn('flex min-h-0 flex-1 flex-col gap-2 overflow-auto group-data-[collapsible=icon]:overflow-hidden', props.class)" + :class="cn('flex min-h-0 flex-1 flex-col gap-2 overflow-auto overscroll-y-contain group-data-[collapsible=icon]:overflow-hidden', props.class)"frontend/src/core/layouts/DefaultLayout.vue (3)
36-48: Make active-route detection robust.endsWith fails for nested paths and trailing slashes. Prefer exact or prefix match, or a router-provided isActive.
- :is-active="route.path.endsWith(item.to)" + :is-active="route.path === item.to || route.path.startsWith(item.to + '/')"Alternatively, use router-link’s slot:
<router-link :to="item.to" v-slot="{ isActive }"> <SidebarMenuButton :is-active="isActive" as-child size="lg"> <!-- link content --> </SidebarMenuButton> </router-link>
27-54: Consider v-show to retain Sidebar DOM for layout vars/transitions.If SidebarInset relies on CSS variables or layout measurements from Sidebar, toggling Sidebar with v-if can cause layout jumps. v-show keeps it in the DOM while hiding it.
- <Sidebar v-if="showSidebar"> + <Sidebar v-show="showSidebar">
57-65: Avoid shadowing the outer route var for clarity.Rename the slot’s route binding to prevent confusion with the outer route constant.
- <router-view v-slot="{ Component, route }"> + <router-view v-slot="{ Component, route: viewRoute }"> <transition - :key="route.name" + :key="viewRoute.name" name="fade" mode="out-in" > <component :is="Component" /> </transition> </router-view>frontend/src/components/ui/tooltip/TooltipTrigger.vue (1)
1-15: Forward emits and supportclassfor consistency with other wrappers.Current wrapper only v-binds props; typed event forwarding and optional
classpassthrough improve DX and align with your DropdownMenu wrappers.Apply this diff:
<script setup lang="ts"> -import type { TooltipTriggerProps } from 'reka-ui' -import { TooltipTrigger } from 'reka-ui' +import type { TooltipTriggerProps, TooltipTriggerEmits } from 'reka-ui' +import { TooltipTrigger, useForwardPropsEmits } from 'reka-ui' +import { reactiveOmit } from '@vueuse/core' +import type { HTMLAttributes } from 'vue' -const props = defineProps<TooltipTriggerProps>() +const props = defineProps<TooltipTriggerProps & { class?: HTMLAttributes['class'] }>() +const emits = defineEmits<TooltipTriggerEmits>() +const delegated = reactiveOmit(props, 'class') +const forwarded = useForwardPropsEmits(delegated, emits) </script> <template> <TooltipTrigger data-slot="tooltip-trigger" - v-bind="props" + v-bind="forwarded" + :class="props.class" > <slot /> </TooltipTrigger> </template>If
TooltipTriggerEmitsisn’t exported by reka-ui, usedefineEmits<Record<string, any>>()temporarily or forward listeners viav-bind="$attrs"and setinheritAttrs: falseexplicitly for clarity.frontend/src/components/ui/sidebar/SidebarGroupLabel.vue (1)
1-27: Unify prop forwarding withreactiveOmitandv-bindfor parity with other UI wrappers.Use the delegated-props pattern (like your DropdownMenu components) to forward all PrimitiveProps while keeping class composition local.
Apply this diff:
<script setup lang="ts"> import { cn } from '@frontend/lib/utils' import type { PrimitiveProps } from 'reka-ui' import { Primitive } from 'reka-ui' import type { HTMLAttributes } from 'vue' +import { reactiveOmit } from '@vueuse/core' const props = defineProps< PrimitiveProps & { class?: HTMLAttributes['class'] } >() + +const delegatedProps = reactiveOmit(props, 'class') </script> <template> <Primitive data-slot="sidebar-group-label" data-sidebar="group-label" - :as="as" - :as-child="asChild" + v-bind="delegatedProps" :class="cn( 'text-sidebar-foreground/70 ring-sidebar-ring flex h-8 shrink-0 items-center rounded-md px-2 text-xs font-medium outline-hidden transition-[margin,opacity] duration-200 ease-linear focus-visible:ring-2 [&>svg]:size-4 [&>svg]:shrink-0', 'group-data-[collapsible=icon]:-mt-8 group-data-[collapsible=icon]:opacity-0', props.class)" > <slot /> </Primitive> </template>Confirm the Tailwind utility
outline-hiddenexists in your setup (it’s not a default Tailwind class). If not, switch tooutline-noneor your project’s equivalent.frontend/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue (1)
23-23: Likely class typo: useoutline-none.
outline-hiddenisn’t a standard Tailwind utility;outline-noneis typical in shadcn components.- 'focus:bg-accent focus:text-accent-foreground relative flex cursor-default items-center gap-2 rounded-sm py-1.5 pr-2 pl-8 text-sm outline-hidden select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*=\'size-\'])]:size-4', + 'focus:bg-accent focus:text-accent-foreground relative flex cursor-default items-center gap-2 rounded-sm py-1.5 pr-2 pl-8 text-sm outline-none select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*=\'size-\'])]:size-4',frontend/src/components/ui/sidebar/Sidebar.vue (1)
64-73: Consider removing unnecessary rotation on spacer.Line 68 applies
rotate-180to the spacer div for right-side sidebars, but since this is a transparent spacing element, the rotation has no visual effect. This appears to be unused code.If this is intentional for future use, consider adding a comment. Otherwise, you can simplify:
:class="cn( 'relative w-(--sidebar-width) bg-transparent transition-[width] duration-200 ease-linear', 'group-data-[collapsible=offcanvas]:w-0', - 'group-data-[side=right]:rotate-180', variant === 'floating' || variant === 'inset' ? 'group-data-[collapsible=icon]:w-[calc(var(--sidebar-width-icon)+(--spacing(4)))]' : 'group-data-[collapsible=icon]:w-(--sidebar-width-icon)', )"frontend/src/components/ui/sidebar/SidebarMenuAction.vue (2)
21-37: Prevent accidental form submissions when rendered as a button.Explicitly set type="button" on the Primitive to avoid unintended form submits inside forms.
<Primitive data-slot="sidebar-menu-action" data-sidebar="menu-action" + :type="as === 'button' ? 'button' : undefined" :class="cn(
21-39: Ensure an accessible name for icon-only usage.If this action renders only an icon, provide an accessible name via aria-label or include a visually-hidden text node in the slot. Consider adding a label prop and forwarding it as aria-label when present.
frontend/src/components/ui/sheet/index.ts (1)
1-8: Export SheetOverlay for a complete public surface.SheetContent imports SheetOverlay, but the barrel omits it. Exporting it improves DX and parity with other Sheet parts.
export { default as Sheet } from './Sheet.vue' export { default as SheetClose } from './SheetClose.vue' export { default as SheetContent } from './SheetContent.vue' export { default as SheetDescription } from './SheetDescription.vue' export { default as SheetFooter } from './SheetFooter.vue' export { default as SheetHeader } from './SheetHeader.vue' export { default as SheetTitle } from './SheetTitle.vue' export { default as SheetTrigger } from './SheetTrigger.vue' +export { default as SheetOverlay } from './SheetOverlay.vue'frontend/src/components/ui/sidebar/SidebarProvider.vue (1)
43-45: Harden cookie with SameSite and Secure flags (non-sensitive, but best practice).Add SameSite=Lax and Secure (when https) to avoid cross-site leakage and align with modern defaults. Keeps behavior identical otherwise.
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${open.value}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` + const secure = typeof location !== 'undefined' && location.protocol === 'https:' ? '; secure' : '' + document.cookie = + `${SIDEBAR_COOKIE_NAME}=${open.value}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; samesite=lax` + securefrontend/src/components/ui/sidebar/SidebarMenuButton.vue (1)
36-52: Mirror string tooltip into aria-label for accessibility.When collapsed, content may be icon-only. Use the string tooltip as aria-label to ensure an accessible name.
<Tooltip v-else> <TooltipTrigger as-child> - <SidebarMenuButtonChild v-bind="{ ...delegatedProps, ...$attrs }"> + <SidebarMenuButtonChild + v-bind="{ ...delegatedProps, ...$attrs }" + :aria-label="typeof tooltip === 'string' ? tooltip : undefined" + > <slot /> </SidebarMenuButtonChild> </TooltipTrigger>frontend/src/components/ui/sidebar/index.ts (1)
12-36: Export the child button props from the barrel for a stable public API.Auto-imports deep-import
SidebarMenuButtonPropsfrom SidebarMenuButtonChild.vue. Re-exporting this type here avoids deep paths for consumers and keeps the public surface centralized.export { default as SidebarMenuButton } from './SidebarMenuButton.vue' +export type { SidebarMenuButtonProps } from './SidebarMenuButtonChild.vue' export { default as SidebarMenuItem } from './SidebarMenuItem.vue'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (80)
frontend/.biomelintrc-auto-import.json(8 hunks)frontend/auto-imports.d.ts(9 hunks)frontend/components.d.ts(4 hunks)frontend/src/components/ui/breadcrumb/Breadcrumb.vue(1 hunks)frontend/src/components/ui/breadcrumb/BreadcrumbEllipsis.vue(1 hunks)frontend/src/components/ui/breadcrumb/BreadcrumbItem.vue(1 hunks)frontend/src/components/ui/breadcrumb/BreadcrumbLink.vue(1 hunks)frontend/src/components/ui/breadcrumb/BreadcrumbList.vue(1 hunks)frontend/src/components/ui/breadcrumb/BreadcrumbPage.vue(1 hunks)frontend/src/components/ui/breadcrumb/BreadcrumbSeparator.vue(1 hunks)frontend/src/components/ui/breadcrumb/index.ts(1 hunks)frontend/src/components/ui/button/Button.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenu.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuContent.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuGroup.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuItem.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuShortcut.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSub.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(1 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(1 hunks)frontend/src/components/ui/dropdown-menu/index.ts(1 hunks)frontend/src/components/ui/input/Input.vue(1 hunks)frontend/src/components/ui/input/index.ts(1 hunks)frontend/src/components/ui/label/Label.vue(1 hunks)frontend/src/components/ui/label/index.ts(1 hunks)frontend/src/components/ui/separator/Separator.vue(1 hunks)frontend/src/components/ui/separator/index.ts(1 hunks)frontend/src/components/ui/sheet/Sheet.vue(1 hunks)frontend/src/components/ui/sheet/SheetClose.vue(1 hunks)frontend/src/components/ui/sheet/SheetContent.vue(1 hunks)frontend/src/components/ui/sheet/SheetDescription.vue(1 hunks)frontend/src/components/ui/sheet/SheetFooter.vue(1 hunks)frontend/src/components/ui/sheet/SheetHeader.vue(1 hunks)frontend/src/components/ui/sheet/SheetOverlay.vue(1 hunks)frontend/src/components/ui/sheet/SheetTitle.vue(1 hunks)frontend/src/components/ui/sheet/SheetTrigger.vue(1 hunks)frontend/src/components/ui/sheet/index.ts(1 hunks)frontend/src/components/ui/sidebar/Sidebar.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarContent.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarFooter.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarGroup.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarGroupAction.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarGroupContent.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarGroupLabel.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarHeader.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarInput.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarInset.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenu.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuAction.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuBadge.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuButton.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuButtonChild.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuItem.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuSkeleton.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuSub.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuSubButton.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarMenuSubItem.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarProvider.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarRail.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarSeparator.vue(1 hunks)frontend/src/components/ui/sidebar/SidebarTrigger.vue(1 hunks)frontend/src/components/ui/sidebar/index.ts(1 hunks)frontend/src/components/ui/sidebar/utils.ts(1 hunks)frontend/src/components/ui/skeleton/Skeleton.vue(1 hunks)frontend/src/components/ui/skeleton/index.ts(1 hunks)frontend/src/components/ui/tooltip/Tooltip.vue(1 hunks)frontend/src/components/ui/tooltip/TooltipContent.vue(1 hunks)frontend/src/components/ui/tooltip/TooltipProvider.vue(1 hunks)frontend/src/components/ui/tooltip/TooltipTrigger.vue(1 hunks)frontend/src/components/ui/tooltip/index.ts(1 hunks)frontend/src/core/layouts/DefaultLayout.vue(1 hunks)frontend/src/features/wikibase-schema/components/WikibaseSchemaEditor.vue(2 hunks)frontend/src/shared/components/Header.vue(1 hunks)frontend/src/shared/components/Sidebar.vue(0 hunks)
💤 Files with no reviewable changes (2)
- frontend/src/shared/components/Sidebar.vue
- frontend/src/components/ui/button/Button.vue
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/ui/sidebar/utils.ts (2)
frontend/src/components/ui/sidebar/index.ts (1)
useSidebar(36-36)frontend/auto-imports.d.ts (2)
ComputedRef(436-436)Ref(443-443)
frontend/src/components/ui/sidebar/index.ts (1)
frontend/auto-imports.d.ts (2)
SidebarProps(490-490)SidebarMenuButtonVariants(490-490)
frontend/auto-imports.d.ts (2)
frontend/src/components/ui/sidebar/utils.ts (7)
SIDEBAR_COOKIE_MAX_AGE(5-5)SIDEBAR_COOKIE_NAME(4-4)SIDEBAR_KEYBOARD_SHORTCUT(9-9)SIDEBAR_WIDTH(6-6)SIDEBAR_WIDTH_ICON(8-8)SIDEBAR_WIDTH_MOBILE(7-7)useSidebar(11-19)frontend/src/components/ui/sidebar/index.ts (2)
sidebarMenuButtonVariants(38-58)useSidebar(36-36)
🔇 Additional comments (59)
frontend/src/features/wikibase-schema/components/WikibaseSchemaEditor.vue (1)
159-163: LGTM! Better naming alignment with the return values.The rename from
getSaveButtonSeveritytogetSaveButtonVariantmore accurately reflects that the function returns variant names ('default', 'destructive', 'outline') rather than severity levels.frontend/src/components/ui/input/index.ts (1)
1-1: LGTM! Standard barrel export pattern.The export follows the established pattern for exposing UI components through index modules.
frontend/src/components/ui/sidebar/SidebarMenuItem.vue (1)
1-18: LGTM! Clean sidebar menu item implementation.The component follows the established pattern with proper data attributes, class composition via
cn, and slot-based content projection.frontend/src/components/ui/tooltip/TooltipProvider.vue (1)
1-14: LGTM! Proper tooltip provider wrapper.The component correctly wraps reka-ui's TooltipProvider with appropriate prop typing and a sensible default
delayDurationof 0 for immediate tooltip display.frontend/src/components/ui/sidebar/SidebarFooter.vue (1)
1-18: LGTM! Clean sidebar footer implementation.The component follows the established pattern with appropriate styling (
flex flex-col gap-2 p-2) and slot-based content projection.frontend/src/components/ui/sidebar/SidebarGroupContent.vue (1)
1-18: LGTM! Clean sidebar group content implementation.The component follows the established pattern with appropriate styling (
w-full text-sm) for group content containers.frontend/src/components/ui/sidebar/SidebarGroup.vue (1)
1-18: LGTM! Clean sidebar group implementation.The component follows the established pattern with appropriate flex layout and slot-based content projection for sidebar groups.
frontend/src/components/ui/skeleton/index.ts (1)
1-1: LGTM.Barrel export is correct.
frontend/components.d.ts (1)
13-125: LGTM! Auto-generated type declarations are consistent.The auto-generated type declarations properly expose all new UI components (Breadcrumb, DropdownMenu, Input, Label, Separator, Sheet, Sidebar, Skeleton, and Tooltip families) as global components. The migration of Sidebar from
shared/componentstoui/sidebaraligns with the PR objectives to consolidate UI components under the shadcn structure.frontend/src/components/ui/breadcrumb/index.ts (1)
1-7: LGTM! Clean barrel export pattern.The breadcrumb index properly exports all breadcrumb component variants following a consistent naming convention.
frontend/src/components/ui/tooltip/index.ts (1)
1-4: LGTM! Consistent barrel export pattern.The tooltip index properly exports all tooltip components, maintaining consistency with other UI module exports in this PR.
frontend/src/components/ui/sheet/Sheet.vue (1)
1-18: LGTM! Clean wrapper component implementation.The Sheet component properly wraps reka-ui's DialogRoot with correct prop/emit forwarding. The data-slot attribute provides a consistent styling hook for the sheet component family.
frontend/src/components/ui/label/index.ts (1)
1-1: LGTM! Standard barrel export.The label index properly exports the Label component following the established pattern.
frontend/src/components/ui/sidebar/SidebarMenuSubItem.vue (1)
1-18: LGTM! Well-structured sidebar sub-item component.The component properly implements a list item wrapper with composable classes and appropriate data attributes for styling hooks. The group/menu-sub-item class enables proper Tailwind group-based styling within the sidebar menu hierarchy.
frontend/src/components/ui/separator/index.ts (1)
1-1: LGTM! Standard barrel export.The separator index properly exports the Separator component following the established pattern.
frontend/src/components/ui/sidebar/SidebarHeader.vue (1)
1-18: LGTM!Clean implementation of a header wrapper component with proper class composition and slot usage.
frontend/src/components/ui/sidebar/SidebarMenu.vue (1)
1-18: LGTM!Proper use of semantic HTML with
ulelement for the menu container. The implementation is clean and follows the established patterns.frontend/src/components/ui/sidebar/SidebarInset.vue (1)
1-21: LGTM!Correct use of semantic
<main>element for the main content area. The peer-data selectors handle the responsive inset variant logic appropriately for the sidebar layout system.frontend/src/components/ui/dropdown-menu/DropdownMenuSub.vue (1)
1-15: LGTM!Correct implementation of the reka-ui wrapper pattern using
useForwardPropsEmitsto properly forward props and emits to the underlying component.frontend/src/components/ui/breadcrumb/BreadcrumbEllipsis.vue (1)
11-23: LGTM!Good accessibility implementation: the visual ellipsis is hidden from screen readers with
aria-hidden="true", while providing meaningful text viasr-onlyclass. The screen reader text is correctly placed outside the slot to ensure it's always present.frontend/src/components/ui/sidebar/SidebarMenuSkeleton.vue (1)
29-33: The CSS variable syntax is valid for Tailwind CSS.Tailwind CSS supports the custom-property shorthand
max-w-(--my-max-width), which is shorthand formax-w-[var(--my-max-width)]. The project uses Tailwind CSS via@tailwindcss/vite(confirmed infrontend/vite.config.ts), and the syntax used in the component is a documented Tailwind feature, not non-standard syntax.Likely an incorrect or invalid review comment.
frontend/src/components/ui/sidebar/SidebarGroupAction.vue (1)
20-25:outline-hiddenis a valid Tailwind CSS v4 utility class — no issue found.In Tailwind CSS v4, the outline-hidden utility sets an invisible but accessible outline (outline: 2px solid transparent; outline-offset: 2px), replacing the v3 behavior. The codebase uses Tailwind CSS v4.1.14, and
outline-hiddenis the correct choice for this context.Likely an incorrect or invalid review comment.
frontend/src/components/ui/sidebar/SidebarInput.vue (1)
1-22: LGTM! Clean wrapper component.The component properly wraps the Input component with sidebar-specific styling and data attributes. The slot forwarding pattern aligns with modern UI libraries that support icon/prefix/suffix content within input wrappers.
frontend/src/components/ui/sidebar/SidebarSeparator.vue (1)
1-19: LGTM! Consistent wrapper implementation.The component correctly wraps the Separator component with sidebar-specific border color and spacing. The implementation follows the same patterns as other sidebar components in this PR.
frontend/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue (1)
1-17: LGTM! Proper reka-ui wrapper.The component correctly wraps reka-ui's DropdownMenuTrigger with proper prop forwarding using
useForwardProps. The implementation follows reka-ui best practices.frontend/src/components/ui/sheet/SheetTitle.vue (1)
1-21: LGTM! Proper prop delegation and styling.The component correctly extends DialogTitle with custom styling while properly delegating props using
reactiveOmit. The class composition and prop forwarding patterns are correct.frontend/src/components/ui/separator/Separator.vue (1)
1-27: LGTM! Sensible defaults and responsive styling.The component provides sensible defaults (
orientation: 'horizontal',decorative: true) and implements proper responsive sizing based on the orientation attribute. The implementation correctly wraps reka-ui's Separator.frontend/src/components/ui/breadcrumb/Breadcrumb.vue (1)
1-17: LGTM! Accessible breadcrumb implementation.The component correctly uses a semantic
<nav>element with properaria-label="breadcrumb"for accessibility. The simple, straightforward implementation is appropriate for a breadcrumb container.frontend/src/components/ui/sheet/SheetOverlay.vue (1)
1-21: LGTM! Proper overlay with animations.The component correctly wraps DialogOverlay with appropriate animation classes for fade-in/fade-out transitions and proper overlay styling (fixed positioning, z-index, semi-transparent backdrop). The prop delegation and class composition are correct.
frontend/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue (1)
1-18: LGTM! Proper props and emits forwarding.The component correctly wraps reka-ui's DropdownMenuRadioGroup with proper forwarding of both props and emits using
useForwardPropsEmits. The implementation follows reka-ui best practices for components that emit events.frontend/src/components/ui/sidebar/SidebarMenuSub.vue (1)
1-22: AI summary is inconsistent with the actual implementation.The AI summary incorrectly states this component "Renders a DropdownMenuSub (from reka-ui)". In reality, the component renders a plain
<ul>element with Tailwind styling and does not import or use any reka-ui components.The actual implementation is correct—a simple presentational component for rendering sidebar submenu lists.
frontend/src/components/ui/sheet/SheetDescription.vue (1)
1-21: LGTM!Standard reka-ui wrapper following best practices: proper type imports, correct prop forwarding using
reactiveOmit, and appropriate styling. The implementation is clean and type-safe.frontend/src/components/ui/sidebar/SidebarTrigger.vue (1)
1-27: LGTM!Clean implementation with proper accessibility. The component correctly integrates with the sidebar context via
useSidebar(), includes appropriate data attributes for testing, and provides screen reader text for the toggle action.frontend/src/components/ui/dropdown-menu/DropdownMenu.vue (1)
1-18: LGTM!Proper root wrapper implementation using
useForwardPropsEmitsto ensure bidirectional prop and event binding. The component correctly serves as the foundation for the dropdown menu system.frontend/src/components/ui/dropdown-menu/DropdownMenuGroup.vue (1)
1-15: LGTM!Straightforward wrapper component with proper type imports and prop forwarding. The implementation follows the established pattern consistently.
frontend/src/components/ui/tooltip/Tooltip.vue (1)
1-18: LGTM!Consistent root wrapper implementation mirroring the pattern used in DropdownMenu. Proper bidirectional binding with
useForwardPropsEmitsensures all props and events are correctly forwarded to the underlying TooltipRoot.frontend/src/components/ui/breadcrumb/BreadcrumbList.vue (1)
1-17: LGTM!Proper use of semantic HTML (
<ol>) for breadcrumb navigation. The responsive styling adapts gap spacing based on viewport size, and the class composition correctly merges default and custom classes.frontend/src/components/ui/sheet/SheetTrigger.vue (1)
1-15: LGTM!Clean wrapper implementation consistent with the other Sheet components in this PR. Props are properly typed and forwarded to the underlying DialogTrigger from reka-ui.
frontend/src/components/ui/breadcrumb/BreadcrumbItem.vue (1)
10-16: LGTM; consistent with breadcrumb semantics.Looks good. Ensure it’s used inside BreadcrumbList (ol/ul within a nav[aria-label="Breadcrumb"]) so list semantics are preserved.
frontend/src/core/layouts/DefaultLayout.vue (1)
2-2: Prefixed Lucide icon names are the established pattern in this codebase and are properly configured.Your import of
LucideFolderOpenandLucidePlusmatches the naming convention used throughout the codebase (Header.vue, WikibaseSchemaEditor.vue, StatementManager.vue, etc.). Prefixed/aliased names likeLucideFolderOpenare supported through lucide-vue-next's aliased-names builds, and your project is already configured to use them consistently. No changes needed.Likely an incorrect or invalid review comment.
frontend/src/components/ui/sheet/SheetHeader.vue (1)
1-15: LGTM — minimal, typed, and composable.The wrapper cleanly merges classes via cn and keeps attrs forwardable through the root div. No issues spotted.
frontend/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue (1)
1-23: LGTM — correct delegated props and class merge.Pattern matches the rest of the dropdown-menu suite; separator styles look standard.
frontend/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue (1)
1-33: LGTM overall; one small utility check.Props/emits forwarding via
useForwardPropsEmitsis solid; indicator and classes look good.This component also uses
outline-hiddenin its class list; ensure this utility is defined in your Tailwind config (or replace withoutline-none).frontend/src/components/ui/breadcrumb/BreadcrumbSeparator.vue (1)
12-21: LGTM — simple, accessible separator.Good defaults, override slot, and class merging look solid.
frontend/src/components/ui/label/Label.vue (1)
1-26: LGTM! Clean Label wrapper implementation.The component correctly wraps reka-ui's Label with proper prop forwarding, accessibility attributes, and comprehensive styling for various states (disabled, peer-disabled).
frontend/src/components/ui/dropdown-menu/DropdownMenuItem.vue (1)
1-36: LGTM! Exemplary prop forwarding pattern.The component demonstrates the correct approach for custom props: excluding them from delegation (Line 21) and explicitly binding them as data attributes (Lines 29-30) for CSS selector targeting.
frontend/src/components/ui/sidebar/SidebarMenuButtonChild.vue (1)
1-36: LGTM! Well-structured sidebar button component.The component properly exports its props interface, uses the variant system correctly, and forwards attributes appropriately. The data attributes enable flexible styling and targeting.
frontend/src/components/ui/sidebar/Sidebar.vue (1)
44-46: Good use of sr-only header for accessibility.The screen-reader-only header provides context for assistive technology users while keeping the UI clean.
frontend/src/components/ui/dropdown-menu/DropdownMenuContent.vue (1)
1-31: LGTM! Proper portal and emit handling.The component correctly uses
useForwardPropsEmitsto handle both props and events, wraps content in a portal for proper layering, and includes comprehensive animation states.frontend/src/components/ui/sidebar/SidebarMenuSubButton.vue (1)
1-41: LGTM! Clean sub-button implementation.The component uses appropriate defaults (anchor element for navigation), implements size variants correctly, and includes comprehensive accessibility and interaction states.
frontend/src/components/ui/dropdown-menu/DropdownMenuLabel.vue (1)
1-25: LGTM! Proper custom prop handling.The component correctly excludes custom props from delegation (Line 12) and explicitly binds
insetas a data attribute (Line 19) for CSS targeting, consistent with the pattern used in DropdownMenuItem.frontend/src/components/ui/sidebar/utils.ts (1)
1-19: LGTM — context API and typings are clear and cohesive.frontend/src/components/ui/sheet/SheetContent.vue (1)
29-57: LGTM — composition and prop forwarding look correct.Side-based transitions and overlay/portal wiring align with expectations.
frontend/src/components/ui/sidebar/SidebarProvider.vue (1)
35-39: useVModel passive semantics are correctly implemented.The code correctly sets
passive: falsewhen the prop is controlled (provided by parent) to emit updates, andpassive: truewhen uncontrolled (prop undefined) to manage internal state with the defaultValue fallback. No changes needed.frontend/src/components/ui/sidebar/index.ts (1)
5-10: Props and barrel exports read well.Typing via HTMLAttributes['class'] and the component surface look consistent with usage across the PR.
Also applies to: 12-36
frontend/auto-imports.d.ts (2)
1-1189: Generated surface matches the new UI modules.No blocking issues spotted in the declarations.
487-492: The deep import is necessary—the barrel does not currently re-export this type.The review comment assumes
SidebarMenuButtonPropsis already re-exported fromsidebar/index.ts, but it is not. The type is only defined inSidebarMenuButtonChild.vueand is not included in the barrel exports. The deep import at line 487 is correct and necessary in its current form. To implement the suggested refactor, you would first need to addexport type { SidebarMenuButtonProps }tosidebar/index.ts, but that change is not part of this PR.Likely an incorrect or invalid review comment.
frontend/src/components/ui/dropdown-menu/index.ts (1)
1-15: Code changes verified; no issues found.Confirmed:
reka-uiis installed infrontend/package.json(version ^2.5.1), and reka-ui exports DropdownMenuPortal. All 14 SFC components exist, and barrel exports are consistent and correct.frontend/.biomelintrc-auto-import.json (1)
1-496: Duplicate and export parity checks for dropdown-menu and sidebar passed. Let me verify the remaining newly-added globals:Now let me verify the remaining new globals:
Now let me verify the remaining components and exports in the codebase:
Based on my verification through shell script execution and web searches, the analysis is complete.
Config validation passed: no duplicates, export parity confirmed for verified barrels.
Dropdown-menu and sidebar exports all verified (35 components total). No duplicate globals found. Breadcrumb, Sheet, and Tooltip follow standard shadcn-vue patterns and are properly exported from their respective barrels. Tooltip collision risk from third-party libraries (BootstrapVue, Radix Vue, Floating Vue, etc.) is valid, though it primarily affects IDE auto-import behavior rather than runtime—consider a UI prefix in a follow-up if external deps introduce conflicts.
| :as="as" | ||
| :as-child="asChild" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Props scoping bug: as/asChild are undefined in template.
You created props = defineProps(...) but reference as and asChild directly. Use props.as/props.asChild (or destructure via toRefs).
- :as="as"
- :as-child="asChild"
+ :as="props.as"
+ :as-child="props.asChild"Alternatively:
+import { toRefs } from 'vue'
-const props = withDefaults(defineProps<PrimitiveProps & { class?: HTMLAttributes['class'] }>(), { as: 'a' })
+const props = withDefaults(defineProps<PrimitiveProps & { class?: HTMLAttributes['class'] }>(), { as: 'a' })
+const { as, asChild } = toRefs(props)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :as="as" | |
| :as-child="asChild" | |
| :as="props.as" | |
| :as-child="props.asChild" |
🤖 Prompt for AI Agents
In frontend/src/components/ui/breadcrumb/BreadcrumbLink.vue around lines 15-16,
the template references as and asChild directly but the component defined props
using defineProps(...) so as and asChild are undefined; update the template to
use props.as and props.asChild or, alternatively, destructure the props in the
script with const { as, asChild } = toRefs(props) and use those variables in the
template so the values are properly scoped and reactive.
| <span | ||
| data-slot="breadcrumb-page" | ||
| role="link" | ||
| aria-disabled="true" | ||
| aria-current="page" | ||
| :class="cn('text-foreground font-normal', props.class)" | ||
| > | ||
| <slot /> | ||
| </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect ARIA semantics for breadcrumb page.
The current breadcrumb page should not use role="link" combined with aria-disabled="true". This creates confusion for screen reader users, as the element announces itself as a link but is marked as disabled. Since the current page is not interactive, remove the role attribute or leave it as implicit text content.
Apply this diff to fix the accessibility issue:
<span
data-slot="breadcrumb-page"
- role="link"
- aria-disabled="true"
aria-current="page"
:class="cn('text-foreground font-normal', props.class)"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span | |
| data-slot="breadcrumb-page" | |
| role="link" | |
| aria-disabled="true" | |
| aria-current="page" | |
| :class="cn('text-foreground font-normal', props.class)" | |
| > | |
| <slot /> | |
| </span> | |
| <span | |
| data-slot="breadcrumb-page" | |
| aria-current="page" | |
| :class="cn('text-foreground font-normal', props.class)" | |
| > | |
| <slot /> | |
| </span> |
🤖 Prompt for AI Agents
In frontend/src/components/ui/breadcrumb/BreadcrumbPage.vue around lines 11 to
19, the span currently uses role="link" with aria-disabled="true" which is
incorrect for a non-interactive current page; remove the role="link" and
aria-disabled="true" attributes so the element is treated as plain text (keep
aria-current="page" and existing classes/slot intact) to correct the ARIA
semantics for screen readers.
| <DropdownMenuSubContent | ||
| data-slot="dropdown-menu-sub-content" | ||
| v-bind="forwarded" | ||
| :class="cn('bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 min-w-[8rem] origin-(--reka-dropdown-menu-content-transform-origin) overflow-hidden rounded-md border p-1 shadow-lg', props.class)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Tailwind var syntax for transform-origin.
origin-(--reka-...) won’t parse; Tailwind expects origin-[var(--...)]. This breaks submenu animation origin.
- :class="cn('bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 min-w-[8rem] origin-(--reka-dropdown-menu-content-transform-origin) overflow-hidden rounded-md border p-1 shadow-lg', props.class)"
+ :class="cn('bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 min-w-[8rem] origin-[var(--reka-dropdown-menu-content-transform-origin)] overflow-hidden rounded-md border p-1 shadow-lg', props.class)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :class="cn('bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 min-w-[8rem] origin-(--reka-dropdown-menu-content-transform-origin) overflow-hidden rounded-md border p-1 shadow-lg', props.class)" | |
| :class="cn('bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 z-50 min-w-[8rem] origin-[var(--reka-dropdown-menu-content-transform-origin)] overflow-hidden rounded-md border p-1 shadow-lg', props.class)" |
🤖 Prompt for AI Agents
frontend/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue around line
20: the Tailwind class uses invalid syntax
origin-(--reka-dropdown-menu-content-transform-origin) which Tailwind won't
parse; replace it with the bracketed var syntax
origin-[var(--reka-dropdown-menu-content-transform-origin)] so the
transform-origin reads the CSS variable correctly and submenu animations work;
update the class string accordingly and keep the rest of the cn(...) call the
same.
| <template> | ||
| <DropdownMenuSubTrigger | ||
| data-slot="dropdown-menu-sub-trigger" | ||
| v-bind="forwardedProps" | ||
| :class="cn( | ||
| 'focus:bg-accent focus:text-accent-foreground data-[state=open]:bg-accent data-[state=open]:text-accent-foreground flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-hidden select-none data-[inset]:pl-8', | ||
| props.class, | ||
| )" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing data-inset attribute binding.
The inset prop is defined but never bound as a data attribute. The class string includes data-[inset]:pl-8 on Line 22, which will never apply without the attribute binding.
Apply this diff to bind the inset prop:
<DropdownMenuSubTrigger
data-slot="dropdown-menu-sub-trigger"
+ :data-inset="inset ? '' : undefined"
v-bind="forwardedProps"
:class="cn(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <template> | |
| <DropdownMenuSubTrigger | |
| data-slot="dropdown-menu-sub-trigger" | |
| v-bind="forwardedProps" | |
| :class="cn( | |
| 'focus:bg-accent focus:text-accent-foreground data-[state=open]:bg-accent data-[state=open]:text-accent-foreground flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-hidden select-none data-[inset]:pl-8', | |
| props.class, | |
| )" | |
| <template> | |
| <DropdownMenuSubTrigger | |
| data-slot="dropdown-menu-sub-trigger" | |
| :data-inset="inset ? '' : undefined" | |
| v-bind="forwardedProps" | |
| :class="cn( | |
| 'focus:bg-accent focus:text-accent-foreground data-[state=open]:bg-accent data-[state=open]:text-accent-foreground flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-hidden select-none data-[inset]:pl-8', | |
| props.class, | |
| )" |
🤖 Prompt for AI Agents
In frontend/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue around
lines 17 to 24, the defined inset prop is never bound as a data attribute so the
class rule data-[inset]:pl-8 never takes effect; bind the prop onto the root
element by adding a data-inset binding (e.g. :data-inset="props.inset" or
v-bind:data-inset="props.inset") to the <DropdownMenuSubTrigger> tag so the
data-[inset] selector becomes active when inset is true/defined.
| <input | ||
| v-model="modelValue" | ||
| data-slot="input" | ||
| :class="cn( | ||
| 'file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', | ||
| 'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]', | ||
| 'aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive', | ||
| props.class, | ||
| )" | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix syntax error: input element must be self-closing.
The <input> element is a void element and must be self-closing in Vue templates. Line 30 currently has > which will cause a template compilation error.
Apply this diff:
<input
v-model="modelValue"
data-slot="input"
:class="cn(
'file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]',
'aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive',
props.class,
)"
- >
+ />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <input | |
| v-model="modelValue" | |
| data-slot="input" | |
| :class="cn( | |
| 'file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', | |
| 'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]', | |
| 'aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive', | |
| props.class, | |
| )" | |
| > | |
| <input | |
| v-model="modelValue" | |
| data-slot="input" | |
| :class="cn( | |
| 'file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', | |
| 'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]', | |
| 'aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive', | |
| props.class, | |
| )" | |
| /> |
🤖 Prompt for AI Agents
In frontend/src/components/ui/input/Input.vue around lines 21 to 30 the <input>
element is not self-closing which causes a Vue template compilation error; fix
it by making the input a void/self-closing element (replace the trailing ">"
with "/>"), ensure there is no separate closing </input> tag and that the
existing attributes (v-model, data-slot, :class, etc.) remain unchanged.
| export const sidebarMenuButtonVariants = cva( | ||
| 'peer/menu-button flex w-full items-center gap-2 overflow-hidden rounded-md p-2 text-left text-sm outline-hidden ring-sidebar-ring transition-[width,height,padding] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground focus-visible:ring-2 active:bg-sidebar-accent active:text-sidebar-accent-foreground disabled:pointer-events-none disabled:opacity-50 group-has-data-[sidebar=menu-action]/menu-item:pr-8 aria-disabled:pointer-events-none aria-disabled:opacity-50 data-[active=true]:bg-sidebar-accent data-[active=true]:font-medium data-[active=true]:text-sidebar-accent-foreground data-[state=open]:hover:bg-sidebar-accent data-[state=open]:hover:text-sidebar-accent-foreground group-data-[collapsible=icon]:size-8! group-data-[collapsible=icon]:p-2! [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0', | ||
| { | ||
| variants: { | ||
| variant: { | ||
| default: 'hover:bg-sidebar-accent hover:text-sidebar-accent-foreground', | ||
| outline: | ||
| 'bg-background shadow-[0_0_0_1px_hsl(var(--sidebar-border))] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground hover:shadow-[0_0_0_1px_hsl(var(--sidebar-accent))]', | ||
| }, | ||
| size: { | ||
| default: 'h-8 text-sm', | ||
| sm: 'h-7 text-xs', | ||
| lg: 'h-12 text-sm group-data-[collapsible=icon]:p-0!', | ||
| }, | ||
| }, | ||
| defaultVariants: { | ||
| variant: 'default', | ||
| size: 'default', | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify utility classes (UnoCSS vs Tailwind); adjust if needed.
The cva base includes tokens that look UnoCSS-specific: outline-hidden and the important suffix pattern size-8! / p-2!. If you're on Tailwind, these won’t compile; Tailwind expects outline-none and the important prefix !class (and may not support size-* without a plugin).
- If Tailwind: consider
outline-none group-data-[collapsible=icon]:!h-8 group-data-[collapsible=icon]:!w-8 group-data-[collapsible=icon]:!p-2and keepdata-[]variants. - If UnoCSS: current classes are fine.
🏁 Script executed:
#!/bin/bash
# Detect CSS engine(s)
fd -a -HI 'tailwind.config.*|uno.config.*' -t f
# Search for potential UnoCSS-only patterns
rg -n "outline-hidden|size-[0-9]+!|:p-[0-9]+!" frontend/src/components | sed -n '1,200p'
# Spot-check that data-attribute variants are enabled elsewhere
rg -n "data-\[state=open\]" -S frontend/src/components | head -n 10Length of output: 7723
---
🏁 Script executed:
```shell
#!/bin/bash
# Check package.json for CSS framework dependencies
cat package.json | grep -E '"(tailwind|unocss|postcss|@unocss)"' || echo "No CSS deps found in package.json"
# Look for Vite or PostCSS config
fd -a -HI 'vite.config.*|postcss.config.*' -t f
# Check for .eslintignore or tailwind config in subdirs
find . -maxdepth 3 -name '*.config.*' -o -name 'tailwind.js' 2>/dev/null | head -20
# Search for unocss preset or tailwind extend references
rg -n "presetUno|@unocss|extends.*tailwind" --type ts --type js | head -20
Length of output: 102
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect vite.config.ts to check CSS plugin configuration
cat /home/jailuser/git/frontend/vite.config.ts
# Get full list of dependencies from package.json
cat /home/jailuser/git/frontend/package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat /home/jailuser/git/frontend/package.json | grep -A 100 '"dependencies"' | head -50
Length of output: 4127
Replace UnoCSS utility classes with Tailwind CSS v4.1.14-compatible alternatives.
The project uses Tailwind CSS v4.1.14 (configured in vite.config.ts with @tailwindcss/vite and tailwindcss v4.1.14 in package.json), but the sidebar component (frontend/src/components/ui/sidebar/index.ts, lines 39–50) contains UnoCSS-specific syntax that won't compile:
outline-hidden→ replace withoutline-nonesize-8!/p-2!/p-0!→ replace with!h-8 !w-8 !p-2/!p-0(Tailwind uses prefix!, not postfix)size-*utilities may require a custom Tailwind plugin or explicit width/height utilities
Update lines 39–50:
'peer/menu-button flex w-full items-center gap-2 overflow-hidden rounded-md p-2 text-left text-sm outline-none ring-sidebar-ring transition-[width,height,padding] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground focus-visible:ring-2 active:bg-sidebar-accent active:text-sidebar-accent-foreground disabled:pointer-events-none disabled:opacity-50 group-has-data-[sidebar=menu-action]/menu-item:pr-8 aria-disabled:pointer-events-none aria-disabled:opacity-50 data-[active=true]:bg-sidebar-accent data-[active=true]:font-medium data-[active=true]:text-sidebar-accent-foreground data-[state=open]:hover:bg-sidebar-accent data-[state=open]:hover:text-sidebar-accent-foreground group-data-[collapsible=icon]:!h-8 group-data-[collapsible=icon]:!w-8 group-data-[collapsible=icon]:!p-2 [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0',
and:
lg: 'h-12 text-sm group-data-[collapsible=icon]:!p-0',
🤖 Prompt for AI Agents
In frontend/src/components/ui/sidebar/index.ts around lines 38 to 58, the class
string uses UnoCSS-specific utilities that Tailwind v4.1.14 doesn't support;
replace those with Tailwind-compatible equivalents: change outline-hidden to
outline-none, convert postfix important size/padding like size-8! / p-2! / p-0!
to prefix form !h-8 !w-8 !p-2 / !p-0 (or explicit h-8 w-8), remove or replace
size-* utilities with explicit h-*/w-* where needed, and update the lg variant
group-data rule to use !p-0; ensure the final class list uses only Tailwind v4
utilities and keep the rest of the data-/group- selectors intact.
| :tabindex="-1" | ||
| title="Toggle Sidebar" | ||
| :class="cn( | ||
| 'hover:after:bg-sidebar-border absolute inset-y-0 z-20 hidden w-4 -translate-x-1/2 transition-all ease-linear group-data-[side=left]:-right-4 group-data-[side=right]:left-0 after:absolute after:inset-y-0 after:left-1/2 after:w-[2px] sm:flex', | ||
| 'in-data-[side=left]:cursor-w-resize in-data-[side=right]:cursor-e-resize', | ||
| '[[data-side=left][data-state=collapsed]_&]:cursor-e-resize [[data-side=right][data-state=collapsed]_&]:cursor-w-resize', | ||
| 'hover:group-data-[collapsible=offcanvas]:bg-sidebar group-data-[collapsible=offcanvas]:translate-x-0 group-data-[collapsible=offcanvas]:after:left-full', | ||
| '[[data-side=left][data-collapsible=offcanvas]_&]:-right-2', | ||
| '[[data-side=right][data-collapsible=offcanvas]_&]:-left-2', | ||
| props.class, | ||
| )" | ||
| @click="toggleSidebar" | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the rail keyboard-accessible and avoid accidental form submits.
A toggle control should be focusable and operable via keyboard. tabindex="-1" prevents keyboard access; also set type="button" and add key handlers. Consider exposing open state for aria-expanded.
- :tabindex="-1"
+ type="button"
+ @keydown.enter.prevent="toggleSidebar"
+ @keydown.space.prevent="toggleSidebar"
+ :aria-expanded="open"-const { toggleSidebar } = useSidebar()
+const { toggleSidebar, open } = useSidebar()If open is named differently in useSidebar(), bind the correct state prop. Removing the tabindex line entirely is fine since buttons are focusable by default.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/ui/sidebar/SidebarRail.vue around lines 18-30, the
rail is not keyboard-accessible and can cause accidental form submits; remove
tabindex="-1" (buttons are focusable by default), add type="button" to prevent
form submission, expose and bind the sidebar open state to aria-expanded (use
the actual state name returned by useSidebar()), and add key handlers for Enter
and Space that call the same toggleSidebar handler (or use a native <button>
element so click + keyboard work automatically); ensure any added key handlers
call preventDefault where appropriate and keep the existing click behavior.
| <SidebarHeader> | ||
| <SidebarMenuButton size="lg"><h1 class="text-xl font-semibold text-gray-800">DataForge</h1></SidebarMenuButton> | ||
| </SidebarHeader> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid an inert interactive control in the header.
SidebarMenuButton without as-child or a click handler renders an interactive element with no action, which is an a11y anti‑pattern. Make it a link to home (as-child) or a plain heading.
- <SidebarHeader>
- <SidebarMenuButton size="lg"><h1 class="text-xl font-semibold text-gray-800">DataForge</h1></SidebarMenuButton>
- </SidebarHeader>
+ <SidebarHeader>
+ <SidebarMenuButton as-child size="lg">
+ <router-link to="/">
+ <h1 class="text-xl font-semibold text-gray-800">DataForge</h1>
+ </router-link>
+ </SidebarMenuButton>
+ </SidebarHeader>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <SidebarHeader> | |
| <SidebarMenuButton size="lg"><h1 class="text-xl font-semibold text-gray-800">DataForge</h1></SidebarMenuButton> | |
| </SidebarHeader> | |
| <SidebarHeader> | |
| <SidebarMenuButton as-child size="lg"> | |
| <router-link to="/"> | |
| <h1 class="text-xl font-semibold text-gray-800">DataForge</h1> | |
| </router-link> | |
| </SidebarMenuButton> | |
| </SidebarHeader> |
🤖 Prompt for AI Agents
In frontend/src/core/layouts/DefaultLayout.vue around lines 29-31, the
SidebarMenuButton in the header is rendering an interactive control with no
action; update it so it is not an inert interactive element by either (A) making
it act as a link to home using the component's as-child prop and wrapping a
router-link/a tag with the heading text, or (B) replacing SidebarMenuButton with
a plain non-interactive heading element (e.g., h1) if no navigation is intended;
choose one approach and remove the unused interactive control or add the
appropriate click/navigation handler so the element is accessible.
No description provided.