-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: UI improvements, hydration fixes, and template generation #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
📝 WalkthroughWalkthroughThis pull request introduces hydration awareness to chat and settings pages, centralizes URL query state keys, refactors the conversation archive confirmation from a modal to inline UI, removes framer-motion animations in favor of CSS, and updates site template domain configuration handling. Additionally, it cleans up configuration files and updates infrastructure settings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/features/chat/components/ConversationSidebar.tsx (1)
396-431:⚠️ Potential issue | 🟠 MajorAvoid nesting the editable input inside a button.
The rename
<input>is rendered inside a<button>(and the button becomes disabled), which is invalid HTML and can prevent the input from receiving focus. Consider using a non-button wrapper or rendering a button only when not editing.✅ One way to avoid nested interactive content
- <button - type="button" - disabled={isEditing} - onClick={isEditing ? undefined : onClick} - className="w-full flex items-start justify-between gap-2 text-left disabled:cursor-default bg-transparent border-none p-0 cursor-pointer" - > + <div + role="button" + tabIndex={isEditing ? -1 : 0} + onClick={isEditing ? undefined : onClick} + onKeyDown={e => { + if (!isEditing && (e.key === "Enter" || e.key === " ")) { + e.preventDefault() + onClick() + } + }} + className="w-full flex items-start justify-between gap-2 text-left bg-transparent border-none p-0 cursor-pointer" + > <div className="flex-1 min-w-0"> {isEditing ? ( <input ref={inputRef} type="text" value={editValue} onChange={e => setEditValue(e.target.value)} onBlur={handleSaveEdit} onKeyDown={handleEditKeyDown} className={`w-full text-sm font-medium ${styles.textPrimary} bg-black/[0.02] dark:bg-white/[0.04] border-0 rounded-lg px-2 py-1 outline-none ring-1 ring-black/[0.12] dark:ring-white/[0.12] focus:ring-black/[0.2] dark:focus:ring-white/[0.2] transition-all duration-150`} /> ) : ( <div className={`text-sm font-medium ${styles.textPrimary} line-clamp-2 flex items-center gap-2`}> {isStreaming && <StreamingDot />} <span className="flex-1 min-w-0 truncate">{conversation.title}</span> </div> )} </div> - </button> + </div>
🤖 Fix all issues with AI agents
In `@apps/web/components/settings/SettingsPageClient.tsx`:
- Around line 112-139: Active tab from the URL can point to admin-only pages
even after tabs are filtered; derive an effective tab from the post-filtered
tabs and use it for rendering and navigation. After computing tabs (the filtered
array), compute effectiveTab by finding tabs.find(t => t.id === activeTab) and
falling back to tabs[0] or a safe default (e.g., "account"); use effectiveTab
(or effectiveTab.id) instead of activeTab when building currentTab and rendering
panels, and if effectiveTab differs from activeTab call
setActiveTab(effectiveTab.id) to update the query state and close the sidebar
via setSidebarOpen(false) as done in handleTabChange.
In `@apps/web/tsconfig.json`:
- Around line 44-46: Update the apps/web/tsconfig.json includes to remove
duplicate entries that simply repeat existing patterns with a "./" prefix and
fix the copy-paste typo: replace ".next-test/dev/dev/types/**/*.ts" with
".next-test/dev/types/**/*.ts" and remove the redundant "./.next/types/**/*.ts"
and "./.next/dev/types/**/*.ts" entries so only the canonical patterns (e.g.,
".next-test/types/**/*.ts", ".next-test/dev/types/**/*.ts",
".next/types/**/*.ts", ".next/dev/types/**/*.ts") remain.
In `@templates/site-template/package.json`:
- Around line 16-18: The package.json devDependencies lists "tldts": "^11.17.0"
which is an invalid/nonexistent version; update the tldts entry in package.json
under devDependencies to a valid published version (for example "^7.0.19" or
"7.*") so dependency resolution succeeds—edit the "tldts" value in the
devDependencies section to the chosen valid version string.
In `@templates/site-template/scripts/generate-config.js`:
- Around line 36-38: The registrableDomain calculation uses parse(domain).domain
without allowing private suffixes, so private domains like github.io are
misparsed; update the call to parse(domain) used to compute registrableDomain
(the variable registrableDomain) to pass the option { allowPrivateDomains: true
} and then derive allowedHosts from that registrableDomain variable (used to
build allowedHosts array) so private suffixes are recognized and allowedHosts is
correctly scoped.
In `@templates/site-template/user/package.json`:
- Line 2: Revert the site-template package.json "name" field from the
site-specific value "myblog_blogspot_com" back to the generic placeholder used
by the template (e.g., "vite_react_shadcn_ts") so the template remains neutral
and matches the replacement behavior in scripts/generate-config.js (which
transforms domain -> domain.replace(/\./g, "_") when generating sites); update
the "name" value in templates/site-template/user/package.json accordingly.
🧹 Nitpick comments (1)
apps/web/app/chat/page.tsx (1)
789-807: Switch the mobile preview ChatInput to the compound pattern.The mobile preview still uses the monolithic ChatInput API; please migrate to the compound pattern for consistency. As per coding guidelines, Always use compound pattern for ChatInput:
<ChatInput><ChatInput.InputArea />...</ChatInput>.
- Add centralized query state keys (lib/url/queryState.ts) for consistency - Refactor ConversationSidebar: inline archive confirmation, auto-open on desktop - Fix hydration mismatches with suppressHydrationWarning and gated animations - Template: use tldts for proper eTLD+1 domain parsing in allowedHosts - Template: remove static vite configs, use generated config only - Fix shell-server-go binary name in systemd service - Update docs and various config files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix tsconfig.json: remove duplicate entries, fix typo in include paths - Fix tldts version in site-template (^7.0.22 instead of invalid ^11.17.0) - Add allowPrivateDomains option to generate-config.js for domains like github.io - Reset user/package.json name to placeholder (vite_react_shadcn_ts) - Fix SettingsPageClient: sync URL when user lacks access to admin-only tab Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
cb06294 to
50736ef
Compare
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/features/chat/components/ConversationSidebar.tsx (1)
397-432:⚠️ Potential issue | 🟠 MajorRestructure to avoid invalid nested interactive elements: move
<input>outside the<button>.Per W3C HTML specification,
<input>is interactive content and cannot be a descendant of<button>elements (excepttype="hidden"). The current structure violates this constraint and can cause focus and keyboard handling issues.Conditional rendering based on
isEditingstate is the cleanest solution: render the button wrapper only when not editing, and a plain div wrapper when editing mode is active.Suggested restructure
- <button - type="button" - disabled={isEditing} - onClick={isEditing ? undefined : onClick} - className="w-full flex items-start justify-between gap-2 text-left disabled:cursor-default bg-transparent border-none p-0 cursor-pointer" - > - <div className="flex-1 min-w-0"> - {isEditing ? ( - <input - ref={inputRef} - type="text" - value={editValue} - onChange={e => setEditValue(e.target.value)} - onBlur={handleSaveEdit} - onKeyDown={handleEditKeyDown} - className={`w-full text-sm font-medium ${styles.textPrimary} bg-black/[0.02] dark:bg-white/[0.04] border-0 rounded-lg px-2 py-1 outline-none ring-1 ring-black/[0.12] dark:ring-white/[0.12] focus:ring-black/[0.2] dark:focus:ring-white/[0.2] transition-all duration-150`} - /> - ) : ( - <div className={`text-sm font-medium ${styles.textPrimary} line-clamp-2 flex items-center gap-2`}> - {isStreaming && <StreamingDot />} - <span className="flex-1 min-w-0 truncate">{conversation.title}</span> - </div> - )} - <div className={`text-xs ${styles.textMuted} mt-0.5 flex items-center gap-1.5`}> - <span>{formatTimestamp(conversation.updatedAt)}</span> - <span className={styles.textSubtle}>·</span> - <span>{conversation.messageCount ?? 0} messages</span> - </div> - </div> - </button> + {isEditing ? ( + <div className="w-full flex items-start justify-between gap-2"> + <div className="flex-1 min-w-0"> + <input + ref={inputRef} + type="text" + value={editValue} + onChange={e => setEditValue(e.target.value)} + onBlur={handleSaveEdit} + onKeyDown={handleEditKeyDown} + className={`w-full text-sm font-medium ${styles.textPrimary} bg-black/[0.02] dark:bg-white/[0.04] border-0 rounded-lg px-2 py-1 outline-none ring-1 ring-black/[0.12] dark:ring-white/[0.12] focus:ring-black/[0.2] dark:focus:ring-white/[0.2] transition-all duration-150`} + /> + <div className={`text-xs ${styles.textMuted} mt-0.5 flex items-center gap-1.5`}> + <span>{formatTimestamp(conversation.updatedAt)}</span> + <span className={styles.textSubtle}>·</span> + <span>{conversation.messageCount ?? 0} messages</span> + </div> + </div> + </div> + ) : ( + <button + type="button" + onClick={onClick} + className="w-full flex items-start justify-between gap-2 text-left bg-transparent border-none p-0 cursor-pointer" + > + <div className="flex-1 min-w-0"> + <div className={`text-sm font-medium ${styles.textPrimary} line-clamp-2 flex items-center gap-2`}> + {isStreaming && <StreamingDot />} + <span className="flex-1 min-w-0 truncate">{conversation.title}</span> + </div> + <div className={`text-xs ${styles.textMuted} mt-0.5 flex items-center gap-1.5`}> + <span>{formatTimestamp(conversation.updatedAt)}</span> + <span className={styles.textSubtle}>·</span> + <span>{conversation.messageCount ?? 0} messages</span> + </div> + </div> + </button> + )}
🤖 Fix all issues with AI agents
In `@apps/web/features/chat/components/ConversationSidebar.tsx`:
- Around line 432-461: The rename and archive action buttons in
ConversationSidebar are hidden at the md breakpoint with opacity-0 and only
shown on group-hover, which blocks keyboard users; update the className for the
Pencil (handleStartEdit) and Archive (onArchive) buttons in
ConversationSidebar.tsx to include focus/focus-visible variants such as
md:focus:opacity-100 and md:focus-visible:opacity-100 (and optionally
md:group-focus:opacity-100) alongside the existing md:group-hover:opacity-100 so
the controls become visible when they receive keyboard focus without changing
visibility logic when isConfirming is true.
🧹 Nitpick comments (2)
apps/web/lib/url/queryState.ts (1)
11-15: RenameQUERY_KEYSto camelCase.
The constant name is uppercase, but the JS/TS guideline requires camelCase. ConsiderqueryKeysand update usages accordingly.As per coding guidelines: Use camelCase for variable names in JavaScript/TypeScript.
apps/web/app/chat/page.tsx (1)
789-807: Use the ChatInput compound pattern here.
This block still mountsChatInputas a single component; please switch to the compound pattern (<ChatInput><ChatInput.InputArea />…</ChatInput>) to match project requirements.As per coding guidelines: Always use compound pattern for ChatInput:
<ChatInput><ChatInput.InputArea />...</ChatInput>.
| {!isEditing && ( | ||
| <div className="flex items-center gap-0.5 shrink-0 mt-2 -mx-3 -mb-2 px-3 pb-2"> | ||
| {isConfirming && ( | ||
| <button | ||
| type="button" | ||
| onClick={handleStartEdit} | ||
| className={`opacity-40 md:opacity-0 md:group-hover:opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | ||
| aria-label="Rename conversation" | ||
| onClick={onCancelArchive} | ||
| className={`opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 text-black/40 dark:text-white/40`} | ||
| aria-label="Cancel archive" | ||
| > | ||
| <Pencil size={13} className={styles.textMuted} /> | ||
| <X size={13} /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={e => onArchive(e, conversation)} | ||
| className={`opacity-40 md:opacity-0 md:group-hover:opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | ||
| aria-label="Archive conversation" | ||
| > | ||
| <Archive size={13} className={styles.textMuted} /> | ||
| </button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </motion.div> | ||
| )} | ||
| <button | ||
| type="button" | ||
| onClick={handleStartEdit} | ||
| className={`${isConfirming ? "hidden" : "opacity-40 md:opacity-0 md:group-hover:opacity-100"} size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | ||
| aria-label="Rename conversation" | ||
| > | ||
| <Pencil size={13} className={styles.textMuted} /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={e => onArchive(e, conversation)} | ||
| className={`${isConfirming ? "bg-black dark:bg-white text-white dark:text-black" : "opacity-40 md:opacity-0 md:group-hover:opacity-100"} size-7 rounded-full flex items-center justify-center ${isConfirming ? "" : styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | ||
| aria-label={isConfirming ? "Confirm archive" : "Archive conversation"} | ||
| > | ||
| {isConfirming ? <Check size={13} /> : <Archive size={13} className={styles.textMuted} />} | ||
| </button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find Tailwind configuration files and check variant settings
fd -a 'tailwind.config.*' . -type f | head -20Repository: eenlars/alive
Length of output: 227
🏁 Script executed:
# Search for focus-visible, focus-within, or variants configuration in Tailwind config
rg -i "focus-visible|focus-within|focus:|variants" --type-list | head -5 && \
rg -i "focus-visible|focus-within|variants" --max-count=30Repository: eenlars/alive
Length of output: 50778
🏁 Script executed:
# Examine the component structure to verify group selector usage
cat -n apps/web/features/chat/components/ConversationSidebar.tsx | sed -n '420,465p'Repository: eenlars/alive
Length of output: 2657
🏁 Script executed:
# Check parent container structure to confirm group class is present
cat -n apps/web/features/chat/components/ConversationSidebar.tsx | sed -n '400,440p'Repository: eenlars/alive
Length of output: 2347
🏁 Script executed:
# Check parent container structure and group class
cat -n apps/web/features/chat/components/ConversationSidebar.tsx | sed -n '395,410p'Repository: eenlars/alive
Length of output: 769
🏁 Script executed:
# Find tailwind.config file
fd -type f 'tailwind.config' .Repository: eenlars/alive
Length of output: 227
🏁 Script executed:
# Check Tailwind config for focus variant configuration
find . -name "tailwind.config.*" -type f -exec cat {} \;Repository: eenlars/alive
Length of output: 3255
🏁 Script executed:
# Verify focus-visible usage in similar button contexts within the codebase
rg "focus-visible.*opacity" --max-count=5Repository: eenlars/alive
Length of output: 50371
Add focus visibility to action buttons so keyboard users can access rename/archive controls at desktop widths.
At the md breakpoint, the rename and archive buttons are hidden with opacity-0 and only appear on group-hover, making them inaccessible to keyboard users. Add focus-based variants to expose controls when they receive keyboard focus:
Suggested focus visibility updates
- className={`${isConfirming ? "hidden" : "opacity-40 md:opacity-0 md:group-hover:opacity-100"} size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`}
+ className={`${isConfirming ? "hidden" : "opacity-40 md:opacity-0 md:group-hover:opacity-100 md:group-focus-within:opacity-100 focus-visible:opacity-100"} size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`}
@@
- className={`${isConfirming ? "bg-black dark:bg-white text-white dark:text-black" : "opacity-40 md:opacity-0 md:group-hover:opacity-100"} size-7 rounded-full flex items-center justify-center ${isConfirming ? "" : styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`}
+ className={`${isConfirming ? "bg-black dark:bg-white text-white dark:text-black" : "opacity-40 md:opacity-0 md:group-hover:opacity-100 md:group-focus-within:opacity-100 focus-visible:opacity-100"} size-7 rounded-full flex items-center justify-center ${isConfirming ? "" : styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`}📝 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.
| {!isEditing && ( | |
| <div className="flex items-center gap-0.5 shrink-0 mt-2 -mx-3 -mb-2 px-3 pb-2"> | |
| {isConfirming && ( | |
| <button | |
| type="button" | |
| onClick={handleStartEdit} | |
| className={`opacity-40 md:opacity-0 md:group-hover:opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | |
| aria-label="Rename conversation" | |
| onClick={onCancelArchive} | |
| className={`opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 text-black/40 dark:text-white/40`} | |
| aria-label="Cancel archive" | |
| > | |
| <Pencil size={13} className={styles.textMuted} /> | |
| <X size={13} /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={e => onArchive(e, conversation)} | |
| className={`opacity-40 md:opacity-0 md:group-hover:opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | |
| aria-label="Archive conversation" | |
| > | |
| <Archive size={13} className={styles.textMuted} /> | |
| </button> | |
| </div> | |
| )} | |
| </div> | |
| </motion.div> | |
| )} | |
| <button | |
| type="button" | |
| onClick={handleStartEdit} | |
| className={`${isConfirming ? "hidden" : "opacity-40 md:opacity-0 md:group-hover:opacity-100"} size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | |
| aria-label="Rename conversation" | |
| > | |
| <Pencil size={13} className={styles.textMuted} /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={e => onArchive(e, conversation)} | |
| className={`${isConfirming ? "bg-black dark:bg-white text-white dark:text-black" : "opacity-40 md:opacity-0 md:group-hover:opacity-100"} size-7 rounded-full flex items-center justify-center ${isConfirming ? "" : styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | |
| aria-label={isConfirming ? "Confirm archive" : "Archive conversation"} | |
| > | |
| {isConfirming ? <Check size={13} /> : <Archive size={13} className={styles.textMuted} />} | |
| </button> | |
| </div> | |
| )} | |
| {!isEditing && ( | |
| <div className="flex items-center gap-0.5 shrink-0 mt-2 -mx-3 -mb-2 px-3 pb-2"> | |
| {isConfirming && ( | |
| <button | |
| type="button" | |
| onClick={onCancelArchive} | |
| className={`opacity-100 size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 text-black/40 dark:text-white/40`} | |
| aria-label="Cancel archive" | |
| > | |
| <X size={13} /> | |
| </button> | |
| )} | |
| <button | |
| type="button" | |
| onClick={handleStartEdit} | |
| className={`${isConfirming ? "hidden" : "opacity-40 md:opacity-0 md:group-hover:opacity-100 md:group-focus-within:opacity-100 focus-visible:opacity-100"} size-7 rounded-full flex items-center justify-center ${styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | |
| aria-label="Rename conversation" | |
| > | |
| <Pencil size={13} className={styles.textMuted} /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={e => onArchive(e, conversation)} | |
| className={`${isConfirming ? "bg-black dark:bg-white text-white dark:text-black" : "opacity-40 md:opacity-0 md:group-hover:opacity-100 md:group-focus-within:opacity-100 focus-visible:opacity-100"} size-7 rounded-full flex items-center justify-center ${isConfirming ? "" : styles.hoverFillStrong} ${styles.transitionAll} active:scale-95 md:hover:scale-110`} | |
| aria-label={isConfirming ? "Confirm archive" : "Archive conversation"} | |
| > | |
| {isConfirming ? <Check size={13} /> : <Archive size={13} className={styles.textMuted} />} | |
| </button> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In `@apps/web/features/chat/components/ConversationSidebar.tsx` around lines 432 -
461, The rename and archive action buttons in ConversationSidebar are hidden at
the md breakpoint with opacity-0 and only shown on group-hover, which blocks
keyboard users; update the className for the Pencil (handleStartEdit) and
Archive (onArchive) buttons in ConversationSidebar.tsx to include
focus/focus-visible variants such as md:focus:opacity-100 and
md:focus-visible:opacity-100 (and optionally md:group-focus:opacity-100)
alongside the existing md:group-hover:opacity-100 so the controls become visible
when they receive keyboard focus without changing visibility logic when
isConfirming is true.
Summary
lib/url/queryState.ts) for URL parameter consistencysuppressHydrationWarningand gated animationsallowedHostsTest plan
?wk=,?tab=) work correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Changes