Migrate site from Gatsby to Next.js 15 with App Router #30
Conversation
* implement parallel test and build jobs in GitHub Actions * add comprehensive HTML reporter with artifacts * configure test result and coverage uploads * add workflow documentation in .github/workflows/README.md * optimize browser coverage (Chromium only for E2E) * configure retries and trace collection for failures
* add linkedin field to Author interface * create AuthorSocialLinks component with Twitter, GitHub, LinkedIn, and website icons * integrate social links in event cards for both next-event and archives * improve presenter profile discoverability
* add default OpenGraph image for all pages * improve page-specific metadata with canonical URLs * add Twitter Card support with large image previews * update descriptions for better SEO * ensure consistent social sharing experience
* extract testable RSS generation logic to lib/rss-generator.ts * add 13 comprehensive unit tests for RSS generator * use postbuild npm lifecycle for automatic execution * generate both RSS feed and sitemap in single script * update robots.txt with correct sitemap URL * remove duplicate sitemap generation (Next.js metadata API incompatible with static export)
* add comprehensive tests for cn utility function (9 tests) * add NextEvent component tests with social links validation (9 tests) * configure Vitest coverage with v8 provider * set coverage thresholds (statements: 40%, branches: 70%, functions: 60%) * exclude e2e tests from Vitest run * add jsx preserve for better test compatibility
* consolidate 7 test files into single critical-flows suite * reduce from 40+ tests to 7 focused critical user flows * configure automatic screenshots, videos, and traces on failure * focus on Chromium only (Desktop + Mobile) for speed * add comprehensive E2E testing guide in e2e/README.md * reduce execution time from 10min to 2-3min (75% faster)
* add RSS and sitemap generation guide (docs/RSS_AND_SITEMAP.md) * add E2E testing strategy documentation (docs/E2E_TESTING.md) * document build script approach rationale * provide troubleshooting and maintenance guides
* add project overview and tech stack * document getting started and development workflow * provide testing instructions with coverage details * add contribution guidelines with event/speaker examples * include community links and MIT license reference * document project structure
- Add @types/rss for RSS module type definitions - Refactor RSS generator to use ResolvedEvent type - Fix test files: add missing imports, fix type annotations - Configure ESLint with .eslintrc.json - Exclude chart.tsx from TypeScript checking (shadcn component) - Update vitest config for jsdom environment matching - Fix Playwright API calls in e2e tests - Fix RSS generator bug in generate-feeds.mjs All core workflows (test, lint, type-check) now pass successfully.
- Change @typescript-eslint/no-explicit-any to warning level - Fix test mock to use Author type instead of any - Restore next.config.mjs lint/typescript ignore settings for builds Note: Pre-existing build error 'TypeError: generate is not a function' remains from commit 049b5a1. This appears to be a Next.js 15.5.5 issue unrelated to the CI workflow fixes. All CI jobs (test, lint, type-check) pass successfully.
✅ Deploy Preview for keen-fairy-bc0a2b ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughRemoved legacy Gatsby/Storybook configs and editor/tooling files; added a Next.js 15 app scaffold with global styles, many shadcn/Radix UI primitives, pages, data loader/types, unit and E2E tests (Vitest + Playwright), updated CI, and modern tooling configs for Tailwind/pnpm. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Test as Unit Tests
participant E2E as Playwright
participant Lint as Linter
participant TSC as TypeCheck
participant Build as Build
participant Gate as ci-success
Dev->>GH: push / pull_request
GH->>Test: run unit tests + coverage
GH->>E2E: run Playwright e2e
GH->>Lint: run linter
GH->>TSC: run tsc --noEmit
Test-->>GH: upload coverage & test results
E2E-->>GH: upload reports
Build->>GH: upload build artifacts & verify outputs
Lint-->>Gate: report status
TSC-->>Gate: report status
Gate->>GH: finalize workflow status
sequenceDiagram
autonumber
participant Page as ArchivesPage
participant Loader as loadEvents
participant Resolver as resolveEvent
participant UI as ArchivesList (client)
participant Browser as UserBrowser
Page->>Loader: loadEvents()
Loader-->>Page: events[]
Page->>Resolver: resolveEvent(events[])
Resolver-->>Page: resolvedEvents[]
Page->>UI: render with resolvedEvents
Browser->>UI: enter searchQuery
UI->>UI: filter(results) & set currentPage = 1
Browser->>UI: click page 2
UI->>UI: set currentPage -> scrollIntoView(top)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (15)
💤 Files with no reviewable changes (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)components/next-event.tsx (2)
lib/data-loader.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 42
🧹 Nitpick comments (48)
.gitignore (1)
19-20: Consider keeping template env files tracked.The blanket
.env*pattern also hides files like.env.examplethat we usually commit for documentation. If you rely on those templates, add negations (!.env.example, etc.) so contributors still get them.components/ui/radio-group.tsx (1)
35-40: Simplify centering approach (optional refactor).The parent Indicator uses flexbox centering (
flex items-center justify-center) while the child CircleIcon uses absolute positioning with transforms for centering. This is redundant.Consider simplifying by removing the absolute positioning from the icon since the parent flex already centers it:
<RadioGroupPrimitive.Indicator data-slot="radio-group-indicator" className="relative flex items-center justify-center" > - <CircleIcon className="fill-primary absolute top-1/2 left-1/2 size-2 -translate-x-1/2 -translate-y-1/2" /> + <CircleIcon className="fill-primary size-2" /> </RadioGroupPrimitive.Indicator>components/ui/input-otp.tsx (1)
39-67: Consider extracting the className for improved maintainability.The component logic is correct and safely handles the context with optional chaining. However, the className on line 54 is exceptionally long (300+ characters), which reduces readability.
Consider extracting the base slot classes into a constant:
+const slotBaseClasses = + 'data-[active=true]:border-ring data-[active=true]:ring-ring/50 data-[active=true]:aria-invalid:ring-destructive/20 dark:data-[active=true]:aria-invalid:ring-destructive/40 aria-invalid:border-destructive data-[active=true]:aria-invalid:border-destructive dark:bg-input/30 border-input relative flex h-9 w-9 items-center justify-center border-y border-r text-sm shadow-xs transition-all outline-none first:rounded-l-md first:border-l last:rounded-r-md data-[active=true]:z-10 data-[active=true]:ring-[3px]' + function InputOTPSlot({ index, className, ...props }: React.ComponentProps<'div'> & { index: number }) { const inputOTPContext = React.useContext(OTPInputContext) const { char, hasFakeCaret, isActive } = inputOTPContext?.slots[index] ?? {} return ( <div data-slot="input-otp-slot" data-active={isActive} - className={cn( - 'data-[active=true]:border-ring data-[active=true]:ring-ring/50 data-[active=true]:aria-invalid:ring-destructive/20 dark:data-[active=true]:aria-invalid:ring-destructive/40 aria-invalid:border-destructive data-[active=true]:aria-invalid:border-destructive dark:bg-input/30 border-input relative flex h-9 w-9 items-center justify-center border-y border-r text-sm shadow-xs transition-all outline-none first:rounded-l-md first:border-l last:rounded-r-md data-[active=true]:z-10 data-[active=true]:ring-[3px]', - className, - )} + className={cn(slotBaseClasses, className)} {...props} >components/ui/tabs.tsx (1)
45-45: Consider breaking down the long className string for improved readability.The className string on Line 45 spans over 700 characters, making it difficult to read, maintain, and review. While functionally correct, this impacts code maintainability.
Consider refactoring using a template literal for better readability:
- "data-[state=active]:bg-background dark:data-[state=active]:text-foreground focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 text-foreground dark:text-muted-foreground inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:shadow-sm [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", + [ + // Base styles + 'inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5', + 'rounded-md border border-transparent px-2 py-1', + 'text-sm font-medium whitespace-nowrap', + 'transition-[color,box-shadow]', + // Active state + 'data-[state=active]:bg-background data-[state=active]:shadow-sm', + // Dark mode active state + 'dark:data-[state=active]:text-foreground dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30', + // Text colors + 'text-foreground dark:text-muted-foreground', + // Focus visible + 'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring', + 'focus-visible:ring-[3px] focus-visible:outline-1', + // Disabled + 'disabled:pointer-events-none disabled:opacity-50', + // SVG children + '[&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*=\'size-\'])]:size-4', + ].join(' '),components/ui/collapsible.tsx (3)
5-9: Consider forwarding refs for better composability.Radix primitives often require refs for focus management, portal positioning, and advanced composition patterns. Without
React.forwardRef, refs passed toCollapsiblewon't reach the underlying primitive.Apply this diff to add ref forwarding:
-function Collapsible({ - ...props -}: React.ComponentProps<typeof CollapsiblePrimitive.Root>) { - return <CollapsiblePrimitive.Root data-slot="collapsible" {...props} /> -} +const Collapsible = React.forwardRef< + React.ElementRef<typeof CollapsiblePrimitive.Root>, + React.ComponentPropsWithoutRef<typeof CollapsiblePrimitive.Root> +>((props, ref) => { + return <CollapsiblePrimitive.Root ref={ref} data-slot="collapsible" {...props} /> +}) +Collapsible.displayName = 'Collapsible'Optional: Simplify parameter syntax.
The
{ ...props }destructuring is valid but unnecessarily verbose compared to a plainpropsparameter.
11-20: Consider forwarding refs for better composability.Apply the same ref-forwarding pattern here for consistency and to enable advanced composition scenarios.
Apply this diff:
-function CollapsibleTrigger({ - ...props -}: React.ComponentProps<typeof CollapsiblePrimitive.CollapsibleTrigger>) { - return ( - <CollapsiblePrimitive.CollapsibleTrigger - data-slot="collapsible-trigger" - {...props} - /> - ) -} +const CollapsibleTrigger = React.forwardRef< + React.ElementRef<typeof CollapsiblePrimitive.CollapsibleTrigger>, + React.ComponentPropsWithoutRef<typeof CollapsiblePrimitive.CollapsibleTrigger> +>((props, ref) => { + return ( + <CollapsiblePrimitive.CollapsibleTrigger + ref={ref} + data-slot="collapsible-trigger" + {...props} + /> + ) +}) +CollapsibleTrigger.displayName = 'CollapsibleTrigger'
22-31: Consider forwarding refs for better composability.Apply the same ref-forwarding pattern to complete the implementation.
Apply this diff:
-function CollapsibleContent({ - ...props -}: React.ComponentProps<typeof CollapsiblePrimitive.CollapsibleContent>) { - return ( - <CollapsiblePrimitive.CollapsibleContent - data-slot="collapsible-content" - {...props} - /> - ) -} +const CollapsibleContent = React.forwardRef< + React.ElementRef<typeof CollapsiblePrimitive.CollapsibleContent>, + React.ComponentPropsWithoutRef<typeof CollapsiblePrimitive.CollapsibleContent> +>((props, ref) => { + return ( + <CollapsiblePrimitive.CollapsibleContent + ref={ref} + data-slot="collapsible-content" + {...props} + /> + ) +}) +CollapsibleContent.displayName = 'CollapsibleContent'components/ui/badge.tsx (2)
7-26: Consider breaking up the base classes for readability.The base class string on line 8 exceeds 300 characters, which can impact maintainability. Consider splitting it into multiple lines or extracting logical groups into named constants.
Example refactor:
-const badgeVariants = cva( - 'inline-flex items-center justify-center rounded-md border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none 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 transition-[color,box-shadow] overflow-hidden', +const badgeVariants = cva( + [ + 'inline-flex items-center justify-center rounded-md border px-2 py-0.5 text-xs font-medium', + 'w-fit whitespace-nowrap shrink-0 overflow-hidden gap-1', + '[&>svg]:size-3 [&>svg]:pointer-events-none', + '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', + 'transition-[color,box-shadow]', + ], {
28-44: Consider extracting the props type and adding ref forwarding.For improved maintainability and composability:
- Extract the inline props type to a named interface
- Add ref forwarding support using
React.forwardRefApply this refactor:
+export interface BadgeProps + extends React.ComponentProps<'span'>, + VariantProps<typeof badgeVariants> { + asChild?: boolean +} + -function Badge({ +const Badge = React.forwardRef<HTMLSpanElement, BadgeProps>(({ className, variant, asChild = false, ...props -}: React.ComponentProps<'span'> & - VariantProps<typeof badgeVariants> & { asChild?: boolean }) { +}, ref) => { const Comp = asChild ? Slot : 'span' return ( <Comp + ref={ref} data-slot="badge" className={cn(badgeVariants({ variant }), className)} {...props} /> ) -} +}) +Badge.displayName = 'Badge'components/ui/toggle.tsx (1)
9-29: Consider breaking up the base classes string for maintainability.The base classes string on line 10 is over 500 characters long, which makes it difficult to read, maintain, and review. Consider breaking it into logical groups or using an array that's joined.
For example:
const toggleVariants = cva( - "inline-flex items-center justify-center gap-2 rounded-md text-sm font-medium hover:bg-muted hover:text-muted-foreground disabled:pointer-events-none disabled:opacity-50 data-[state=on]:bg-accent data-[state=on]:text-accent-foreground [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 [&_svg]:shrink-0 focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] outline-none transition-[color,box-shadow] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive whitespace-nowrap", + [ + // Layout & typography + "inline-flex items-center justify-center gap-2 rounded-md text-sm font-medium whitespace-nowrap", + // States + "hover:bg-muted hover:text-muted-foreground", + "disabled:pointer-events-none disabled:opacity-50", + "data-[state=on]:bg-accent data-[state=on]:text-accent-foreground", + // SVG styling + "[&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 [&_svg]:shrink-0", + // Focus & accessibility + "outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]", + "transition-[color,box-shadow]", + "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", + ].join(" "), { variants: {components/ui/table.tsx (1)
7-105: Consider adding displayName to components for better debugging.Setting
displayNameon each component improves the developer experience in React DevTools and stack traces. This is particularly valuable for a UI component library.Example for the Table component:
function Table({ className, ...props }: React.ComponentProps<'table'>) { return ( <div data-slot="table-container" className="relative w-full overflow-x-auto" > <table data-slot="table" className={cn('w-full caption-bottom text-sm', className)} {...props} /> </div> ) } +Table.displayName = 'Table'Apply the same pattern to all eight components (TableHeader, TableBody, TableFooter, TableRow, TableHead, TableCell, TableCaption).
components/ui/carousel.tsx (2)
111-111: Remove redundant property shorthand.Apply this diff:
- api: api, + api,
113-114: Simplify orientation logic.The fallback to
opts?.axisis unreachable becauseorientationhas a default value of'horizontal'(line 46). This makes the conditional logic confusing and misleading.Apply this diff to remove the dead code:
- orientation: - orientation || (opts?.axis === 'y' ? 'vertical' : 'horizontal'), + orientation,components/ui/hover-card.tsx (1)
8-42: Consider explicit ref forwarding for better TypeScript support.While the current implementation works because props are spread to Radix primitives, explicitly using
React.forwardRefprovides clearer ref semantics and improved TypeScript inference when consumers need to attach refs.Example for
HoverCardTrigger:const HoverCardTrigger = React.forwardRef< React.ElementRef<typeof HoverCardPrimitive.Trigger>, React.ComponentPropsWithoutRef<typeof HoverCardPrimitive.Trigger> >(({ ...props }, ref) => ( <HoverCardPrimitive.Trigger ref={ref} data-slot="hover-card-trigger" {...props} /> )) HoverCardTrigger.displayName = 'HoverCardTrigger'components/ui/resizable.tsx (1)
31-54: Consider extracting Tailwind classes for improved readability.The className string on line 42 is over 400 characters long, which impacts readability and makes the component harder to maintain. While this pattern is common in shadcn/ui-style components, consider extracting related utility classes into named constants or CSS classes.
Example refactor:
+const handleBaseStyles = 'bg-border focus-visible:ring-ring relative flex w-px items-center justify-center focus-visible:ring-1 focus-visible:ring-offset-1 focus-visible:outline-hidden' +const handlePseudoStyles = 'after:absolute after:inset-y-0 after:left-1/2 after:w-1 after:-translate-x-1/2' +const handleVerticalStyles = 'data-[panel-group-direction=vertical]:h-px data-[panel-group-direction=vertical]:w-full data-[panel-group-direction=vertical]:after:left-0 data-[panel-group-direction=vertical]:after:h-1 data-[panel-group-direction=vertical]:after:w-full data-[panel-group-direction=vertical]:after:translate-x-0 data-[panel-group-direction=vertical]:after:-translate-y-1/2' +const handleIconRotation = '[&[data-panel-group-direction=vertical]>div]:rotate-90' + function ResizableHandle({ withHandle, className, ...props }: React.ComponentProps<typeof ResizablePrimitive.PanelResizeHandle> & { withHandle?: boolean }) { return ( <ResizablePrimitive.PanelResizeHandle data-slot="resizable-handle" className={cn( - 'bg-border focus-visible:ring-ring relative flex w-px items-center justify-center after:absolute after:inset-y-0 after:left-1/2 after:w-1 after:-translate-x-1/2 focus-visible:ring-1 focus-visible:ring-offset-1 focus-visible:outline-hidden data-[panel-group-direction=vertical]:h-px data-[panel-group-direction=vertical]:w-full data-[panel-group-direction=vertical]:after:left-0 data-[panel-group-direction=vertical]:after:h-1 data-[panel-group-direction=vertical]:after:w-full data-[panel-group-direction=vertical]:after:translate-x-0 data-[panel-group-direction=vertical]:after:-translate-y-1/2 [&[data-panel-group-direction=vertical]>div]:rotate-90', + handleBaseStyles, + handlePseudoStyles, + handleVerticalStyles, + handleIconRotation, className, )} {...props} >components/ui/alert.tsx (1)
22-35: LGTM! Proper accessibility with role="alert".The component correctly implements the
role="alert"attribute for screen reader announcements and properly merges variant classes with custom styling.Consider adding an
asprop or using a semantic element (e.g.,<section>) if appropriate for your design system, though the currentdivimplementation provides maximum flexibility for composition.components/ui/item.tsx (1)
54-72: Forward refs throughItemfor properasChildusage.Without
forwardRef, consumers can’t attach refs (e.g., to focus anchors rendered viaasChild), which is an expected capability for these primitives. Wrapping the component inReact.forwardRefkeeps the API aligned with other UI elements and Radix patterns.-function Item({ - className, - variant = 'default', - size = 'default', - asChild = false, - role = 'listitem', - ...props -}: React.ComponentProps<'div'> & - VariantProps<typeof itemVariants> & { asChild?: boolean }) { - const Comp = asChild ? Slot : 'div' - return ( - <Comp - data-slot="item" - data-variant={variant} - data-size={size} - role={role} - className={cn(itemVariants({ variant, size, className }))} - {...props} - /> - ) -} +const Item = React.forwardRef< + React.ElementRef<'div'>, + React.ComponentProps<'div'> & + VariantProps<typeof itemVariants> & { asChild?: boolean } +>(({ className, variant = 'default', size = 'default', asChild = false, role = 'listitem', ...props }, ref) => { + const Comp = asChild ? Slot : 'div' + return ( + <Comp + ref={ref} + data-slot="item" + data-variant={variant} + data-size={size} + role={role} + className={cn(itemVariants({ variant, size, className }))} + {...props} + /> + ) +}) + +Item.displayName = 'Item'components/ui/empty.tsx (2)
5-16: Consider using semantic HTML elements.The
Emptycomponent uses a genericdivelement. For better accessibility and semantic structure, consider using a<section>element with an appropriatearia-labelto identify the empty state region.Example:
- return ( - <div - data-slot="empty" - className={cn( - 'flex min-w-0 flex-1 flex-col items-center justify-center gap-6 rounded-lg border border-dashed p-6 text-center text-balance md:p-12', - className, - )} - {...props} - /> - ) + return ( + <section + data-slot="empty" + aria-label="Empty state" + className={cn( + 'flex min-w-0 flex-1 flex-col items-center justify-center gap-6 rounded-lg border border-dashed p-6 text-center text-balance md:p-12', + className, + )} + {...props} + /> + )
61-69: Use heading element for better semantic structure.The
EmptyTitlecomponent uses adivelement for title text. For proper document structure and accessibility, use a heading element. Consider allowing the heading level to be configurable via anasprop pattern or default to an appropriate level likeh2orh3.Example with configurable heading level:
-function EmptyTitle({ className, ...props }: React.ComponentProps<'div'>) { +function EmptyTitle({ + className, + as: Component = 'h3', + ...props +}: React.ComponentProps<'div'> & { as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' }) { return ( - <div + <Component data-slot="empty-title" className={cn('text-lg font-medium tracking-tight', className)} {...props} /> ) }components/header.tsx (1)
28-112: Consider extracting navigation links to reduce duplication.The navigation links are duplicated between desktop and mobile menus, which increases maintenance burden.
Extract navigation items into a shared array:
+const navigationLinks = [ + { href: '/', label: 'Accueil' }, + { href: '/about', label: 'À propos' }, + { href: '/archives', label: 'Archives' }, + { href: '/faq', label: 'FAQ' }, + { href: '/contact', label: 'Contact' }, +] + export function Header() { const [mobileMenuOpen, setMobileMenuOpen] = useState(false) return ( <header className="fixed top-0 left-0 right-0 z-50 bg-background/80 backdrop-blur-md border-b border-border"> <nav className="container mx-auto px-4 py-4"> <div className="flex items-center justify-between"> {/* Logo */} {/* Desktop Navigation */} <div className="hidden md:flex items-center gap-8"> - <Link href="/" className="text-foreground/80 hover:text-primary transition-colors"> - Accueil - </Link> - {/* ... */} + {navigationLinks.map(link => ( + <Link key={link.href} href={link.href} className="text-foreground/80 hover:text-primary transition-colors"> + {link.label} + </Link> + ))} <Button asChild variant="outline" /* ... */> <a href="https://www.facebook.com/groups/saglac.io" /* ... */> Rejoindre </a> </Button> </div>Apply similar pattern to mobile navigation.
components/ui/button.tsx (2)
7-9: Consider splitting the long base class string for readability.The base class string on line 8 is 466 characters long, making it difficult to scan and maintain. Breaking it into a multi-line template literal or array would improve readability.
Apply this diff to improve readability:
const buttonVariants = cva( - "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none 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", + [ + "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all", + "disabled:pointer-events-none disabled:opacity-50", + "[&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0", + "outline-none 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" + ].join(" "), {
39-58: Consider using React.forwardRef for ref forwarding.The Button component doesn't explicitly forward refs, which may be needed for accessibility patterns (focus management, imperative scrolling, etc.) and integration with libraries that require refs (e.g., Radix Slot with certain use cases).
Apply this diff to add ref forwarding:
-function Button({ +const Button = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<'button'> & + VariantProps<typeof buttonVariants> & { + asChild?: boolean + } +>(function Button({ className, variant, size, asChild = false, ...props -}: React.ComponentProps<'button'> & - VariantProps<typeof buttonVariants> & { - asChild?: boolean - }) { +}, ref) { const Comp = asChild ? Slot : 'button' return ( <Comp data-slot="button" + ref={ref} className={cn(buttonVariants({ variant, size, className }))} {...props} /> ) -} +}) + +Button.displayName = 'Button'components/footer.tsx (1)
12-19: Consider removingunoptimizedfor better performance.The logo image uses
unoptimized, which bypasses Next.js image optimization (automatic format conversion, responsive sizing, lazy loading). Unless there's a specific requirement to serve the original PNG, removing this prop will improve load times and bandwidth usage.If optimization is desired, apply this diff:
<Image src="/images/logos/SaglacIO_Logo_Meetups_Inverted.png" alt="Logo de SagLac IO" width={180} height={45} className="h-10 w-auto" - unoptimized />app/faq/page.tsx (2)
68-72: Consider whether questions need full Markdown rendering.The questions are rendered with
ReactMarkdowninside theAccordionTrigger. If questions are simple text without formatting, this adds unnecessary overhead. Consider rendering questions as plain text unless they contain Markdown formatting.If questions don't use Markdown features, apply this diff:
<AccordionTrigger className="text-left hover:text-primary"> - <div className="prose prose-sm dark:prose-invert max-w-none [&>p]:my-0"> - <ReactMarkdown remarkPlugins={[remarkGfm]}>{q.question}</ReactMarkdown> - </div> + {q.question} </AccordionTrigger>
77-86: Conditional external link handling
Current FAQ content in data/faq.yml only contains external URLs, so forcingtarget="_blank"today only affects those links. To prevent future internal links from opening in new tabs, update the customacomponent in app/faq/page.tsx to applytarget="_blank"andrel="noopener noreferrer"only whenhrefmatches an external URL (e.g. starts withhttp://orhttps://and isn’t the same origin).components/faq-section.tsx (1)
35-56: Extract common Markdown rendering logic to reduce duplication.The FAQ question/answer rendering logic is duplicated between this component and
app/faq/page.tsx(lines 68-90). Consider extracting a sharedFAQItemorMarkdownRenderercomponent to maintain consistency and reduce duplication.Create a new file
components/faq-item.tsx:import ReactMarkdown from 'react-markdown' import remarkGfm from 'remark-gfm' import { AccordionContent, AccordionItem, AccordionTrigger } from '@/components/ui/accordion' interface FAQItemProps { slug: string question: string answer: string className?: string } export function FAQItem({ slug, question, answer, className }: FAQItemProps) { return ( <AccordionItem value={slug} id={slug} className={className} > <AccordionTrigger className="text-left hover:text-primary"> <div className="prose prose-sm dark:prose-invert max-w-none [&>p]:my-0"> <ReactMarkdown remarkPlugins={[remarkGfm]}>{question}</ReactMarkdown> </div> </AccordionTrigger> <AccordionContent className="text-foreground/80 leading-relaxed prose prose-sm dark:prose-invert max-w-none"> <ReactMarkdown remarkPlugins={[remarkGfm]} components={{ a: ({ node, ...props }) => ( <a {...props} target="_blank" rel="noopener noreferrer" className="text-primary hover:underline" /> ), }} > {answer} </ReactMarkdown> </AccordionContent> </AccordionItem> ) }Then use it in both components:
- {section.questions.slice(0, 3).map((q) => ( - <AccordionItem - key={q.slug} - value={q.slug} - className="bg-card border border-border rounded-lg px-6" - > - <AccordionTrigger className="text-left hover:text-primary"> - <div className="prose prose-sm dark:prose-invert max-w-none [&>p]:my-0"> - <ReactMarkdown remarkPlugins={[remarkGfm]}>{q.question}</ReactMarkdown> - </div> - </AccordionTrigger> - <AccordionContent className="text-foreground/80 leading-relaxed prose prose-sm dark:prose-invert max-w-none"> - <ReactMarkdown - remarkPlugins={[remarkGfm]} - components={{ - a: ({ node, ...props }) => ( - <a - {...props} - target="_blank" - rel="noopener noreferrer" - className="text-primary hover:underline" - /> - ), - }} - > - {q.answer} - </ReactMarkdown> - </AccordionContent> - </AccordionItem> + {section.questions.slice(0, 3).map((q) => ( + <FAQItem + key={q.slug} + slug={q.slug} + question={q.question} + answer={q.answer} + className="bg-card border border-border rounded-lg px-6" + /> ))}coverage/block-navigation.js (1)
1-87: Add defensive check for empty coverage elements.The code doesn't handle the case where
missingCoverageElementsis empty (line 20). CallinggoToNext()orgoToPrevious()when there are no elements will attempt to access index 0 of an empty NodeList, which could cause issues.Apply this diff to add a safety check:
var jumpToCode = (function init() { // Classes of code we would like to highlight in the file view var missingCoverageClasses = ['.cbranch-no', '.cstat-no', '.fstat-no']; // Elements to highlight in the file listing view var fileListingElements = ['td.pct.low']; // We don't want to select elements that are direct descendants of another match var notSelector = ':not(' + missingCoverageClasses.join('):not(') + ') > '; // becomes `:not(a):not(b) > ` // Selector that finds elements on the page to which we can jump var selector = fileListingElements.join(', ') + ', ' + notSelector + missingCoverageClasses.join(', ' + notSelector); // becomes `:not(a):not(b) > a, :not(a):not(b) > b` // The NodeList of matching elements var missingCoverageElements = document.querySelectorAll(selector); + + // Early return if no elements found + if (missingCoverageElements.length === 0) { + return function() {}; + } var currentIndex;components/__tests__/next-event.test.tsx (1)
158-171: Clarify test name and improve selector robustness.The test name "should not display speakers with only some social links" is misleading - it actually verifies that speakers with partial social links don't show missing link types. Consider renaming for clarity.
Additionally, the selector logic using
closest('div')andparentElement?.textContent(lines 162, 168) is fragile and could break with layout changes.Apply these improvements:
- it('should not display speakers with only some social links', () => { + it('should only display available social links for each speaker', () => { // Jane Smith only has GitHub, so twitter/linkedin/website should not appear for her render(<NextEvent />) - const janeAuthorEl = screen.getByText('Jane Smith').closest('div') - expect(janeAuthorEl).toBeTruthy() - // Jane should have GitHub but not twitter/linkedin/website - const allTwitterLinks = screen.getAllByTitle(/sur Twitter/) - const janeTwitter = allTwitterLinks.find((link) => - link.parentElement?.textContent?.includes('Jane Smith') - ) - expect(janeTwitter).toBeFalsy() + // Verify Jane's container has GitHub link + const janeGithub = screen.getByTitle('Jane Smith sur GitHub') + expect(janeGithub).toBeTruthy() + expect(janeGithub.getAttribute('href')).toBe('https://github.com/janesmith') + + // Verify Jane doesn't have Twitter link (John has it, Jane doesn't) + const twitterLinks = screen.getAllByTitle(/sur Twitter/) + expect(twitterLinks.length).toBe(1) // Only John's Twitter link + expect(twitterLinks[0].getAttribute('href')).toBe('https://twitter.com/johndoe') })README.md (1)
103-116: Add language specifier to code fence.The project structure diagram uses a fenced code block without a language identifier. Add
plaintextortextto suppress the linter warning and improve rendering consistency across markdown viewers.Apply this diff:
-``` +```plaintext saglac.io/ ├── app/ # Next.js App Router pagescomponents/ui/tooltip.tsx (1)
21-29: Consider hoisting TooltipProvider to app root.Each
Tooltipinstance creates its ownTooltipProvider, which is unnecessary overhead when multiple tooltips exist on a page. Per Radix UI best practices,TooltipProvidershould be rendered once at the app root to share delay configuration and improve performance.Based on learnings.
Suggested approach:
- Add
TooltipProviderto your root layout (app/layout.tsx)- Export a simple
Tooltipwrapper without the provider:function Tooltip({ ...props }: React.ComponentProps<typeof TooltipPrimitive.Root>) { return ( - <TooltipProvider> - <TooltipPrimitive.Root data-slot="tooltip" {...props} /> - </TooltipProvider> + <TooltipPrimitive.Root data-slot="tooltip" {...props} /> ) }components/newsletter-form.tsx (2)
40-41: Consider externalizing Mailchimp configuration.The Mailchimp endpoint URL is hardcoded, which makes it difficult to update or configure for different environments (development, staging, production).
Consider moving this to an environment variable or configuration file:
const url = process.env.NEXT_PUBLIC_MAILCHIMP_URL || 'https://saglac.us17.list-manage.com/subscribe/post?u=...'
93-106: Extract duplicate timeout logic.The status reset logic is duplicated in both success and error handlers. Consider extracting it to reduce duplication.
const resetStatusAfterDelay = () => { setTimeout(() => { setStatus('idle') setMessage('') }, 5000) } // Then use: resetStatusAfterDelay() in both placescomponents/ui/use-toast.ts (1)
174-182: Optimize useEffect dependency array.Including
statein the dependency array causes the effect to re-run on every state update, but the effect only needs to run once to set up the listener subscription. Removestatefrom the dependencies.Apply this diff:
React.useEffect(() => { listeners.push(setState) return () => { const index = listeners.indexOf(setState) if (index > -1) { listeners.splice(index, 1) } } - }, [state]) + }, [])components/ui/button-group.tsx (1)
7-22: Consider splitting complex variant styles.The
buttonGroupVariantsclassName contains very long, complex selector chains that may be difficult to maintain. While functional, consider extracting repeated patterns or documenting the styling strategy.For example, the focus-visible rules and child selector patterns could be documented:
// Ensures focused buttons appear above siblings (z-index stacking) // Removes left border and border-radius for joined appearanceThis is a minor maintainability concern; the current implementation works correctly.
components/ui/field.tsx (1)
128-135: Use a dedicateddata-slotforFieldTitle.Line 131 still advertises
data-slot="field-label", so any styling or selectors looking for a truefield-titleslot will miss this element and it also collides with the actual label slot. Please rename the slot to keep the contract between markup and styling/hooks consistent.- data-slot="field-label" + data-slot="field-title"components/ui/input.tsx (1)
5-19: Consider forwarding refs for form library compatibility.The Input component doesn't forward refs, which limits integration with form libraries like react-hook-form (used elsewhere in the PR per learnings) and prevents parent components from accessing the underlying input element.
Apply this diff to add ref forwarding:
-function Input({ className, type, ...props }: React.ComponentProps<'input'>) { +const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<'input'>>( + ({ className, type, ...props }, ref) => { - return ( - <input - type={type} - data-slot="input" - className={cn( - 'file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input 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', - className, - )} - {...props} - /> - ) -} + return ( + <input + type={type} + data-slot="input" + ref={ref} + className={cn( + 'file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input 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', + className, + )} + {...props} + /> + ) + } +) +Input.displayName = 'Input'app/layout.tsx (1)
79-79: Improve the Suspense fallback for better UX and consistency.The Suspense fallback displays "Loading..." in English, which is inconsistent with the French-language site (lang="fr"). Consider using a localized message, skeleton loader, or spinner for a more polished experience.
Apply this diff to use a French message:
- <Suspense fallback={<div>Loading...</div>}>{children}</Suspense> + <Suspense fallback={<div>Chargement...</div>}>{children}</Suspense>Or for a better UX, consider creating a loading skeleton component:
// components/loading-skeleton.tsx export function LoadingSkeleton() { return ( <div className="min-h-screen flex items-center justify-center"> <div className="animate-pulse text-foreground/60">Chargement...</div> </div> ) }Then update the layout:
+import { LoadingSkeleton } from '@/components/loading-skeleton' + - <Suspense fallback={<div>Loading...</div>}>{children}</Suspense> + <Suspense fallback={<LoadingSkeleton />}>{children}</Suspense>app/qrcode/page.tsx (1)
10-14: Use typed robots metadata (avoid string).Prefer the object form for type-safety and future Next validation.
-export const metadata: Metadata = { - title: 'Code QR 🤯 - SagLac IO', - description: 'Vous avez trouvé notre easter egg!', - robots: 'noindex, nofollow', // Easter egg - don't index -} +export const metadata: Metadata = { + title: 'Code QR 🤯 - SagLac IO', + description: 'Vous avez trouvé notre easter egg!', + robots: { index: false, follow: false }, // Easter egg - don't index +}components/archives-list.tsx (3)
17-29: Deduplicate avatar URL logic.This helper duplicates getAuthorAvatar in lib/data-loader (used elsewhere). Centralize to a shared util to avoid drift.
Search for existing helper and replace local copies; export a single getAuthorAvatar from a lib/util, then import it here and in components/next-event.tsx.
31-81: Extract AuthorSocialLinks to a shared component.This component is duplicated here and in components/next-event.tsx. Extract once (e.g., components/author-social-links.tsx) and reuse.
286-297: Guard against missing authors.If talk.authors can be undefined, this will throw. Prefer a safe fallback.
- {talk.authors.map((author) => ( + {(talk.authors ?? []).map((author) => (components/__tests__/archives-list.test.tsx (1)
131-137: Make pagination button assertions more robust.Text “1” can be brittle. Query by role and name instead.
-// Should show page 1, 2 (total 2 pages with 15 events, 10 per page) -expect(screen.getAllByText('1')).toHaveLength(2) // Top and bottom pagination -expect(screen.getAllByText('2')).toHaveLength(2) +// Should show page 1, 2 (total 2 pages with 15 events, 10 per page) +expect(screen.getAllByRole('button', { name: '1' })).toHaveLength(2) +expect(screen.getAllByRole('button', { name: '2' })).toHaveLength(2)components/ui/card.tsx (2)
21-24: Verify Tailwind selector variant.The class uses has-data-[slot=card-action]. Tailwind’s attribute selector variant is typically has-[data-slot=card-action]. Please verify config or adjust to the standard form.
- className={cn( - '@container/card-header grid auto-rows-min grid-rows-[auto_auto] items-start gap-2 px-6 has-data-[slot=card-action]:grid-cols-[1fr_auto] [.border-b]:pb-6', + className={cn( + '@container/card-header grid auto-rows-min grid-rows-[auto_auto] items-start gap-2 px-6 has-[data-slot=card-action]:grid-cols-[1fr_auto] [.border-b]:pb-6',
31-39: Consider polymorphic Title for semantics.CardTitle renders a div; consider asChild or heading tags for better a11y semantics where appropriate.
coverage/components/about-section.tsx.html (1)
1-30: Remove coverage artifacts from VCS.Coverage HTML should not be committed; it bloats the repo and causes noisy diffs. Generate in CI and publish as artifacts.
- Add to .gitignore:
- coverage/
- Remove committed files and rely on CI to upload coverage reports.
components/next-event.tsx (1)
9-59: Extract AuthorSocialLinks and reuse.Same component exists in components/archives-list.tsx. Extract a shared component (e.g., components/author-social-links.tsx) and import in both places.
components/ui/input-group.tsx (2)
70-75: Focus textarea as well as input on addon click.Currently only focuses input; include textarea to support multiline controls.
- e.currentTarget.parentElement?.querySelector('input')?.focus() + const root = e.currentTarget.parentElement + const control = + (root?.querySelector('input') as HTMLInputElement | null) ?? + (root?.querySelector('textarea') as HTMLTextAreaElement | null) + control?.focus()
10-35: Expose disabled state on root for styling hooks.You use group-data-[disabled=true]/input-group… but InputGroup never sets data-disabled. Consider mirroring disabled via data-disabled on the root when child control is disabled.
app/globals.css
Outdated
| @import "tailwindcss"; | ||
| @import "tw-animate-css"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Tailwind CSS v4 compatibility.
The code uses Tailwind CSS v4 syntax with @import "tailwindcss" and @theme inline. Ensure that:
- Package.json includes Tailwind CSS v4 or higher
- All team members are aware of the v4 syntax changes
- Build tools support the new @import and @theme syntax
🏁 Script executed:
#!/bin/bash
# Check Tailwind CSS version in package.json
cat package.json | jq '.dependencies.tailwindcss, .devDependencies.tailwindcss'Length of output: 13
🏁 Script executed:
#!/bin/bash
# Check if postcss.config.js includes the Tailwind CSS plugin
if [ -f postcss.config.js ]; then
grep -R "require.*tailwindcss" -n postcss.config.js || grep -R "tailwindcss" -n postcss.config.js || echo "tailwindcss plugin not found in postcss.config.js"
else
echo "postcss.config.js not found"
fiLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check if Next.js is used in this project
cat package.json | jq '.dependencies.next, .devDependencies.next'Length of output: 13
Add a root-level postcss.config.js with Tailwind CSS and Autoprefixer plugins. Without it, Next.js v15.5.5 won’t process the @import "tailwindcss" or @theme inline directives.
🤖 Prompt for AI Agents
In app/globals.css around lines 1-2, Next.js v15.5.5 will not process the
@import "tailwindcss" (or @theme inline) without a root-level PostCSS config;
add a postcss.config.js at the project root that requires and configures the
tailwindcss and autoprefixer plugins (exporting the plugins array), ensure the
file is committed at repo root (not inside app/), and restart the dev/build so
Next.js picks up and processes Tailwind directives.
| <button | ||
| className="md:hidden text-foreground" | ||
| onClick={() => setMobileMenuOpen(!mobileMenuOpen)} | ||
| aria-label="Toggle menu" | ||
| > | ||
| {mobileMenuOpen ? <X size={24} /> : <Menu size={24} />} | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add keyboard and ARIA support for mobile menu.
The mobile menu button lacks proper accessibility attributes and keyboard handling.
<button
className="md:hidden text-foreground"
onClick={() => setMobileMenuOpen(!mobileMenuOpen)}
- aria-label="Toggle menu"
+ aria-label={mobileMenuOpen ? 'Fermer le menu' : 'Ouvrir le menu'}
+ aria-expanded={mobileMenuOpen}
+ aria-controls="mobile-menu"
>
{mobileMenuOpen ? <X size={24} /> : <Menu size={24} />}
</button>Additionally, add keyboard handling to close the menu on Escape key:
useEffect(() => {
const handleEscape = (e: KeyboardEvent) => {
if (e.key === 'Escape' && mobileMenuOpen) {
setMobileMenuOpen(false)
}
}
document.addEventListener('keydown', handleEscape)
return () => document.removeEventListener('keydown', handleEscape)
}, [mobileMenuOpen])And add the id to the mobile menu container:
{mobileMenuOpen && (
- <div className="md:hidden mt-4 pb-4 flex flex-col gap-4">
+ <div id="mobile-menu" className="md:hidden mt-4 pb-4 flex flex-col gap-4">🤖 Prompt for AI Agents
In components/header.tsx around lines 56 to 62, the mobile menu toggle button
needs proper ARIA attributes and Escape-key handling: add
aria-expanded={mobileMenuOpen} and aria-controls="{mobileMenuId}" (use a stable
id string like "mobile-menu") to the button, keep or enhance the existing
aria-label, ensure the mobile menu container element has id="mobile-menu"
(matching aria-controls) and appropriate role (e.g., role="menu" or nav) as
needed, and add a useEffect hook that attaches a keydown listener to close the
menu when e.key === 'Escape' while mobileMenuOpen is true, cleaning up the
listener on unmount; also toggle focus management if necessary so focus moves
into the menu when opened.
| function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { | ||
| return ( | ||
| <tfoot | ||
| data-slot="table-footer" | ||
| className={cn( | ||
| 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix invalid CSS pseudo-class selector.
Line 47 uses [&>tr]:last:border-b-0 which contains an invalid pseudo-class :last. This should be :last-child.
Apply this diff:
function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) {
return (
<tfoot
data-slot="table-footer"
className={cn(
- 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0',
+ 'bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0',
className,
)}
{...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.
| function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { | |
| return ( | |
| <tfoot | |
| data-slot="table-footer" | |
| className={cn( | |
| 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { | |
| return ( | |
| <tfoot | |
| data-slot="table-footer" | |
| className={cn( | |
| 'bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In components/ui/table.tsx around lines 42 to 53, the CSS class string uses an
invalid pseudo-class `:last` in the token `[&>tr]:last:border-b-0`; replace
`:last` with the correct `:last-child` so the token becomes
`[&>tr]:last-child:border-b-0`, updating the className string accordingly to fix
the selector.
| import * as React from 'react' | ||
|
|
||
| import { cn } from '@/lib/utils' | ||
|
|
||
| function Textarea({ className, ...props }: React.ComponentProps<'textarea'>) { | ||
| return ( | ||
| <textarea | ||
| data-slot="textarea" | ||
| className={cn( | ||
| 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> |
There was a problem hiding this comment.
Expose this textarea as a client, ref‑forwarding component.
Without 'use client' the component can’t be imported inside client code, and without forwardRef react-hook-form and similar libraries can’t attach refs to the underlying <textarea>.
Apply this diff:
+'use client'
+
import * as React from 'react'
import { cn } from '@/lib/utils'
-function Textarea({ className, ...props }: React.ComponentProps<'textarea'>) {
- return (
- <textarea
- data-slot="textarea"
- className={cn(
- 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
- className,
- )}
- {...props}
- />
- )
-}
+const Textarea = React.forwardRef<
+ HTMLTextAreaElement,
+ React.ComponentProps<'textarea'>
+>(({ className, ...props }, ref) => (
+ <textarea
+ data-slot="textarea"
+ ref={ref}
+ className={cn(
+ 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
+ className,
+ )}
+ {...props}
+ />
+))
+
+Textarea.displayName = 'Textarea'
export { Textarea }📝 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.
| import * as React from 'react' | |
| import { cn } from '@/lib/utils' | |
| function Textarea({ className, ...props }: React.ComponentProps<'textarea'>) { | |
| return ( | |
| <textarea | |
| data-slot="textarea" | |
| className={cn( | |
| 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| 'use client' | |
| import * as React from 'react' | |
| import { cn } from '@/lib/utils' | |
| const Textarea = React.forwardRef< | |
| HTMLTextAreaElement, | |
| React.ComponentProps<'textarea'> | |
| >(({ className, ...props }, ref) => ( | |
| <textarea | |
| data-slot="textarea" | |
| ref={ref} | |
| className={cn( | |
| 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| )) | |
| Textarea.displayName = 'Textarea' | |
| export { Textarea } |
🤖 Prompt for AI Agents
In components/ui/textarea.tsx lines 1-14, the Textarea component isn't usable in
client components and doesn't forward refs; add "use client" at the top, convert
Textarea to a React.forwardRef component that accepts (props:
React.ComponentProps<'textarea'>, ref: React.Ref<HTMLTextAreaElement>) and pass
the ref into the underlying <textarea>, update the function signature to include
className and ...props as before, export the forwarded component as default (or
named export) and set displayName for debugging; ensure typing remains correct
so react-hook-form and other libs can attach refs.
| import * as React from 'react' | ||
|
|
||
| const MOBILE_BREAKPOINT = 768 | ||
|
|
||
| export function useIsMobile() { | ||
| const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) |
There was a problem hiding this comment.
Add the 'use client' directive so Next.js treats this hook as client code.
Modules that call React hooks must be marked as client components in the App Router; without it, Next.js will throw a build-time error when this hook is imported.
+'use client'
+
import * as React from 'react'📝 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.
| import * as React from 'react' | |
| const MOBILE_BREAKPOINT = 768 | |
| export function useIsMobile() { | |
| const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) | |
| 'use client' | |
| import * as React from 'react' | |
| const MOBILE_BREAKPOINT = 768 | |
| export function useIsMobile() { | |
| const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) |
🤖 Prompt for AI Agents
In components/ui/use-mobile.tsx around lines 1 to 6, this hook file must be
marked as a client module for Next.js; add the exact directive 'use client' as
the very first line of the file (before any imports) so the module is treated as
client-side and React hooks can be used without build-time errors.
| import type { ToastActionElement, ToastProps } from '@/components/ui/toast' | ||
|
|
||
| const TOAST_LIMIT = 1 | ||
| const TOAST_REMOVE_DELAY = 1000000 |
There was a problem hiding this comment.
Fix excessively long toast removal delay.
TOAST_REMOVE_DELAY = 1000000 (1,000 seconds ≈ 277 hours) appears to be unintentional. This means dismissed toasts remain in memory for over 11 days before being garbage collected, causing a memory leak in long-running sessions.
Based on typical toast patterns, this should likely be:
1000(1 second) for quick dismissal, or5000(5 seconds) for user-visible fade-out
Apply this diff:
const TOAST_LIMIT = 1
-const TOAST_REMOVE_DELAY = 1000000
+const TOAST_REMOVE_DELAY = 1000📝 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.
| const TOAST_REMOVE_DELAY = 1000000 | |
| const TOAST_LIMIT = 1 | |
| const TOAST_REMOVE_DELAY = 1000 |
🤖 Prompt for AI Agents
In components/ui/use-toast.ts around line 9, TOAST_REMOVE_DELAY is set to
1000000 (≈11.5 days) which is unintentionally long; change the constant to a
sensible toast removal timeout (e.g., replace 1000000 with 5000 for a 5s visible
fade-out, or 1000 for a 1s quick dismissal) so dismissed toasts are removed
promptly and do not cause memory buildup.
- Update package.json to use explicit port 3000 for dev server - Configure Playwright to test against static build (out/) using serve - Fix strict mode violations by using more specific selectors - Replace link click navigation with direct page navigation for reliability - Remove duplicate styles/globals.css file - Update Tailwind CSS @theme directive from 'inline' to standard - Simplify archive event verification (check for content vs specific elements) - Simplify Facebook link check to use href selector instead of text - All 14 E2E tests now passing (Desktop + Mobile Chrome)
- Add .first() to desktop navigation check to prevent strict mode violation - Test now passes consistently (verified with 10 consecutive runs) - Strict mode error occurred when multiple 'Archives' links were present
- Remove coverage/, playwright-report/, and test-results/ from git tracking - Add test artifact directories to .gitignore - These directories are generated by tests and should not be committed
There was a problem hiding this comment.
Actionable comments posted: 20
♻️ Duplicate comments (40)
.gitignore (1)
27-27: Stop ignoringnext-env.d.ts.
Line 27 hides the framework typings file that Next.js relies on; fresh checkouts lose the TypeScript refs and type-checking fails until someone regenerates it manually. Drop this entry so the file stays tracked.# typescript *.tsbuildinfo -next-env.d.tscomponents/ui/select.tsx (1)
27-171: Restore ref forwarding on the wrapped primitives.All of these wrappers (
SelectTrigger,SelectContent,SelectLabel,SelectItem,SelectSeparator,SelectScrollUpButton,SelectScrollDownButton) are still plain functions, so any ref consumers pass is silently dropped. That breaks focusing, scroll positioning, form libs, etc.—the exact issue called out previously. Please convert each wrapper toReact.forwardRef, pass the ref through to the underlying Radix primitive, and set adisplayNamefor DevTools parity.Representative diff for
SelectTrigger; apply the same pattern to the other wrappers:-function SelectTrigger({ - className, - size = 'default', - children, - ...props -}: React.ComponentProps<typeof SelectPrimitive.Trigger> & { - size?: 'sm' | 'default' -}) { - return ( - <SelectPrimitive.Trigger - data-slot="select-trigger" - data-size={size} - className={cn(/* … */)} - {...props} - > - {children} - … - </SelectPrimitive.Trigger> - ) -} +const SelectTrigger = React.forwardRef< + React.ElementRef<typeof SelectPrimitive.Trigger>, + React.ComponentPropsWithoutRef<typeof SelectPrimitive.Trigger> & { + size?: 'sm' | 'default' + } +>(({ className, size = 'default', children, ...props }, ref) => ( + <SelectPrimitive.Trigger + ref={ref} + data-slot="select-trigger" + data-size={size} + className={cn(/* … */)} + {...props} + > + {children} + … + </SelectPrimitive.Trigger> +)) +SelectTrigger.displayName = SelectPrimitive.Trigger.displayName ?? 'SelectTrigger'Repeat for
SelectContent,SelectLabel,SelectItem,SelectSeparator,SelectScrollUpButton, andSelectScrollDownButton, adjusting the primitive type and prop typings accordingly.components/ui/item.tsx (1)
54-70: Restore list semantics by defaulting Item torole="listitem".
ItemGroupadvertises itself as a list, butItemomits the matchinglistitemrole, so assistive tech won’t treat the children as list entries. Please defaultroleto"listitem"(still allowing overrides) and pass it through toComp.function Item({ className, variant = 'default', size = 'default', asChild = false, + role = 'listitem', ...props }: React.ComponentProps<'div'> & VariantProps<typeof itemVariants> & { asChild?: boolean }) { const Comp = asChild ? Slot : 'div' return ( <Comp data-slot="item" data-variant={variant} data-size={size} + role={role} className={cn(itemVariants({ variant, size, className }))} {...props} /> ) }components/ui/slider.tsx (1)
16-24: Fix default thumb count fallback (duplicate issue).This fallback still creates two thumbs when neither
valuenordefaultValueis provided, causing single-value sliders to unexpectedly render as range selectors. The suggested fix from the previous review remains valid and should be applied.Apply the previously suggested diff:
- const _values = React.useMemo( - () => - Array.isArray(value) - ? value - : Array.isArray(defaultValue) - ? defaultValue - : [min, max], - [value, defaultValue, min, max], - ) + const thumbCount = React.useMemo(() => { + if (Array.isArray(value)) return value.length + if (Array.isArray(defaultValue)) return defaultValue.length + return 1 + }, [value, defaultValue])And update line 52:
- {Array.from({ length: _values.length }, (_, index) => ( + {Array.from({ length: thumbCount }, (_, index) => (components/ui/hover-card.tsx (1)
35-35: Critical: Invalid Tailwind syntax still present (previously flagged).The
origin-(--radix-hover-card-content-transform-origin)syntax remains invalid. Tailwind's parentheses syntax is for opacity modifiers, not CSS variables. This will cause the transform-origin to not be applied, breaking the hover card's animation transform origin.Apply this diff to fix the transform-origin:
- '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 w-64 origin-(--radix-hover-card-content-transform-origin) rounded-md border p-4 shadow-md outline-hidden', + '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 w-64 origin-[var(--radix-hover-card-content-transform-origin)] rounded-md border p-4 shadow-md outline-hidden',components/ui/empty.tsx (2)
10-10: Add border width class for dashed border to be visible.The
border-dashedutility only sets the border style but does not define border width. Without a border-width class likeborderorborder-2, the dashed border will not render.Apply this diff to add the border width:
- 'flex min-w-0 flex-1 flex-col items-center justify-center gap-6 rounded-lg border-dashed p-6 text-center text-balance md:p-12', + 'flex min-w-0 flex-1 flex-col items-center justify-center gap-6 rounded-lg border border-dashed p-6 text-center text-balance md:p-12',
71-82: Fix type mismatch between component props and rendered element.The
EmptyDescriptioncomponent is typed asReact.ComponentProps<'p'>but renders a<div>element (line 73). This type mismatch can lead to incorrect prop typing and semantic confusion.Apply this diff to use the correct element that matches the type signature:
function EmptyDescription({ className, ...props }: React.ComponentProps<'p'>) { return ( - <div + <p data-slot="empty-description" className={cn( 'text-muted-foreground [&>a:hover]:text-primary text-sm/relaxed [&>a]:underline [&>a]:underline-offset-4', className, )} {...props} /> ) }components/ui/table.tsx (1)
42-53: Fix invalid CSS pseudo-class selector.Line 47 still uses
[&>tr]:last:border-b-0which contains an invalid pseudo-class:last. This should be:last-child.Apply this diff:
function TableFooter({ className, ...props }: React.ComponentProps<'tfoot'>) { return ( <tfoot data-slot="table-footer" className={cn( - 'bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', + 'bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0', className, )} {...props} /> ) }components/ui/chart.tsx (1)
233-239: Ensure zero values render in the tooltip.The guard
item.value && (…)still hides legitimate zero-valued datapoints, so users never see a tooltip for zeros. Please switch to an explicit null/undefined check so 0 displays correctly. This was already reported earlier and remains unresolved.- {item.value && ( + {item.value !== undefined && item.value !== null && (components/ui/spinner.tsx (1)
1-3: The 'use client' directive is still missing.This issue was previously flagged. For Next.js App Router compatibility, add the 'use client' directive as the first line of the file to prevent server→client boundary errors when this component is imported into client code.
Apply this diff:
+'use client' + import { Loader2Icon } from 'lucide-react' import { cn } from '@/lib/utils'components/ui/carousel.tsx (3)
78-89: Restore vertical keyboard navigation
handleKeyDownstill ignores Up/Down whenorientation="vertical", so vertical carousels remain keyboard-inaccessible. Please branch by orientation (and update the dependencies) so both axes work.const handleKeyDown = React.useCallback( (event: React.KeyboardEvent<HTMLDivElement>) => { - if (event.key === 'ArrowLeft') { + const isHorizontal = orientation === 'horizontal' + const prevKey = isHorizontal ? 'ArrowLeft' : 'ArrowUp' + const nextKey = isHorizontal ? 'ArrowRight' : 'ArrowDown' + + if (event.key === prevKey) { event.preventDefault() scrollPrev() - } else if (event.key === 'ArrowRight') { + } else if (event.key === nextKey) { event.preventDefault() scrollNext() } }, - [scrollPrev, scrollNext], + [orientation, scrollPrev, scrollNext], )
96-105: Remove all listeners in cleanupThe effect registers both
'reInit'and'select', but the cleanup still drops only'select', so'reInit'handlers accumulate on rerender/unmount. Please unregister both.api.on('select', onSelect) return () => { + api?.off('reInit', onSelect) api?.off('select', onSelect) }
121-128: Label the carousel region
role="region"without a label is an unlabeled landmark. Provide a defaultaria-label(allowing callers to override via props).className={cn('relative', className)} role="region" aria-roledescription="carousel" + aria-label="Carousel" data-slot="carousel" {...props}components/ui/use-toast.ts (1)
8-9: Acknowledge prior review: TOAST_REMOVE_DELAY is excessively long.A previous review correctly identified that
TOAST_REMOVE_DELAY = 1000000(1,000 seconds ≈ 16.7 minutes) is unintentionally long and should be reduced to 1-5 seconds for proper toast cleanup.components/ui/menubar.tsx (4)
59-59: Duplicate: Inconsistent outline utility.This issue was already flagged in previous review comments. The
outline-hiddenutility should be replaced with the standardoutline-none.
106-106: Duplicate: Inconsistent outline utility.This issue was already flagged in previous review comments. The
outline-hiddenutility should be replaced with the standardoutline-none.
124-124: Duplicate: Non-standard Tailwind utilities.This issue was already flagged in previous review comments. Both
outline-hiddenandrounded-xsshould be replaced with standard Tailwind utilities.
149-149: Duplicate: Non-standard Tailwind utilities.This issue was already flagged in previous review comments. Both
outline-hiddenandrounded-xsshould be replaced with standard Tailwind utilities (consistent with the MenubarCheckboxItem fix).components/newsletter-form.tsx (1)
44-82: Removetarget="_blank"to prevent unwanted popup.The form submission opens in a new window/tab (line 47), which creates a poor user experience. Since the form is immediately removed after submission and you're showing success/error states in the UI, there's no need to navigate away.
Apply this diff:
const form = document.createElement('form') form.method = 'POST' form.action = url - form.target = '_blank' form.style.display = 'none'components/ui/kbd.tsx (1)
18-26: Fix type mismatch and semantic HTML violation.The component has two issues:
- Type mismatch: Props are typed as
React.ComponentProps<'div'>(line 18) but the component renders a<kbd>element (line 20), violating type safety.- Semantic violation: The
<kbd>element should represent individual keyboard keys, not group containers. Using it as a wrapper violates HTML semantics.Apply this diff to fix both issues by using a
<div>element:function KbdGroup({ className, ...props }: React.ComponentProps<'div'>) { return ( - <kbd + <div data-slot="kbd-group" className={cn('inline-flex items-center gap-1', className)} {...props} /> ) }components/ui/use-mobile.tsx (1)
1-19: Add the'use client'directive so Next.js treats this hook as client code.Modules that call React hooks must be marked as client components in the App Router; without it, Next.js will throw a build-time error when this hook is imported.
Apply this diff:
+'use client' + import * as React from 'react'coverage/components/archives-list.tsx.html (1)
1-1081: Remove coverage artifacts from version control.Coverage HTML reports are build artifacts and should not be committed. This was already flagged in a previous review.
Add coverage reports to
.gitignore:+# Coverage reports +coverage/coverage/block-navigation.js (1)
1-87: Remove coverage artifacts from version control.This JavaScript file is part of the coverage report infrastructure and should not be committed. This issue was already flagged for the coverage HTML files.
This file will be excluded when you add
coverage/to.gitignoreas suggested in the review of the coverage HTML file.components/ui/alert.tsx (1)
42-44: Make alert title truncation opt-inLine 42 still applies
line-clamp-1, forcing every title into a single line and risking hidden critical text (same concern noted previously). Please drop the clamp or gate it behind a prop/variant so consumers can opt in when appropriate.- 'col-start-2 line-clamp-1 min-h-4 font-medium tracking-tight', + 'col-start-2 min-h-4 font-medium tracking-tight',components/ui/calendar.tsx (1)
175-209: Forward the day-button ref to preserve keyboard focusDayPicker injects a ref into
CalendarDayButton. Because this component isn’t wrapped inReact.forwardRef, React warns and the roving focus logic fails—keyboard users can’t rely on the built-in focus management. Convert it toforwardRefand merge the forwarded ref with the local one used for the autofocus effect, as outlined below.-function CalendarDayButton({ - className, - day, - modifiers, - ...props -}: React.ComponentProps<typeof DayButton>) { +const CalendarDayButton = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<typeof DayButton> +>(function CalendarDayButton( + { className, day, modifiers, ...props }, + forwardedRef, +) { const defaultClassNames = getDefaultClassNames() - const ref = React.useRef<HTMLButtonElement>(null) + const localRef = React.useRef<HTMLButtonElement | null>(null) + const mergedRef = React.useCallback( + (node: HTMLButtonElement | null) => { + localRef.current = node + if (typeof forwardedRef === 'function') { + forwardedRef(node) + } else if (forwardedRef) { + forwardedRef.current = node + } + }, + [forwardedRef], + ) + React.useEffect(() => { - if (modifiers.focused) ref.current?.focus() + if (modifiers.focused) localRef.current?.focus() }, [modifiers.focused]) return ( <Button - ref={ref} + ref={mergedRef} variant="ghost" size="icon" @@ {...props} /> ) -} +})components/ui/pagination.tsx (3)
68-81: Allow overriding the “Previous” labelExpose
childrenand fall back to the current icon/label so consumers can localize or customize the text.function PaginationPrevious({ className, + children, ...props }: React.ComponentProps<typeof PaginationLink>) { return ( <PaginationLink aria-label="Go to previous page" size="default" className={cn('gap-1 px-2.5 sm:pl-2.5', className)} {...props} > - <ChevronLeftIcon /> - <span className="hidden sm:block">Previous</span> + {children ?? ( + <> + <ChevronLeftIcon /> + <span className="hidden sm:block">Previous</span> + </> + )} </PaginationLink> ) }
85-98: Allow overriding the “Next” labelSame rationale as the previous control—let consumers provide custom content for localization.
function PaginationNext({ className, + children, ...props }: React.ComponentProps<typeof PaginationLink>) { return ( <PaginationLink aria-label="Go to next page" size="default" className={cn('gap-1 px-2.5 sm:pr-2.5', className)} {...props} > - <span className="hidden sm:block">Next</span> - <ChevronRightIcon /> + {children ?? ( + <> + <span className="hidden sm:block">Next</span> + <ChevronRightIcon /> + </> + )} </PaginationLink> ) }
102-115: Fix ellipsis semantics and SR labelingRendering a span with
aria-hiddenhides the label from AT and breaks list semantics. Use anli, expose the label, and keep the icon decorative.-function PaginationEllipsis({ - className, - ...props -}: React.ComponentProps<'span'>) { +function PaginationEllipsis({ + className, + ...props +}: React.ComponentProps<'li'>) { return ( - <span - aria-hidden - data-slot="pagination-ellipsis" - className={cn('flex size-9 items-center justify-center', className)} - {...props} - > - <MoreHorizontalIcon className="size-4" /> - <span className="sr-only">More pages</span> - </span> + <li + data-slot="pagination-ellipsis" + className={cn('flex', className)} + aria-label="More pages" + {...props} + > + <span className="flex size-9 items-center justify-center"> + <MoreHorizontalIcon className="size-4" aria-hidden="true" /> + </span> + </li> ) }components/ui/dropdown-menu.tsx (2)
45-48: Fix Tailwind arbitrary values
max-h-(...)andorigin-(...)won’t compile—Tailwind expects bracket notation withvar(...).- '... z-50 max-h-(--radix-dropdown-menu-content-available-height) min-w-[8rem] origin-(--radix-dropdown-menu-content-transform-origin) ...', + '... z-50 max-h-[var(--radix-dropdown-menu-content-available-height)] min-w-[8rem] origin-[var(--radix-dropdown-menu-content-transform-origin)] ...',- '... z-50 min-w-[8rem] origin-(--radix-dropdown-menu-content-transform-origin) ...', + '... z-50 min-w-[8rem] origin-[var(--radix-dropdown-menu-content-transform-origin)] ...',Also applies to: 233-235
77-79: Replace invalid Tailwind selector
data-[variant=destructive]:*:[svg]is not valid; use an arbitrary descendant variant.- "… data-[variant=destructive]:*:[svg]:!text-destructive …", + "… data-[variant=destructive]:[&_svg]:!text-destructive …",components/ui/command.tsx (1)
55-56: Remove invalid “:” Tailwind selector**The
**:prefix breaks Tailwind parsing; convert to an arbitrary descendant selector referencing your data-slot.- <Command className="[&_[cmdk-group-heading]]:text-muted-foreground **:data-[slot=command-input-wrapper]:h-12 …"> + <Command className="[&_[cmdk-group-heading]]:text-muted-foreground [&_[data-slot=command-input-wrapper]]:h-12 …">components/ui/context-menu.tsx (1)
88-108: Fix invalid Tailwind arbitrary values.Tailwind ignores
origin-(--radix-…)andmax-h-(--radix-…); arbitrary values must be wrapped in square brackets (and should reference the CSS variable viavar(...)). Today the menu renders without the intended transform-origin and available-height cap. Update these classes (and any siblings) so they compile.- '... origin-(--radix-context-menu-content-transform-origin) ...', + '... origin-[var(--radix-context-menu-content-transform-origin)] ...', @@ - '... max-h-(--radix-context-menu-content-available-height) ... origin-(--radix-context-menu-content-transform-origin) ...', + '... max-h-[var(--radix-context-menu-content-available-height)] ... origin-[var(--radix-context-menu-content-transform-origin)] ...',components/ui/alert-dialog.tsx (1)
15-143: Forward refs through the AlertDialog wrappers.All wrappers (
AlertDialogTrigger,Overlay,Content,Title,Action,Cancel, etc.) are plain function components, so any consumerrefis discarded before reaching the Radix primitive. This breaks focus control and integration with Radix. Please switch each wrapper toReact.forwardRef(with matching element types anddisplayName) so refs propagate. Based on learningscomponents/ui/textarea.tsx (1)
1-18: Make Textarea a client, ref-forwarding component.This UI primitive still renders as a server component and drops refs, so any client consumer (react-hook-form, focus mgmt) breaks. Please restore the
'use client'directive and wrap it inReact.forwardRef, passing the ref through to<textarea>and settingdisplayName.Apply this diff:
+ 'use client' + import * as React from 'react' import { cn } from '@/lib/utils' - function Textarea({ className, ...props }: React.ComponentProps<'textarea'>) { - return ( - <textarea - data-slot="textarea" - className={cn( - 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', - className, - )} - {...props} - /> - ) - } + const Textarea = React.forwardRef< + HTMLTextAreaElement, + React.ComponentProps<'textarea'> + >(({ className, ...props }, ref) => ( + <textarea + data-slot="textarea" + ref={ref} + className={cn( + 'border-input placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 flex field-sizing-content min-h-16 w-full rounded-md border bg-transparent px-3 py-2 text-base shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 md:text-sm', + className, + )} + {...props} + /> + )) + + Textarea.displayName = 'Textarea' export { Textarea }components/header.tsx (1)
56-113: Restore accessible mobile-menu controls.The toggle still lacks
aria-expanded/aria-controls, and there’s no Escape-key close handler, so keyboard and AT users can’t reliably operate the menu. Please wire the ARIA attributes, add an id on the panel, and attach an Escape listener when open (cleared on unmount).-import { useState } from "react" +import { useEffect, useState } from "react" @@ -export function Header() { +const MOBILE_MENU_ID = "mobile-menu" + +export function Header() { const [mobileMenuOpen, setMobileMenuOpen] = useState(false) + + useEffect(() => { + if (!mobileMenuOpen) return + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape") { + setMobileMenuOpen(false) + } + } + + window.addEventListener("keydown", handleKeyDown) + return () => window.removeEventListener("keydown", handleKeyDown) + }, [mobileMenuOpen]) @@ - <button + <button className="md:hidden text-foreground" onClick={() => setMobileMenuOpen(!mobileMenuOpen)} - aria-label="Toggle menu" + aria-label={mobileMenuOpen ? "Fermer le menu mobile" : "Ouvrir le menu mobile"} + aria-expanded={mobileMenuOpen} + aria-controls={MOBILE_MENU_ID} > @@ - {mobileMenuOpen && ( - <div className="md:hidden mt-4 pb-4 flex flex-col gap-4"> + {mobileMenuOpen && ( + <div + id={MOBILE_MENU_ID} + className="md:hidden mt-4 pb-4 flex flex-col gap-4" + role="menu" + >.github/workflows/README.md (3)
58-59: Past review comment already flagged this issue.The mismatch between documented retention (7 days) and actual workflow retention (30 days) for the playwright-report artifact was already identified in a previous review.
214-217: Past review comment already flagged this issue.The artifact retention documentation inconsistency was already identified in a previous review.
124-129: Past review comment already flagged this issue.The action version reference mismatch (documented as @v4, actual workflow uses @v5) was already identified in a previous review.
components/ui/breadcrumb.tsx (2)
52-63: Remove link semantics from current page.Current page should not expose link role nor aria-disabled. Use only aria-current="page" on a non-interactive element.
function BreadcrumbPage({ className, ...props }: React.ComponentProps<'span'>) { return ( <span data-slot="breadcrumb-page" - role="link" - aria-disabled="true" aria-current="page" className={cn('text-foreground font-normal', className)} {...props} /> ) }
83-99: Fix conflicting aria-hidden with sr-only text.Parent aria-hidden hides the subtree, including “More”. Either remove aria-hidden on the container (and keep icon aria-hidden) or drop the sr-only label if decorative.
function BreadcrumbEllipsis({ className, ...props }: React.ComponentProps<'span'>) { return ( <span data-slot="breadcrumb-ellipsis" role="presentation" - aria-hidden="true" className={cn('flex size-9 items-center justify-center', className)} {...props} > - <MoreHorizontal className="size-4" /> + <MoreHorizontal className="size-4" aria-hidden="true" /> <span className="sr-only">More</span> </span> ) }
🧹 Nitpick comments (21)
components/ui/radio-group.tsx (2)
9-20: Consider adding displayName for improved debugging.Setting
displayNameon both components helps identify them in React DevTools, especially when wrapped in higher-order components or used in complex component trees.Apply this diff to add displayName:
function RadioGroup({ className, ...props }: React.ComponentProps<typeof RadioGroupPrimitive.Root>) { return ( <RadioGroupPrimitive.Root data-slot="radio-group" className={cn('grid gap-3', className)} {...props} /> ) } +RadioGroup.displayName = 'RadioGroup' function RadioGroupItem({ className, ...props }: React.ComponentProps<typeof RadioGroupPrimitive.Item>) { return ( <RadioGroupPrimitive.Item data-slot="radio-group-item" className={cn( 'border-input text-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 aspect-square size-4 shrink-0 rounded-full border shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50', className, )} {...props} > <RadioGroupPrimitive.Indicator data-slot="radio-group-indicator" className="relative flex items-center justify-center" > <CircleIcon className="fill-primary absolute top-1/2 left-1/2 size-2 -translate-x-1/2 -translate-y-1/2" /> </RadioGroupPrimitive.Indicator> </RadioGroupPrimitive.Item> ) } +RadioGroupItem.displayName = 'RadioGroupItem'Also applies to: 22-43
29-32: Optional: Break up className string for readability.The long className string combines multiple concerns (states, themes, layout). Grouping related classes can improve maintainability.
Example grouping by concern:
className={cn( - 'border-input text-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 aspect-square size-4 shrink-0 rounded-full border shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50', + // Base layout & appearance + 'aspect-square size-4 shrink-0 rounded-full border shadow-xs', + // Colors & theming + 'border-input text-primary dark:bg-input/30', + // Focus states + 'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] outline-none', + // Invalid/error states + 'aria-invalid:border-destructive aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40', + // Disabled state + 'disabled:cursor-not-allowed disabled:opacity-50', + // Transitions + 'transition-[color,box-shadow]', className, )}components/ui/aspect-ratio.tsx (1)
5-9: Consider forwarding refs for better component flexibility.The component correctly forwards props and adds the data-slot attribute. However, wrapping Radix primitives without ref forwarding limits consumers who may need to access the underlying DOM element (e.g., for measuring, focus management, or imperative APIs).
Apply this diff to forward refs:
-function AspectRatio({ +const AspectRatio = React.forwardRef< + React.ElementRef<typeof AspectRatioPrimitive.Root>, + React.ComponentPropsWithoutRef<typeof AspectRatioPrimitive.Root> +>(({ ...props -}: React.ComponentProps<typeof AspectRatioPrimitive.Root>) { - return <AspectRatioPrimitive.Root data-slot="aspect-ratio" {...props} /> -} +}, ref) => { + return <AspectRatioPrimitive.Root ref={ref} data-slot="aspect-ratio" {...props} /> +}) + +AspectRatio.displayName = 'AspectRatio'The displayName assignment aids debugging in React DevTools.
components/ui/badge.tsx (1)
7-26: Consider using a semantic color token for destructive variant text.Line 17 uses
text-whitefor the destructive variant, while other variants consistently use semantic color tokens (text-primary-foreground,text-secondary-foreground,text-foreground). Using a semantic token liketext-destructive-foregroundwould improve theme consistency and adaptability.Apply this diff to align with the semantic token pattern:
destructive: - 'border-transparent bg-destructive text-white [a&]:hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60', + 'border-transparent bg-destructive text-destructive-foreground [a&]:hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60',components/ui/tabs.tsx (2)
8-19: Consider adding ref forwarding to all tab components.The components don't forward refs, which may limit consumer usage patterns like programmatic focus management, animations, or DOM measurements. Since these are wrapper components around Radix primitives that support refs, forwarding would be beneficial.
Apply this pattern to all four components (example for
Tabs):-function Tabs({ +const Tabs = React.forwardRef< + React.ElementRef<typeof TabsPrimitive.Root>, + React.ComponentPropsWithoutRef<typeof TabsPrimitive.Root> +>(function Tabs({ className, ...props -}: React.ComponentProps<typeof TabsPrimitive.Root>) { +}, ref) { return ( <TabsPrimitive.Root + ref={ref} data-slot="tabs" className={cn('flex flex-col gap-2', className)} {...props} /> ) -} +})Repeat for
TabsList,TabsTrigger, andTabsContentusing their respective primitive types.
44-46: Long className string reduces readability.The
TabsTriggerclassName spans multiple concerns (states, focus, colors, sizing) in a single 400+ character string, making it harder to scan and maintain.Consider breaking it into logical groups:
className={cn( - "data-[state=active]:bg-background dark:data-[state=active]:text-foreground focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 text-foreground dark:text-muted-foreground inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:shadow-sm [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", + // Base layout & typography + 'inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5', + 'rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap', + // Colors + 'text-foreground dark:text-muted-foreground', + // Active state + 'data-[state=active]:bg-background data-[state=active]:shadow-sm', + 'dark:data-[state=active]:text-foreground dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30', + // Focus state + 'focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring focus-visible:ring-[3px] focus-visible:outline-1', + // Disabled & transitions + 'disabled:pointer-events-none disabled:opacity-50 transition-[color,box-shadow]', + // SVG styles + '[&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*="size-"])]:size-4', className, )}components/ui/resizable.tsx (2)
25-29: Consider adding className support for consistency.
ResizablePaneldoesn't accept aclassNameprop, unlikeResizablePanelGroupandResizableHandle. This creates an inconsistent API. Consider adding className support to allow custom styling:function ResizablePanel({ + className, ...props }: React.ComponentProps<typeof ResizablePrimitive.Panel>) { - return <ResizablePrimitive.Panel data-slot="resizable-panel" {...props} /> + return ( + <ResizablePrimitive.Panel + data-slot="resizable-panel" + className={cn(className)} + {...props} + /> + ) }
31-54: LGTM with optional readability suggestion.The component implementation is solid. The
withHandleprop provides nice flexibility, and the accessibility styles (focus-visible) are well-implemented. The conditional icon rendering is clean.The className string at lines 41-43 is very long but functionally correct. If desired, you could extract repeated patterns or break it into logical groups for easier maintenance.
components/ui/scroll-area.tsx (1)
8-29: Consider exposing ScrollBar orientation for flexibility.The component always renders a vertical ScrollBar. For use cases requiring horizontal scrolling, you may want to expose the orientation prop.
Apply this diff to add optional orientation support:
function ScrollArea({ className, children, + orientation = 'vertical', ...props }: React.ComponentProps<typeof ScrollAreaPrimitive.Root>) { return ( <ScrollAreaPrimitive.Root data-slot="scroll-area" className={cn('relative', className)} {...props} > <ScrollAreaPrimitive.Viewport data-slot="scroll-area-viewport" className="focus-visible:ring-ring/50 size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-[3px] focus-visible:outline-1" > {children} </ScrollAreaPrimitive.Viewport> - <ScrollBar /> + <ScrollBar orientation={orientation} /> <ScrollAreaPrimitive.Corner /> </ScrollAreaPrimitive.Root> ) }components/ui/use-toast.ts (1)
90-114: Consider extracting side effects from reducer.The
DISMISS_TOASTcase callsaddToRemoveQueuedirectly within the reducer, which is an anti-pattern (reducers should be pure). While the inline comment acknowledges this, extracting side effects into a separate action creator or middleware would improve testability and maintainability.Example refactor:
// Replace dismiss dispatch call with a helper that handles side effects function dismissToast(toastId?: string) { if (toastId) { addToRemoveQueue(toastId) } else { memoryState.toasts.forEach((toast) => addToRemoveQueue(toast.id)) } dispatch({ type: 'DISMISS_TOAST', toastId }) }Then update the reducer to be pure:
case 'DISMISS_TOAST': { const { toastId } = action - - // ! Side effects ! - This could be extracted into a dismissToast() action, - // but I'll keep it here for simplicity - if (toastId) { - addToRemoveQueue(toastId) - } else { - state.toasts.forEach((toast) => { - addToRemoveQueue(toast.id) - }) - } return { ...state, toasts: state.toasts.map((t) => t.id === toastId || toastId === undefined ? { ...t, open: false } : t, ), } }components/ui/toggle.tsx (1)
9-29: Consider breaking up the long base classes string for readability.The base classes string on line 10 is over 400 characters long, making it difficult to scan and maintain. While functionally correct, splitting it into logical groups with line breaks would improve readability.
Example refactor:
const toggleVariants = cva( - "inline-flex items-center justify-center gap-2 rounded-md text-sm font-medium hover:bg-muted hover:text-muted-foreground disabled:pointer-events-none disabled:opacity-50 data-[state=on]:bg-accent data-[state=on]:text-accent-foreground [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 [&_svg]:shrink-0 focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] outline-none transition-[color,box-shadow] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive whitespace-nowrap", + [ + // Layout & typography + "inline-flex items-center justify-center gap-2 rounded-md text-sm font-medium whitespace-nowrap", + // Interactive states + "hover:bg-muted hover:text-muted-foreground", + "disabled:pointer-events-none disabled:opacity-50", + "data-[state=on]:bg-accent data-[state=on]:text-accent-foreground", + // SVG styling + "[&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 [&_svg]:shrink-0", + // Focus & validation + "focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] outline-none", + "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", + // Transitions + "transition-[color,box-shadow]", + ].join(" "), { variants: {components/ui/input-otp.tsx (1)
53-56: Consider refactoring the long className for readability.The className on line 54 is over 500 characters long, making it difficult to review and maintain. Consider breaking it into a more readable format.
Apply this diff to improve readability:
<div data-slot="input-otp-slot" data-active={isActive} className={cn( - 'data-[active=true]:border-ring data-[active=true]:ring-ring/50 data-[active=true]:aria-invalid:ring-destructive/20 dark:data-[active=true]:aria-invalid:ring-destructive/40 aria-invalid:border-destructive data-[active=true]:aria-invalid:border-destructive dark:bg-input/30 border-input relative flex h-9 w-9 items-center justify-center border-y border-r text-sm shadow-xs transition-all outline-none first:rounded-l-md first:border-l last:rounded-r-md data-[active=true]:z-10 data-[active=true]:ring-[3px]', + [ + // Base styles + 'relative flex h-9 w-9 items-center justify-center', + 'border-y border-r border-input text-sm shadow-xs', + 'transition-all outline-none', + // Border radius + 'first:rounded-l-md first:border-l last:rounded-r-md', + // Dark mode + 'dark:bg-input/30', + // Invalid state + 'aria-invalid:border-destructive', + // Active state + 'data-[active=true]:z-10 data-[active=true]:border-ring', + 'data-[active=true]:ring-[3px] data-[active=true]:ring-ring/50', + // Active + invalid state + 'data-[active=true]:aria-invalid:border-destructive', + 'data-[active=true]:aria-invalid:ring-destructive/20', + 'dark:data-[active=true]:aria-invalid:ring-destructive/40', + ].join(' '), className, )} {...props}components/ui/label.tsx (1)
8-22: Forward refs through Label.Radix primitives are ref-aware; without forwarding, consumers can’t focus/manage the underlying element consistently with the rest of the UI kit. Please wrap this in
React.forwardRefand setdisplayName.- function Label({ - className, - ...props - }: React.ComponentProps<typeof LabelPrimitive.Root>) { - return ( - <LabelPrimitive.Root - data-slot="label" - className={cn( - 'flex items-center gap-2 text-sm leading-none font-medium select-none group-data-[disabled=true]:pointer-events-none group-data-[disabled=true]:opacity-50 peer-disabled:cursor-not-allowed peer-disabled:opacity-50', - className, - )} - {...props} - /> - ) - } + const Label = React.forwardRef< + React.ElementRef<typeof LabelPrimitive.Root>, + React.ComponentPropsWithoutRef<typeof LabelPrimitive.Root> + >(({ className, ...props }, ref) => ( + <LabelPrimitive.Root + data-slot="label" + ref={ref} + className={cn( + 'flex items-center gap-2 text-sm leading-none font-medium select-none group-data-[disabled=true]:pointer-events-none group-data-[disabled=true]:opacity-50 peer-disabled:cursor-not-allowed peer-disabled:opacity-50', + className, + )} + {...props} + /> + )) + + Label.displayName = LabelPrimitive.Root.displayName ?? 'Label' export { Label }app/layout.tsx (1)
78-80: Consider enhancing the Suspense fallback.The current
<div>Loading...</div>fallback is functional but could be improved for better UX and accessibility. Consider adding a visually styled loading indicator with proper ARIA attributes.Example enhancement:
- <Suspense fallback={<div>Loading...</div>}>{children}</Suspense> + <Suspense fallback={ + <div className="flex items-center justify-center min-h-screen" role="status" aria-live="polite"> + <span className="text-lg text-foreground/60">Chargement...</span> + </div> + }>{children}</Suspense>components/ui/avatar.tsx (1)
8-22: Forward refs and set displayName for better DX and composability.Enable refs for focus/measure and improve DevTools names. Typical shadcn/Radix pattern uses forwardRef + displayName.
@@ -function Avatar({ - className, - ...props -}: React.ComponentProps<typeof AvatarPrimitive.Root>) { - return ( - <AvatarPrimitive.Root - data-slot="avatar" - className={cn( - 'relative flex size-8 shrink-0 overflow-hidden rounded-full', - className, - )} - {...props} - /> - ) -} +const Avatar = React.forwardRef< + React.ElementRef<typeof AvatarPrimitive.Root>, + React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Root> +>(({ className, ...props }, ref) => ( + <AvatarPrimitive.Root + ref={ref} + data-slot="avatar" + className={cn('relative flex size-8 shrink-0 overflow-hidden rounded-full', className)} + {...props} + /> +)) +Avatar.displayName = AvatarPrimitive.Root.displayName @@ -function AvatarImage({ - className, - ...props -}: React.ComponentProps<typeof AvatarPrimitive.Image>) { - return ( - <AvatarPrimitive.Image - data-slot="avatar-image" - className={cn('aspect-square size-full', className)} - {...props} - /> - ) -} +const AvatarImage = React.forwardRef< + React.ElementRef<typeof AvatarPrimitive.Image>, + React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Image> +>(({ className, ...props }, ref) => ( + <AvatarPrimitive.Image + ref={ref} + data-slot="avatar-image" + className={cn('aspect-square size-full', className)} + {...props} + /> +)) +AvatarImage.displayName = AvatarPrimitive.Image.displayName @@ -function AvatarFallback({ - className, - ...props -}: React.ComponentProps<typeof AvatarPrimitive.Fallback>) { - return ( - <AvatarPrimitive.Fallback - data-slot="avatar-fallback" - className={cn( - 'bg-muted flex size-full items-center justify-center rounded-full', - className, - )} - {...props} - /> - ) -} +const AvatarFallback = React.forwardRef< + React.ElementRef<typeof AvatarPrimitive.Fallback>, + React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> +>(({ className, ...props }, ref) => ( + <AvatarPrimitive.Fallback + ref={ref} + data-slot="avatar-fallback" + className={cn('bg-muted flex size-full items-center justify-center rounded-full', className)} + {...props} + /> +)) +AvatarFallback.displayName = AvatarPrimitive.Fallback.displayNameAlso applies to: 24-35, 37-51
README.md (1)
103-116: Add a language to the fenced code block (markdownlint MD040).Specify a language for the “Project Structure” code fence to satisfy linters and improve rendering.
-``` +```text saglac.io/ ├── app/ # Next.js App Router pages ├── components/ # React components │ ├── ui/ # shadcn/ui components │ └── __tests__/ # Component tests ├── data/ # YAML content files │ ├── io-events/ # Event definitions │ ├── authors/ # Speaker profiles │ └── locations/ # Venue information ├── lib/ # Utilities and types ├── scripts/ # Build scripts (RSS generation) └── public/ # Static assetscomponents/__tests__/archives-list.test.tsx (2)
120-137: Prefer role-based, less brittle queries for pagination.Using getAllByText('Précédent'/'Suivant') and getAllByText('1'/'2') can match unintended nodes. Target controls by role to improve stability.
Examples:
- Replace:
- expect(screen.getAllByText('Précédent')).toHaveLength(2)
- expect(screen.getAllByText('Suivant')).toHaveLength(2)
- expect(screen.getAllByText('1')).toHaveLength(2)
- expect(screen.getAllByText('2')).toHaveLength(2)
- With:
- expect(screen.getAllByRole('button', { name: 'Précédent' })).toHaveLength(2)
- expect(screen.getAllByRole('button', { name: 'Suivant' })).toHaveLength(2)
- expect(screen.getAllByRole('button', { name: '1' })).toHaveLength(2)
- expect(screen.getAllByRole('button', { name: '2' })).toHaveLength(2)
286-290: Make date assertion resilient to locale/formatting.Hard-coding “15 janvier 2024” can flake if locale/formatting changes. Prefer a regex or an aria-label tied to the component, e.g., getByTestId or role+name.
components/ui/input-group.tsx (2)
65-69: Consider removing nested role="group" on addon.Nested ARIA groupings can confuse assistive tech. The root already exposes role="group"; the addon can be a plain div/span.
10-35: Forward refs on group and controls for better integration.Forwarding refs (InputGroup, Addon, Text/Textarea wrappers, Button passthrough) improves focus management and interop with form libs.
Also applies to: 59-79, 99-116, 118-128, 130-144, 146-160
components/ui/field.tsx (1)
207-214: Consider filtering errors before mapping.The current implementation uses inline conditionals within the map function. Filtering out invalid errors first would make the code cleaner and more explicit.
Apply this diff to filter before mapping:
return ( <ul className="ml-4 flex list-disc flex-col gap-1"> - {errors.map( - (error, index) => - error?.message && <li key={index}>{error.message}</li>, - )} + {errors + .filter((error) => error?.message) + .map((error, index) => ( + <li key={index}>{error.message}</li> + ))} </ul> )
components.json
Outdated
| "rsc": true, | ||
| "tsx": true, | ||
| "tailwind": { | ||
| "config": "", |
There was a problem hiding this comment.
❓ Verification inconclusive
Verify empty Tailwind config path.
The tailwind.config property is set to an empty string (line 7), while other paths like css (line 8) are explicitly defined. Confirm this is intentional if your Tailwind config is at the default location (tailwind.config.js or tailwind.config.ts at the project root).
Run the following script to check for Tailwind config files:
🏁 Script executed:
#!/bin/bash
# Description: Find Tailwind configuration files
fd -g "tailwind.config.*" --max-depth 1Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for Tailwind config files in entire repository
fd -g "tailwind.config.js" -g "tailwind.config.ts"Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search for Tailwind configuration files across the repository
find . -type f -name "tailwind.config.*"Length of output: 0
Add Tailwind config file or set its path in components.json
The config field (components.json:7) is empty but no tailwind.config.* file exists—add a config file at the project root or update "config" with its correct path.
🤖 Prompt for AI Agents
In components.json around line 7, the "config" field is empty but there is no
tailwind.config.* in the repo; either add a Tailwind config file at the project
root (e.g., create tailwind.config.js or tailwind.config.cjs with your
theme/plugins) or update the "config" value to the correct relative path to an
existing Tailwind config file; ensure the path is correct and committed so
tooling can locate the config.
| export function FAQSection() { | ||
| const sections = loadFAQ() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify that synchronous data loading is appropriate for this component.
loadFAQ() is called directly in the component body without use or async/await. In Next.js 15's App Router:
- If this is a Server Component, synchronous data loading is fine
- If this is a Client Component (indicated by state/effects or 'use client'), data should be passed as props or fetched with proper React hooks
Run the following script to check if this component or any parent has 'use client':
🏁 Script executed:
#!/bin/bash
# Check if FAQSection or its imports are client components
rg -n "use client" components/faq-section.tsx
# Check where FAQSection is imported and used
rg -n "FAQSection" app/ --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check for 'use client' directive in the component
rg -n "use client" components/faq-section.tsx
# Check for 'use client' in any parent layouts or pages
rg -n "use client" -g "app/**/*.tsx"
# Find all usages of FAQSection in the app directory
rg -n "FAQSection" -g "app/**/*.tsx"Length of output: 105
🏁 Script executed:
#!/bin/bash
# Locate all occurrences of loadFAQ and show surrounding context
rg -n "loadFAQ" -g "**/*.{ts,tsx}" -C3Length of output: 3091
🏁 Script executed:
#!/bin/bash
# Search for 'use client' directives across all TSX files
rg -n "use client" -g "**/*.tsx"Length of output: 1842
Split data loading and UI rendering in FAQSection
components/faq-section.tsx currently imports client-only Accordion components without a use client directive and calls loadFAQ() (fs-based) synchronously. Move loadFAQ() into a server component (e.g., in app/page.tsx), pass the FAQ data as props, and add 'use client' to a child component that handles rendering with Accordion.
| function ContextMenuSubTrigger({ | ||
| className, | ||
| inset, | ||
| children, | ||
| ...props | ||
| }: React.ComponentProps<typeof ContextMenuPrimitive.SubTrigger> & { | ||
| inset?: boolean | ||
| }) { | ||
| return ( | ||
| <ContextMenuPrimitive.SubTrigger | ||
| data-slot="context-menu-sub-trigger" | ||
| data-inset={inset} | ||
| className={cn( | ||
| "focus:bg-accent focus:text-accent-foreground data-[state=open]:bg-accent data-[state=open]:text-accent-foreground [&_svg:not([class*='text-'])]:text-muted-foreground flex cursor-default items-center rounded-sm px-2 py-1.5 text-sm outline-hidden select-none data-[inset]:pl-8 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <ChevronRightIcon className="ml-auto" /> | ||
| </ContextMenuPrimitive.SubTrigger> | ||
| ) | ||
| } | ||
|
|
||
| function ContextMenuSubContent({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof ContextMenuPrimitive.SubContent>) { | ||
| return ( | ||
| <ContextMenuPrimitive.SubContent | ||
| data-slot="context-menu-sub-content" | ||
| className={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-(--radix-context-menu-content-transform-origin) overflow-hidden rounded-md border p-1 shadow-lg', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| function ContextMenuContent({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof ContextMenuPrimitive.Content>) { | ||
| return ( | ||
| <ContextMenuPrimitive.Portal> | ||
| <ContextMenuPrimitive.Content | ||
| data-slot="context-menu-content" | ||
| className={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 max-h-(--radix-context-menu-content-available-height) min-w-[8rem] origin-(--radix-context-menu-content-transform-origin) overflow-x-hidden overflow-y-auto rounded-md border p-1 shadow-md', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| </ContextMenuPrimitive.Portal> | ||
| ) | ||
| } | ||
|
|
||
| function ContextMenuItem({ | ||
| className, | ||
| inset, | ||
| variant = 'default', | ||
| ...props | ||
| }: React.ComponentProps<typeof ContextMenuPrimitive.Item> & { | ||
| inset?: boolean | ||
| variant?: 'default' | 'destructive' | ||
| }) { | ||
| return ( | ||
| <ContextMenuPrimitive.Item | ||
| data-slot="context-menu-item" | ||
| data-inset={inset} | ||
| data-variant={variant} | ||
| className={cn( | ||
| "focus:bg-accent focus:text-accent-foreground data-[variant=destructive]:text-destructive data-[variant=destructive]:focus:bg-destructive/10 dark:data-[variant=destructive]:focus:bg-destructive/20 data-[variant=destructive]:focus:text-destructive data-[variant=destructive]:*:[svg]:!text-destructive [&_svg:not([class*='text-'])]:text-muted-foreground relative flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-sm outline-hidden select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 data-[inset]:pl-8 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| function ContextMenuCheckboxItem({ | ||
| className, | ||
| children, | ||
| checked, | ||
| ...props | ||
| }: React.ComponentProps<typeof ContextMenuPrimitive.CheckboxItem>) { | ||
| return ( | ||
| <ContextMenuPrimitive.CheckboxItem | ||
| data-slot="context-menu-checkbox-item" | ||
| className={cn( | ||
| "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", | ||
| className, | ||
| )} | ||
| checked={checked} | ||
| {...props} | ||
| > | ||
| <span className="pointer-events-none absolute left-2 flex size-3.5 items-center justify-center"> | ||
| <ContextMenuPrimitive.ItemIndicator> | ||
| <CheckIcon className="size-4" /> | ||
| </ContextMenuPrimitive.ItemIndicator> | ||
| </span> | ||
| {children} | ||
| </ContextMenuPrimitive.CheckboxItem> | ||
| ) | ||
| } | ||
|
|
||
| function ContextMenuRadioItem({ | ||
| className, | ||
| children, | ||
| ...props | ||
| }: React.ComponentProps<typeof ContextMenuPrimitive.RadioItem>) { | ||
| return ( | ||
| <ContextMenuPrimitive.RadioItem | ||
| data-slot="context-menu-radio-item" | ||
| className={cn( | ||
| "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", | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| <span className="pointer-events-none absolute left-2 flex size-3.5 items-center justify-center"> | ||
| <ContextMenuPrimitive.ItemIndicator> | ||
| <CircleIcon className="size-2 fill-current" /> | ||
| </ContextMenuPrimitive.ItemIndicator> | ||
| </span> | ||
| {children} | ||
| </ContextMenuPrimitive.RadioItem> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Forward refs through the Radix wrappers.
Every wrapper (ContextMenuSubTrigger, ContextMenuContent, ContextMenuItem, etc.) is a plain function component, so any ref a consumer provides is dropped before it reaches the Radix primitive. That breaks focus management and imperative APIs Radix expects. Please convert these wrappers to React.forwardRef (matching the underlying element type and setting displayName) so refs propagate correctly. Based on learnings
🤖 Prompt for AI Agents
components/ui/context-menu.tsx around lines 56 to 185: the wrapper components
drop incoming refs because they are plain function components; convert each
wrapper (ContextMenuSubTrigger, ContextMenuSubContent, ContextMenuContent,
ContextMenuItem, ContextMenuCheckboxItem, ContextMenuRadioItem) to use
React.forwardRef with the correct element/primitive prop types, forward the ref
into the underlying ContextMenuPrimitive component (e.g.,
<ContextMenuPrimitive.Item ref={ref} ... />), preserve props spreading and
className logic, and set a displayName for each forwarded ref component (e.g.,
ContextMenuItem.displayName = 'ContextMenuItem').
| function FieldTitle({ className, ...props }: React.ComponentProps<'div'>) { | ||
| return ( | ||
| <div | ||
| data-slot="field-label" | ||
| className={cn( | ||
| 'flex w-fit items-center gap-2 text-sm leading-snug font-medium group-data-[disabled=true]/field:opacity-50', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify duplicate data-slot attribute.
Both FieldTitle (line 131) and FieldLabel (line 116) use data-slot="field-label". This could cause confusion when styling or selecting elements, as CSS selectors won't distinguish between them.
If this is intentional for shared styling purposes, consider documenting it. Otherwise, use a unique value like data-slot="field-title".
🏁 Script executed:
#!/bin/bash
# Description: Check if data-slot="field-label" selector is used elsewhere in styles or other components
# Search for CSS selectors targeting field-label
rg -n --type=ts --type=tsx --type=css -C3 'data-slot=field-label|field-label'Length of output: 27
🏁 Script executed:
#!/bin/bash
grep -R 'data-slot="field-label"' -n .Length of output: 119
Use a unique data-slot for FieldTitle
FieldLabel (line 116) and FieldTitle (line 131) both use data-slot="field-label", which will cause selector/styling conflicts. Change FieldTitle’s attribute to something like data-slot="field-title".
🤖 Prompt for AI Agents
In components/ui/field.tsx around lines 128 to 139, FieldTitle currently uses
data-slot="field-label" which conflicts with FieldLabel; change FieldTitle’s
attribute to data-slot="field-title" and update any related CSS/JS selectors
that target the old data-slot to use data-slot="field-title" so styling and DOM
queries remain correct.
| const ToggleGroupContext = React.createContext< | ||
| VariantProps<typeof toggleVariants> | ||
| >({ | ||
| size: 'default', | ||
| variant: 'default', | ||
| }) |
There was a problem hiding this comment.
Context initialization prevents item-level fallback.
The context is initialized with default values ({ size: 'default', variant: 'default' }), which means the fallback logic in ToggleGroupItem (lines 56-57, 60-61) using context.variant || variant will never use the item's own variant or size props when the group doesn't explicitly pass them. The context will always have 'default' values, not undefined.
Consider either:
- Initialize the context with
undefinedvalues to enable proper fallback:{ size: undefined, variant: undefined } - Remove the item-level
variantandsizeprops if they're not intended to be used
Apply this diff if fallback to item props is desired:
const ToggleGroupContext = React.createContext<
VariantProps<typeof toggleVariants>
>({
- size: 'default',
- variant: 'default',
+ size: undefined,
+ variant: undefined,
})🤖 Prompt for AI Agents
In components/ui/toggle-group.tsx around lines 10 to 15, the context is
initialized with concrete defaults which prevents ToggleGroupItem from falling
back to its own props; change the context initial value to have undefined fields
(e.g. { size: undefined, variant: undefined }) so item-level fallback works, and
adjust the context type to allow undefined (use Partial<VariantProps<typeof
toggleVariants>> or make the properties optional) so the compiler accepts
undefined values.
| 'bg-foreground text-background 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 origin-(--radix-tooltip-content-transform-origin) rounded-md px-3 py-1.5 text-xs text-balance', | ||
| className, | ||
| )} |
There was a problem hiding this comment.
Fix malformed Tailwind transform-origin token
Line 49 uses origin-(--radix-tooltip-content-transform-origin), which Tailwind does not parse. The class is dropped, so the tooltip animation falls back to the default transform origin. Replace it with the bracketed form Tailwind expects so the animation anchors correctly.
- 'bg-foreground text-background 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 origin-(--radix-tooltip-content-transform-origin) rounded-md px-3 py-1.5 text-xs text-balance',
+ 'bg-foreground text-background 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 origin-[var(--radix-tooltip-content-transform-origin)] rounded-md px-3 py-1.5 text-xs text-balance',📝 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.
| 'bg-foreground text-background 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 origin-(--radix-tooltip-content-transform-origin) rounded-md px-3 py-1.5 text-xs text-balance', | |
| className, | |
| )} | |
| 'bg-foreground text-background 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 origin-[var(--radix-tooltip-content-transform-origin)] rounded-md px-3 py-1.5 text-xs text-balance', | |
| className, | |
| )} |
🤖 Prompt for AI Agents
In components/ui/tooltip.tsx around lines 49 to 51, the Tailwind token
origin-(--radix-tooltip-content-transform-origin) is malformed and gets dropped;
replace it with the bracketed arbitrary value syntax
origin-[var(--radix-tooltip-content-transform-origin)] so Tailwind parses the
transform-origin correctly, keeping the rest of the class string and className
merging intact.
| React.useEffect(() => { | ||
| listeners.push(setState) | ||
| return () => { | ||
| const index = listeners.indexOf(setState) | ||
| if (index > -1) { | ||
| listeners.splice(index, 1) | ||
| } | ||
| } | ||
| }, [state]) |
There was a problem hiding this comment.
Fix infinite listener accumulation.
The useEffect dependency array includes [state], causing the effect to re-run on every state update. This continuously adds duplicate listeners to the module-level listeners array without removing the old ones in time, leading to memory leaks and redundant state updates.
Since setState from useState has a stable reference, the dependency array should be empty.
Apply this diff:
React.useEffect(() => {
listeners.push(setState)
return () => {
const index = listeners.indexOf(setState)
if (index > -1) {
listeners.splice(index, 1)
}
}
- }, [state])
+ }, [])📝 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.
| React.useEffect(() => { | |
| listeners.push(setState) | |
| return () => { | |
| const index = listeners.indexOf(setState) | |
| if (index > -1) { | |
| listeners.splice(index, 1) | |
| } | |
| } | |
| }, [state]) | |
| React.useEffect(() => { | |
| listeners.push(setState) | |
| return () => { | |
| const index = listeners.indexOf(setState) | |
| if (index > -1) { | |
| listeners.splice(index, 1) | |
| } | |
| } | |
| }, []) |
🤖 Prompt for AI Agents
In components/ui/use-toast.ts around lines 174 to 182, the useEffect currently
depends on [state] causing it to run on every state update and repeatedly push
duplicate setState listeners; change the effect to have an empty dependency
array ([]) so it only registers the stable setState once on mount and removes it
on unmount, leaving the cleanup logic as-is to splice out the listener; ensure
you do not reference state in the dependency array and keep setState (from
useState) as the only value used inside the effect.
| - **[RSS & Sitemap Generation](./docs/RSS_AND_SITEMAP.md)** - Feed generation details | ||
| - **[Technical Analysis](./ANALYSIS_SUMMARY.md)** - Architecture decisions | ||
|
|
||
| ## Community | ||
|
|
||
| - **Website**: [saglac.io](https://saglac.io) | ||
| - **Facebook**: [SagLac IO Group](https://www.facebook.com/groups/saglac.io) | ||
| - **Twitter**: [@saglacio](https://twitter.com/saglacio) | ||
| - **LinkedIn**: [SagLac IO Company](https://www.linkedin.com/company/saglac-io) | ||
| - **Discord**: [Join our server](https://discord.gg/8pY5XVhvYM) | ||
|
|
||
| ## License | ||
|
|
||
| This project is licensed under the MIT License - see the [LICENSE](LICENSE) file for details. | ||
|
|
||
| ## Acknowledgments | ||
|
|
||
| - Built with ❤️ by the SagLac IO community | ||
| - Powered by [Next.js](https://nextjs.org) and [Vercel](https://vercel.com) | ||
| - UI components from [shadcn/ui](https://ui.shadcn.com) | ||
| - Hosted on [GitHub Pages](https://pages.github.com) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify docs paths and clarify hosting note (Vercel vs GitHub Pages).
- Ensure docs links resolve.
- “Powered by Vercel” + “Hosted on GitHub Pages” may confuse readers. If hosting is GH Pages, consider “Built with Next.js by Vercel” to avoid implying deployment on Vercel.
Run to verify files and print their status:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf "Checking docs exist...\n"
for f in "docs/RSS_AND_SITEMAP.md" "ANALYSIS_SUMMARY.md" "LICENSE"; do
if [ -f "$f" ]; then
echo "OK - $f"
else
echo "MISS- $f"
fi
done
echo
echo "Grepping hosting statement for consistency:"
rg -n -C2 -e 'Vercel|GitHub Pages' README.md || trueLength of output: 572
Fix broken Technical Analysis link and clarify hosting note
- The
./ANALYSIS_SUMMARY.mdfile is missing; add it or correct the link in README.md. - Rephrase “Powered by Vercel” versus “Hosted on GitHub Pages” to avoid implying deployment on Vercel (e.g. “Built with Next.js and deployed on GitHub Pages”).
🧰 Tools
🪛 LanguageTool
[grammar] ~181-~181: There might be a mistake here.
Context: ..._SITEMAP.md)** - Feed generation details - **[Technical Analysis](./ANALYSIS_SUMMARY.m...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ... ## Community - Website: saglac.io - Facebook: [SagLac IO Group](https://ww...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ...lac.io) - Facebook: SagLac IO Group - Twitter: [@saglacio](https://twitter.c...
(QB_NEW_EN)
[grammar] ~188-~188: There might be a mistake here.
Context: ...ups/saglac.io) - Twitter: @saglacio - LinkedIn: [SagLac IO Company](https://...
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ...acio) - LinkedIn: SagLac IO Company - Discord: [Join our server](https://dis...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around lines 181 to 201, the Technical Analysis link points to a
missing ./ANALYSIS_SUMMARY.md and the wording implies Vercel hosts the site
while GitHub Pages is listed as host; update the link to an existing file
(rename to ./ANALYSIS_SUMMARY.md or change link to the correct path or remove
the entry if the file will not be added) and rephrase the deployment/hosting
lines to avoid conflicting claims (e.g., “Built with Next.js and deployed on
GitHub Pages” and remove or clarify the “Powered by Vercel” line if Vercel is
not the deployment target).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.gitignore (1)
26-27: Keepnext-env.d.tstracked.Next.js needs this file committed so TypeScript picks up the framework types on clean checkouts; ignoring it forces every fresh environment to regenerate it manually and breaks CI type-checking until that happens. Please drop this ignore entry and track the file.
- next-env.d.tsapp/globals.css (1)
1-1: PostCSS configuration concern resolved.The past review raised a concern about Next.js v15.5.5 requiring a root-level PostCSS config to process the
@import "tailwindcss"directive. This has been addressed by addingpostcss.config.mjsat the project root.
🧹 Nitpick comments (6)
playwright.config.ts (2)
42-42: Consider adjusting trace collection for better debugging.The current
'on-first-retry'setting means traces are only collected after the first failure. For critical E2E flows, consider using'retain-on-failure'to capture traces on any failure, not just retries. This provides better debugging information when tests fail, especially on CI where retries are enabled.Apply this diff:
- trace: 'on-first-retry', + trace: 'retain-on-failure',Alternatively, use
'on'to always collect traces if CI performance is not a concern.
28-31: Consider adding html reporter configuration options.Based on Playwright best practices, you can configure additional html reporter options for better CI integration and test report usability. For example, disabling the "Copy prompt" button in CI artifacts or using
mergeFilesfor more compact test listings.Consider this enhancement:
reporter: [ - ['html', { open: 'never' }], + ['html', { + open: 'never', + ...(process.env.CI ? { copyPrompt: false } : {}) + }], ['list'], ],e2e/critical-flows.spec.ts (4)
55-55: Consider replacingnetworkidlewith more specific wait.The
waitForLoadState('networkidle')can be slow and flaky as it waits for all network activity to cease. Consider waiting for specific elements or usingwaitForLoadState('domcontentloaded')instead, which is more reliable and faster.Apply this diff:
- await page.waitForLoadState('networkidle') + await page.waitForLoadState('domcontentloaded')Or wait for the search box to be ready:
- await page.waitForLoadState('networkidle') + const searchBox = page.getByPlaceholder(/Rechercher/i) + await searchBox.waitFor({ state: 'visible' })
62-62: Replace fixed timeout with more robust waiting strategy.The fixed
waitForTimeout(500)for debounce is fragile and can lead to flaky tests. Consider waiting for a network request to complete or for content to update instead.Based on Playwright best practices, consider waiting for a network response:
await searchBox.fill('homelab') - // Wait a bit for debounce - await page.waitForTimeout(500) + // Wait for search to complete + await page.waitForLoadState('networkidle')Or if you know the search triggers a specific network call, wait for that response explicitly.
90-100: Consider verifying Facebook link location in header.The test name suggests checking the header, but the current implementation only verifies a Facebook link exists anywhere on the page. The comment on line 93 acknowledges this. Consider making the assertion more specific to match the test name.
Apply this diff to verify the link is in the header:
- // Find any "Rejoindre" link on the page (header, hero, or elsewhere) - // Use a more specific selector or just check that a Facebook link exists - const facebookLinks = page.locator('a[href*="facebook.com/groups/saglac.io"]') + // Verify Facebook group link exists in header + const facebookLinks = page.locator('header a[href*="facebook.com/groups/saglac.io"]') - // Verify at least one Facebook group link exists (may not be visible in mobile menu) + // Verify at least one Facebook group link exists in header const count = await facebookLinks.count() expect(count).toBeGreaterThan(0)Alternatively, rename the test to reflect that it checks for Facebook link existence anywhere on the page.
113-113: Handle potential promise rejection more robustly.The
.catch(() => false)on line 113 swallows all errors, not just "not visible" errors. This could hide unexpected failures. Consider using a try-catch block for clearer error handling.Apply this diff:
- const isVisible = await menuButton.isVisible().catch(() => false) + let isVisible = false + try { + isVisible = await menuButton.isVisible() + } catch { + isVisible = false + }Or use Playwright's built-in timeout handling:
- const isVisible = await menuButton.isVisible().catch(() => false) + const isVisible = await menuButton.isVisible({ timeout: 1000 }).catch(() => false)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)app/globals.css(1 hunks)e2e/critical-flows.spec.ts(1 hunks)package.json(1 hunks)playwright.config.ts(1 hunks)postcss.config.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (3)
playwright.config.ts (1)
12-71: LGTM! Well-structured Playwright configuration.The configuration is well-organized with appropriate CI settings, reasonable timeouts, and good debugging defaults. The web server integration with the static build is correctly configured, and the focus on Chromium projects (desktop and mobile) provides good coverage while keeping test execution fast.
e2e/critical-flows.spec.ts (1)
14-125: Well-structured E2E test suite with good coverage.The test suite effectively covers critical user flows with clear assertions and good defensive coding for mobile scenarios. The tests are well-organized, use appropriate selectors, and include helpful comments. The mobile test gracefully handles both mobile menu and desktop navigation scenarios.
postcss.config.mjs (1)
1-5: PostCSS config is correct—no separate Autoprefixer needed
Tailwind CSS v4’s PostCSS plugin (LightningCSS-powered) handles vendor prefixing, so you can omit Autoprefixer.
| test('User can search for events in archives', async ({ page }) => { | ||
| await page.goto('/archives') | ||
|
|
||
| // Wait for page to be ready | ||
| await page.waitForLoadState('networkidle') | ||
|
|
||
| // Type in search box | ||
| const searchBox = page.getByPlaceholder(/Rechercher/i) | ||
| await searchBox.fill('homelab') | ||
|
|
||
| // Wait a bit for debounce | ||
| await page.waitForTimeout(500) | ||
|
|
||
| // Verify search results update (check if page content changes) | ||
| const pageContent = await page.textContent('body') | ||
| expect(pageContent).toBeTruthy() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Strengthen search result verification.
The current test only verifies that page content exists (expect(pageContent).toBeTruthy()), which would pass even if the search didn't work. Consider verifying that the search actually filters content by checking for the presence of search terms or the absence of unrelated content.
Consider this enhancement:
await page.waitForTimeout(500)
- // Verify search results update (check if page content changes)
- const pageContent = await page.textContent('body')
- expect(pageContent).toBeTruthy()
+ // Verify search results are filtered
+ const pageContent = await page.textContent('main')
+ expect(pageContent?.toLowerCase()).toContain('homelab')Alternatively, check that specific event cards are visible or that the results count has changed.
📝 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.
| test('User can search for events in archives', async ({ page }) => { | |
| await page.goto('/archives') | |
| // Wait for page to be ready | |
| await page.waitForLoadState('networkidle') | |
| // Type in search box | |
| const searchBox = page.getByPlaceholder(/Rechercher/i) | |
| await searchBox.fill('homelab') | |
| // Wait a bit for debounce | |
| await page.waitForTimeout(500) | |
| // Verify search results update (check if page content changes) | |
| const pageContent = await page.textContent('body') | |
| expect(pageContent).toBeTruthy() | |
| }) | |
| test('User can search for events in archives', async ({ page }) => { | |
| await page.goto('/archives') | |
| // Wait for page to be ready | |
| await page.waitForLoadState('networkidle') | |
| // Type in search box | |
| const searchBox = page.getByPlaceholder(/Rechercher/i) | |
| await searchBox.fill('homelab') | |
| // Wait a bit for debounce | |
| await page.waitForTimeout(500) | |
| // Verify search results are filtered | |
| const pageContent = await page.textContent('main') | |
| expect(pageContent?.toLowerCase()).toContain('homelab') | |
| }) |
🤖 Prompt for AI Agents
In e2e/critical-flows.spec.ts around lines 51–67, the test only asserts that the
page body is truthy after typing the search term, which does not verify that
filtering occurred; replace the generic check with a deterministic assertion:
before typing capture the initial results locator (e.g., list of event cards or
a results-count element) and its count/text, fill the search box with 'homelab',
wait for the results locator to update (use locator.waitFor or
waitForResponse/networkidle), then assert that the results now contain the
search term (e.g., expect(locator).toContainText('homelab') or
expect(resultsLocator).toHaveCount(<expected or < initial count)) or assert that
a specific known event card appears/disappears to prove filtering worked.
- Add serve@14.2.5 as devDependency to avoid npx installation delays - Update playwright.config.ts to use 'pnpm exec serve' instead of 'npx serve' - Fixes CI timeout error where npx was installing serve during test run - E2E tests verified working locally (14/14 passing)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
50-58: Pin thelatestdependencies.
@vercel/analyticsandgeistare still set to"latest", so installs remain non-deterministic and can break unexpectedly. Please lock each to the exact version you’ve validated (e.g., runnpm info <pkg> versionto pick a semver tag) and update the lockfile.
🧹 Nitpick comments (2)
playwright.config.ts (2)
24-25: Consider documenting the rationale for single-worker CI execution.Setting
workers: 1on CI ensures stability but can significantly increase test execution time. If this is intentional (e.g., to avoid resource contention or flaky tests), consider adding a brief comment explaining the trade-off.Apply this diff to add clarification:
- /* Opt out of parallel tests on CI */ + /* Opt out of parallel tests on CI for stability (prevents resource contention) */ workers: process.env.CI ? 1 : undefined,
28-31: Consider the newnoSnippetsreporter option.Playwright 1.56.0 introduced a
noSnippetsoption for the HTML reporter to reduce report size by excluding code snippets. If your CI generates large HTML reports, this could improve performance.Based on learnings
Example usage:
reporter: [ ['html', { open: 'never', noSnippets: true }], ['list'], ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)playwright.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - keen-fairy-bc0a2b
- GitHub Check: Header rules - keen-fairy-bc0a2b
- GitHub Check: Pages changed - keen-fairy-bc0a2b
- GitHub Check: E2E Tests
- GitHub Check: E2E Tests
🔇 Additional comments (1)
playwright.config.ts (1)
1-72: LGTM! Well-structured Playwright configuration.The configuration follows best practices:
- Appropriate CI-specific settings (forbidOnly, retries, conditional worker config)
- Efficient artifact collection (trace on first retry, screenshots/videos on failure)
- Focused test matrix (Chromium-based browsers only for speed)
- Proper webServer setup with conditional reuse
The configuration aligns well with the PR's goal of adding modern E2E testing infrastructure to the Next.js rewrite.
| webServer: { | ||
| command: 'pnpm exec serve out -l 3000', | ||
| url: 'http://localhost:3000', | ||
| reuseExistingServer: !process.env.CI, | ||
| timeout: 120 * 1000, | ||
| stdout: 'ignore', | ||
| stderr: 'pipe', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the webServer configuration dependencies.
Ensure that:
- The
servepackage is installed (checkpackage.jsondependencies or devDependencies). - The build output directory is
out(Next.js exports tooutby default withoutput: 'export'innext.config.ts). - Port 3000 is not used by other services during test execution.
Run the following script to verify the webServer setup:
🏁 Script executed:
#!/bin/bash
# Verify 'serve' package and 'out' directory
# Check if 'serve' is in package.json
echo "=== Checking for 'serve' package ==="
if jq -e '.dependencies.serve // .devDependencies.serve' package.json > /dev/null 2>&1; then
echo "✓ 'serve' found in dependencies"
else
echo "✗ 'serve' NOT found in dependencies"
fi
# Check next.config for output configuration
echo -e "\n=== Checking Next.js output configuration ==="
if [ -f "next.config.ts" ]; then
rg -n "output.*export" next.config.ts || echo "Note: 'output: export' not found in next.config.ts"
else
echo "Note: next.config.ts not found"
fi
# Check if build script generates 'out' directory
echo -e "\n=== Checking build scripts ==="
jq -r '.scripts.build // "not found"' package.jsonLength of output: 189
Add static export to build step and confirm port availability
- The build script runs only
next build, which doesn’t output anoutdirectory. Update it to e.g."build": "next build && next export"or addoutput: 'export'innext.config.tssooutis generated. - Ensure no other process binds to port 3000 during test runs.
🤖 Prompt for AI Agents
In playwright.config.ts around lines 64 to 71, the webServer expects an out/
static export on port 3000 but the project currently only runs next build;
update the build process to produce the out directory by either changing
package.json "build" to run "next build && next export" or setting output:
'export' in next.config.ts so next export generates out, and ensure test runs
don't have another process bound to port 3000 by reserving or checking the port
in CI (e.g., choose an alternate port, kill/cleanup any existing process before
starting, or add a pre-test check that the port is free).
- Add 'Build static site for E2E tests' step to e2e job - E2E tests require the out/ directory to be built first - Fixes webServer timeout error where serve couldn't find out/ directory
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
57-65: Guard artifact uploads against missing outputsWith
if: always()this step executes even whenpnpm test:coverageaborts early. If the run crashes before producing XML reports (or the coverage folder),actions/upload-artifact@v4will fail the step because it can’t find the declared paths, making it harder to see the real failure. Addif-no-files-found: ignore(and optionally guard each path) so the job reports the original error cleanly.
98-112: Avoid hard failures when Playwright artifacts are absentWhen
pnpm test:e2efails before generatingplaywright-report/ortest-results/, this upload step (also forced to run viaif: always()) fails with “no files found,” which obscures the underlying test failure. Addif-no-files-found: ignore(or conditionally check for the directories) so the workflow surfaces the real problem while still capturing artifacts when they’re available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E Tests
- GitHub Check: E2E Tests
GabLeRoux
changed the title
- docs: update playwright-report retention from 7 to 30 days in workflows README - docs: update actions/checkout version from v4 to v5 in workflows README - fix: add postcss.config.mjs path to components.json tailwind config - refactor: split FAQSection into server (data) and client (UI) components - fix: validate location URLs before rendering link in next-event - docs: remove non-existent ANALYSIS_SUMMARY.md reference from README - test: improve search filtering test with conditional assertions - fix: replace 'latest' with explicit versions for @vercel/analytics (^1.5.0) and geist (^1.5.1) All changes verified with type checking, linting, and E2E tests passing (14/14)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/faq-section-client.tsx (1)
48-48: Remove the unusednodeparameter.The
nodeparameter is destructured but never used in the anchor component override.Apply this diff to clean up the unused parameter:
- a: ({ node, ...props }) => ( + a: (props) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/workflows/README.md(1 hunks)README.md(1 hunks)components.json(1 hunks)components/faq-section-client.tsx(1 hunks)components/faq-section.tsx(1 hunks)components/next-event.tsx(1 hunks)e2e/critical-flows.spec.ts(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/faq-section.tsx
- components.json
🧰 Additional context used
🧬 Code graph analysis (1)
components/next-event.tsx (1)
lib/types.ts (1)
Author(2-10)
🪛 LanguageTool
README.md
[grammar] ~18-~18: There might be a mistake here.
Context: ...tjs.org) with App Router (static export) - Language: TypeScript - Styling: [T...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...tatic export) - Language: TypeScript - Styling: [Tailwind CSS 4](https://tail...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...ypeScript - Styling: Tailwind CSS 4 - Components: [shadcn/ui](https://ui.sha...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...(https://ui.shadcn.com) (New York style) - Testing: Vitest ...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...itest](https://vitest.dev) + Playwright - Package Manager: pnpm - Deployment...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...ywright.dev) - Package Manager: pnpm - Deployment: GitHub Pages ## Features ...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...ta with location and speaker information - 👤 Speaker Profiles - Author profile...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...les with social links and GitHub avatars - 🔍 Event Search - Filter and search ...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... - Filter and search through past events - 📡 RSS Feed - Stay updated with late...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...events - 📡 RSS Feed - Stay updated with latest events - 🗺️ Sitemap - SEO-o...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...Feed** - Stay updated with latest events - 🗺️ Sitemap - SEO-optimized with aut...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ... SEO-optimized with automatic generation - 🌙 Dark Mode - System-aware theme sw...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...tomatic generation - 🌙 Dark Mode - System-aware theme switching - 🇫🇷 **French Co...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...rk Mode** - System-aware theme switching - 🇫🇷 French Content - Locale: `fr_CA...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...t** - Locale: fr_CA - ♿ Accessible - WCAG compliant components ## Getting ...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ... - Locale: fr_CA - ♿ Accessible - WCAG compliant components ## Getting Starte...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...Accessible** - WCAG compliant components ## Getting Started ### Prerequisites - No...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...tarted ### Prerequisites - Node.js 20+ - pnpm 9+ ### Installation ```bash # Clo...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...ight show-report ``` Test Coverage: - ✅ unit tests - Components and busine...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ... tests** - Components and business logic - ✅ E2E tests - Critical user flows - ...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ... - ✅ E2E tests - Critical user flows - ✅ Automatic screenshots on E2E failu...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...Automatic screenshots on E2E failure - ✅ Visual debugging with Playwright t...
(QB_NEW_EN)
[grammar] ~125-~125: There might be a mistake here.
Context: ...itory** 2. Create a feature branch: git checkout -b feature/amazing-feature 3. Make your changes and add tests if app...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...d tests if applicable 4. Run tests: pnpm test:all 5. Commit your changes: `git commit -m 'A...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ...m test:all5. **Commit your changes**:git commit -m 'Add amazing feature'6. **Push to your fork**:git push origin fe...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...ing feature'6. **Push to your fork**:git push origin feature/amazing-feature` 7. Open a Pull Request ### Adding an Eve...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ...com/presentation" ``` 3. Test locally: pnpm dev 4. Submit a pull request ### Adding a Spea...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ...e - TypeScript: Strict mode enabled - ESLint: Run pnpm lint before committ...
(QB_NEW_EN)
[grammar] ~176-~176: There might be a mistake here.
Context: ...*: Functional components with TypeScript - Tests: Write tests for new features #...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ... ## Community - Website: saglac.io - Facebook: [SagLac IO Group](https://ww...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ...lac.io) - Facebook: SagLac IO Group - Twitter: [@saglacio](https://twitter.c...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ...ups/saglac.io) - Twitter: @saglacio - LinkedIn: [SagLac IO Company](https://...
(QB_NEW_EN)
[grammar] ~188-~188: There might be a mistake here.
Context: ...acio) - LinkedIn: SagLac IO Company - Discord: [Join our server](https://dis...
(QB_NEW_EN)
.github/workflows/README.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...uns on all branches (branches: ['**']) - Pull Request: Runs on PRs targeting an...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ... ## Jobs ### 1. 🧪 Unit Tests (test) Purpose: Run Vitest unit tests with co...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...nit tests with code coverage Steps: - Checkout code - Setup pnpm with caching ...
(QB_NEW_EN)
[grammar] ~26-~26: Ensure spelling is correct
Context: ... Checkout code - Setup pnpm with caching - Setup Node.js 20 with pnpm cache - Install de...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ching - Setup Node.js 20 with pnpm cache - Install dependencies (frozen lockfile) -...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...- Install dependencies (frozen lockfile) - Run tests with coverage (`pnpm test:cove...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...sts with coverage (pnpm test:coverage) - Upload coverage to Codecov - Upload cove...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...:coverage`) - Upload coverage to Codecov - Upload coverage artifacts (HTML report) ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... Upload coverage artifacts (HTML report) - Upload test results Artifacts: - `c...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...t) - Upload test results Artifacts: - coverage-report: Full HTML coverage report (30 days ret...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...HTML coverage report (30 days retention) - test-results: Test results and coverage data (7 days...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...ays retention) Coverage Thresholds: - Statements: 40% - Branches: 70% - Functi...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...Coverage Thresholds**: - Statements: 40% - Branches: 70% - Functions: 60% - Lines: ...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...lds**: - Statements: 40% - Branches: 70% - Functions: 60% - Lines: 40% ### 2. 🎭 E...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...ts: 40% - Branches: 70% - Functions: 60% - Lines: 40% ### 2. 🎭 E2E Tests (e2e) ...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ... Lines: 40% ### 2. 🎭 E2E Tests (e2e) Purpose: Run Playwright end-to-end tes...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ... Playwright end-to-end tests Steps: - Checkout code - Setup pnpm with caching ...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...d Playwright HTML report Artifacts: - playwright-report: Test report with screenshots/videos (3...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ...os (30 days retention) Tests Cover: - Homepage functionality - Navigation acro...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...Tests Cover*: - Homepage functionality - Navigation across all pages - Archives p...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...ctionality - Navigation across all pages - Archives page with search - FAQ accordio...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...ss all pages - Archives page with search - FAQ accordion - Contact page - Accessibi...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...rchives page with search - FAQ accordion - Contact page - Accessibility checks - Re...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...th search - FAQ accordion - Contact page - Accessibility checks - Responsive design...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...on - Contact page - Accessibility checks - Responsive design ### 3. 🏗️ Build (`bu...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...nsive design ### 3. 🏗️ Build (build) Purpose: Build static Next.js site for...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...and e2e jobs to pass first Steps: - Checkout code - Setup pnpm with caching ...
(QB_NEW_EN)
[grammar] ~77-~77: Ensure spelling is correct
Context: ... Checkout code - Setup pnpm with caching - Setup Node.js 20 with pnpm cache - Install de...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ching - Setup Node.js 20 with pnpm cache - Install dependencies (frozen lockfile) -...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...- Install dependencies (frozen lockfile) - Build site (pnpm build) - Upload build...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...en lockfile) - Build site (pnpm build) - Upload build artifacts - Verify build ou...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ... (pnpm build) - Upload build artifacts - Verify build output (index.html, sitemap...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...l, sitemap.xml, rss.xml) Artifacts: - build-output: Complete out/ directory (7 days rete...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...days retention) ### 4. 📝 Lint (lint) Purpose: Run ESLint to catch code qual...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...to catch code quality issues Steps: - Checkout code - Setup pnpm with caching ...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...t) ### 5. 🔍 Type Check (type-check`) Purpose: Run TypeScript compiler in ty...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... compiler in type-check mode Steps: - Checkout code - Setup pnpm with caching ...
(QB_NEW_EN)
[grammar] ~104-~104: Ensure spelling is correct
Context: ... Checkout code - Setup pnpm with caching - Setup Node.js 20 with pnpm cache - Install de...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~105-~105: There might be a mistake here.
Context: ...ching - Setup Node.js 20 with pnpm cache - Install dependencies (frozen lockfile) -...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...- Install dependencies (frozen lockfile) - Run TypeScript (tsc --noEmit) ### 6. ...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...r all other jobs complete Behavior: - Always runs (if: always()) - Fails if ...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...havior**: - Always runs (if: always()) - Fails if any required job failed - Provi...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...s()`) - Fails if any required job failed - Provides clear success/failure message ...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...uilt-in cache support ``` Benefits: - Faster dependency installation - Automat...
(QB_NEW_EN)
[grammar] ~143-~143: There might be a mistake here.
Context: ...fits**: - Faster dependency installation - Automatic cache key management - Cache h...
(QB_NEW_EN)
[grammar] ~144-~144: There might be a mistake here.
Context: ...llation - Automatic cache key management - Cache hits across branches ### Docker I...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ... # List all jobs act -l ``` Notes: - Use `--container-architecture linux/amd6...
(QB_NEW_EN)
[grammar] ~203-~203: There might be a mistake here.
Context: ...N **Note**: Codecov action won't fail if token is missing (fail_ci_if_error: fa...
(QB_NEW_EN)
[grammar] ~209-~209: There might be a mistake here.
Context: ...aded from the GitHub Actions UI: 1. Go to Actions tab 2. Click on a workflow run ...
(QB_NEW_EN)
[grammar] ~215-~215: There might be a mistake here.
Context: ...report** (30 days): HTML coverage report - test-results (7 days): Raw test result...
(QB_NEW_EN)
[grammar] ~216-~216: There might be a mistake here.
Context: ...est-results** (7 days): Raw test results - playwright-report (30 days): E2E test ...
(QB_NEW_EN)
[grammar] ~217-~217: There might be a mistake here.
Context: ...ight-report** (30 days): E2E test report - build-output (7 days): Static site bui...
(QB_NEW_EN)
[grammar] ~296-~296: There might be a mistake here.
Context: ...formance: - Unit Tests: ~30 seconds - E2E Tests: ~2 minutes (Chromium only) ...
(QB_NEW_EN)
[grammar] ~297-~297: There might be a mistake here.
Context: ...E2E Tests*: ~2 minutes (Chromium only) - Build: ~45 seconds - Lint: ~20 sec...
(QB_NEW_EN)
[grammar] ~298-~298: There might be a mistake here.
Context: ...(Chromium only) - Build: ~45 seconds - Lint: ~20 seconds - Type Check: ~2...
(QB_NEW_EN)
[grammar] ~299-~299: There might be a mistake here.
Context: ...d**: ~45 seconds - Lint: ~20 seconds - Type Check: ~25 seconds **Total paral...
(QB_NEW_EN)
[grammar] ~369-~369: There might be a mistake here.
Context: ...sources - GitHub Actions Documentation - [pnpm Action Setup](https://github.com/pn...
(QB_NEW_EN)
[grammar] ~370-~370: There might be a mistake here.
Context: ...hub.com/en/actions) - pnpm Action Setup - [Act Documentation](https://github.com/ne...
(QB_NEW_EN)
[grammar] ~371-~371: There might be a mistake here.
Context: .../pnpm/action-setup) - Act Documentation - [Codecov Documentation](https://docs.code...
(QB_NEW_EN)
[grammar] ~372-~372: There might be a mistake here.
Context: ...com/nektos/act) - Codecov Documentation - [Playwright CI Guide](https://playwright....
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
README.md
155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
198-198: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E Tests
- GitHub Check: E2E Tests
🔇 Additional comments (1)
components/faq-section-client.tsx (1)
48-55: Conditionally apply target/rel only for external FAQ links
Verify that FAQ content doesn’t include internal links (e.g./about). Otherwise update the custom<a>renderer to checkhref.startsWith('http')(or//) before addingtarget="_blank"andrel="noopener noreferrer".
- fix: add stable unique IDs to FAQSection type (generated from slugified headers) - fix: replace array index with section.id as React key in faq-section-client - fix: add aria-label to all social media icon links for screen reader accessibility - fix: use toLocaleString with explicit timeZone (America/Toronto) to prevent hydration issues - fix: rename 2019-12-17-mta-saglac-io-2019.yml to meta (typo correction) All changes verified with TypeScript, linting, and E2E tests (14/14 passing)
|
🚀 Summary
Complete rewrite of the website from Gatsby to Next.js for better performance, maintainability, and modern developer experience.
✨ Highlights
Summary by CodeRabbit
New Features
Tests
Documentation
Chores