fix: restore the ThinkContent component to its previous markup and styling so the timer text behaves as before #987
Conversation
WalkthroughRefactors ThreadView sidebar rendering: consolidates backdrop and nested Transition into a single outer container with an outer Transition. Adds click.self to close on backdrop click, uses dir-aware classes for LTR/RTL slide animations, and retains Escape-key handling. No API surface changes; visibility still controlled by chatStore.isSidebarOpen. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as ThreadView.vue
participant Store as chatStore
rect rgb(243,246,249)
note right of UI: Sidebar open when chatStore.isSidebarOpen = true
end
User->>UI: Click backdrop (outer container)
UI->>UI: @click.self triggers closeSidebar()
UI->>Store: set isSidebarOpen = false
UI-->>User: Run leave transition (LTR/RTL aware)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/ThreadView.vue (1)
1-65: Critical: PR objectives don't match the actual file changes.The PR title and description state this is about reverting ThinkContent component shimmer changes, but the file under review is ThreadView.vue with sidebar rendering modifications. This is a major inconsistency that suggests either:
- The wrong file was included in this PR, or
- The PR description is incorrect
Please verify and update either the PR description or the file list to ensure consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/ThreadView.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/components/ThreadView.vue
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/ThreadView.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/ThreadView.vue
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/components/ThreadView.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/ThreadView.vue
⏰ 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: build-check (x64)
🔇 Additional comments (1)
src/renderer/src/components/ThreadView.vue (1)
39-65: Script section looks good.The Composition API usage is correct, stores are properly imported, event listeners are cleaned up in onBeforeUnmount, and TypeScript typing is appropriate.
| <div | ||
| v-if="chatStore.isSidebarOpen" | ||
| class="fixed inset-0 z-50 flex" | ||
| :class="langStore.dir === 'rtl' ? 'justify-end' : 'justify-start'" | ||
| :dir="langStore.dir" | ||
| @click.self="closeSidebar" | ||
| > | ||
| <Transition | ||
| enter-active-class="transition-transform duration-200 ease-out" | ||
| leave-active-class="transition-transform duration-150 ease-in" | ||
| :enter-from-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'" | ||
| :leave-to-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'" | ||
| > | ||
| <Transition | ||
| enter-active-class="transition-transform duration-200 ease-out" | ||
| leave-active-class="transition-transform duration-150 ease-in" | ||
| :enter-from-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'" | ||
| :leave-to-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'" | ||
| <div | ||
| v-if="chatStore.isSidebarOpen" | ||
| :class="[ | ||
| 'h-full w-60 max-w-60 shadow-lg bg-card', | ||
| langStore.dir === 'rtl' ? 'border-l' : 'border-r', | ||
| 'border-border' | ||
| ]" | ||
| @click.stop | ||
| > | ||
| <div | ||
| v-if="chatStore.isSidebarOpen" | ||
| class="h-full w-60 max-w-60 shadow-lg border-r border-border bg-card" | ||
| > | ||
| <ThreadsView class="h-full" /> | ||
| </div> | ||
| </Transition> | ||
| </div> | ||
| <ThreadsView class="h-full" /> | ||
| </div> | ||
| </Transition> | ||
| </div> |
There was a problem hiding this comment.
Missing accessibility attributes for modal-like sidebar.
The sidebar overlay is missing essential accessibility features:
- No ARIA role (should be
role="complementary"orrole="dialog") - No
aria-modal="true"to indicate modal behavior - No
aria-labeloraria-labelledbyto describe the sidebar - No focus trap to prevent tabbing outside the sidebar
- No focus management when opening/closing
These omissions make the sidebar difficult to use with screen readers and keyboard navigation.
As per coding guidelines on accessibility best practices.
🤖 Prompt for AI Agents
In src/renderer/src/components/ThreadView.vue around lines 9 to 34, the sidebar
overlay and panel lack accessibility attributes and focus management; add an
appropriate ARIA role (e.g., role="dialog" or role="complementary"),
aria-modal="true", and either aria-label or aria-labelledby on the panel
container, implement focus management to move focus into the sidebar when it
opens (focus the panel or its first focusable element) and restore focus to the
previously focused element on close, and add a keyboard handler (Escape) to
close the sidebar; also implement a simple focus trap so Tab/Shift+Tab cycles
inside the sidebar while open and ensure @click.self still closes the overlay.
| <div | ||
| v-if="chatStore.isSidebarOpen" | ||
| class="fixed inset-0 z-50 flex" | ||
| :class="langStore.dir === 'rtl' ? 'justify-end' : 'justify-start'" | ||
| :dir="langStore.dir" | ||
| @click.self="closeSidebar" | ||
| > |
There was a problem hiding this comment.
Missing backdrop overlay styling.
The outer container acts as a clickable backdrop but has no visual styling (e.g., semi-transparent background). Users won't see a clear overlay indicating the sidebar is modal.
Apply this diff to add a semi-transparent backdrop:
<div
v-if="chatStore.isSidebarOpen"
- class="fixed inset-0 z-50 flex"
+ class="fixed inset-0 z-50 flex bg-black/50"
:class="langStore.dir === 'rtl' ? 'justify-end' : 'justify-start'"
:dir="langStore.dir"
@click.self="closeSidebar"
>📝 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.
| <div | |
| v-if="chatStore.isSidebarOpen" | |
| class="fixed inset-0 z-50 flex" | |
| :class="langStore.dir === 'rtl' ? 'justify-end' : 'justify-start'" | |
| :dir="langStore.dir" | |
| @click.self="closeSidebar" | |
| > | |
| <div | |
| v-if="chatStore.isSidebarOpen" | |
| class="fixed inset-0 z-50 flex bg-black/50" | |
| :class="langStore.dir === 'rtl' ? 'justify-end' : 'justify-start'" | |
| :dir="langStore.dir" | |
| @click.self="closeSidebar" | |
| > |
🤖 Prompt for AI Agents
In src/renderer/src/components/ThreadView.vue around lines 9 to 15, the outer
div acting as the clickable backdrop has no visual overlay; update the element
to include a semi-transparent backdrop by adding a background utility (e.g.,
Tailwind: "bg-black/50" or for older Tailwind "bg-black bg-opacity-50") and
optionally a subtle backdrop blur ("backdrop-blur-sm") so users see a modal
overlay; keep the existing :class, :dir and @click.self behavior unchanged.
| <div | ||
| v-if="chatStore.isSidebarOpen" |
There was a problem hiding this comment.
Redundant v-if may break the inner Transition animation.
The inner div has v-if="chatStore.isSidebarOpen" on Line 23, but the outer container already has the same condition on Line 10. This redundancy can interfere with the inner Transition's animation because the child element should always be present for the Transition to work correctly—the Transition itself should control the visibility.
Apply this diff to remove the redundant v-if:
<Transition
enter-active-class="transition-transform duration-200 ease-out"
leave-active-class="transition-transform duration-150 ease-in"
:enter-from-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'"
:leave-to-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'"
>
<div
- v-if="chatStore.isSidebarOpen"
:class="[
'h-full w-60 max-w-60 shadow-lg bg-card',
langStore.dir === 'rtl' ? 'border-l' : 'border-r',
'border-border'
]"
@click.stop
>📝 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.
| <div | |
| v-if="chatStore.isSidebarOpen" | |
| <Transition | |
| enter-active-class="transition-transform duration-200 ease-out" | |
| leave-active-class="transition-transform duration-150 ease-in" | |
| :enter-from-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'" | |
| :leave-to-class="langStore.dir === 'rtl' ? 'translate-x-full' : '-translate-x-full'" | |
| > | |
| <div | |
| :class="[ | |
| 'h-full w-60 max-w-60 shadow-lg bg-card', | |
| langStore.dir === 'rtl' ? 'border-l' : 'border-r', | |
| 'border-border' | |
| ]" | |
| @click.stop | |
| > |
🤖 Prompt for AI Agents
In src/renderer/src/components/ThreadView.vue around lines 22–23, remove the
redundant v-if="chatStore.isSidebarOpen" on the inner div (the outer container
already guards visibility) so the element stays mounted for the inner
<Transition> to control animations; simply delete the v-if from the inner div so
only the outer condition determines presence and the Transition manages
showing/hiding.
zerob13
changed the title
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68ebab620f70832c84183b8f1701fda4
Summary by CodeRabbit
New Features
Refactor