Skip to content

fix: add scrollable support to PopoverContent to prevent overflow #720

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.

Sign up for GitHub

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

Merged
zerob13 merged 2 commits into from
Aug 11, 2025
Merged

fix: add scrollable support to PopoverContent to prevent overflow #720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27c26b0
fix: add scrollable support to PopoverContent to prevent overflow
yyhhyyyyyy Aug 9, 2025
45b1661
refactor: extract scrollable popover functionality from shadcn ui com…
yyhhyyyyyy Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions src/renderer/src/components/NewThread.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,14 @@
/>
</PopoverContent>
</Popover>
<Popover v-model:open="settingsPopoverOpen" @update:open="handleSettingsPopoverUpdate">
<PopoverTrigger as-child>
<ScrollablePopover
v-model:open="settingsPopoverOpen"
@update:open="handleSettingsPopoverUpdate"
align="start"
content-class="w-80"
:enable-scrollable="true"
>
<template #trigger>
<Button
class="w-7 h-full rounded-none border-none shadow-none transition-all duration-300"
:class="{
Expand All @@ -77,21 +83,19 @@
>
<Icon icon="lucide:settings-2" class="w-4 h-4" />
</Button>
</PopoverTrigger>
<PopoverContent align="start" class="p-0 w-80">
<ChatConfig
v-model:temperature="temperature"
v-model:context-length="contextLength"
v-model:max-tokens="maxTokens"
v-model:system-prompt="systemPrompt"
v-model:artifacts="artifacts"
:context-length-limit="contextLengthLimit"
:max-tokens-limit="maxTokensLimit"
:model-id="activeModel?.id"
:provider-id="activeModel?.providerId"
/>
</PopoverContent>
</Popover>
</template>
<ChatConfig
v-model:temperature="temperature"
v-model:context-length="contextLength"
v-model:max-tokens="maxTokens"
v-model:system-prompt="systemPrompt"
v-model:artifacts="artifacts"
:context-length-limit="contextLengthLimit"
:max-tokens-limit="maxTokensLimit"
:model-id="activeModel?.id"
:provider-id="activeModel?.providerId"
/>
</ScrollablePopover>
</div>
</template>
</ChatInput>
Expand All @@ -104,6 +108,7 @@
import { useI18n } from 'vue-i18n'
import ChatInput from './ChatInput.vue'
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
import ScrollablePopover from './ScrollablePopover.vue'
import { Button } from '@/components/ui/button'
import ModelIcon from './icons/ModelIcon.vue'
import { Badge } from '@/components/ui/badge'
Expand Down
34 changes: 34 additions & 0 deletions src/renderer/src/components/ScrollablePopover.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<template>
<Popover v-bind="$attrs">
<PopoverTrigger as-child>
<slot name="trigger" />
</PopoverTrigger>
<PopoverContent
:align="align"
:class="cn('p-0', contentClass, enableScrollable && 'max-h-96 overflow-hidden')"
>
<div v-if="enableScrollable" class="max-h-96 overflow-y-auto">
<slot />
</div>
<slot v-else />
</PopoverContent>
</Popover>
</template>

<script setup lang="ts">
import type { HTMLAttributes } from 'vue'
import { cn } from '@/lib/utils'
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'

interface ScrollablePopoverProps {
align?: 'start' | 'center' | 'end'
enableScrollable?: boolean
contentClass?: HTMLAttributes['class']
}

withDefaults(defineProps<ScrollablePopoverProps>(), {
align: 'center',
enableScrollable: false,
contentClass: ''
})
Comment on lines +23 to +33
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Type the v-model:open API and prefer type alias over interface (per guidelines)

For better DX with Volar and to satisfy “Use TypeScript types over interfaces,” explicitly type the open prop and update:open emit. Also forward them to the underlying Popover, so v-model:open is fully typed and not just passed via $attrs.

-interface ScrollablePopoverProps {
-  align?: 'start' | 'center' | 'end'
-  enableScrollable?: boolean
-  contentClass?: HTMLAttributes['class']
-}
+type ScrollablePopoverProps = {
+  align?: 'start' | 'center' | 'end'
+  enableScrollable?: boolean
+  contentClass?: HTMLAttributes['class']
+  // expose v-model:open for proper typing
+  open?: boolean
+}
 
-withDefaults(defineProps<ScrollablePopoverProps>(), {
+withDefaults(defineProps<ScrollablePopoverProps>(), {
   align: 'center',
   enableScrollable: false,
   contentClass: ''
 })

Additionally update the template to explicitly forward the open state and emit updates:

<script setup lang="ts">
const emit = defineEmits<{ (e: 'update:open', value: boolean): void }>()
// (keep the existing defineProps above)
</script>

<template>
  <Popover
-   v-bind="$attrs"
+   :open="open"
+   @update:open="emit('update:open', $event)"
+   v-bind="$attrs"
  >
    ...
  </Popover>
</template>

Optional naming consistency with guidelines: consider renaming enableScrollable to isScrollable (and aliasing the old name for backward compatibility).

🤖 Prompt for AI Agents 
In src/renderer/src/components/ScrollablePopover.vue around lines 23–33, replace
the interface with a TypeScript type alias that includes an explicit open?:
boolean prop (so v-model:open is typed), keep withDefaults for defaults, and add
a typed defineEmits signature for update:open; then update the template Popover
usage to explicitly pass :open="open" and forward @update:open by calling
emit('update:open', $event) before v-bind="$attrs". Optionally rename
enableScrollable to isScrollable and provide a backward-compatible alias to
preserve existing usage.

</script>
41 changes: 20 additions & 21 deletions src/renderer/src/components/TitleView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,30 +43,28 @@
</div>

<div class="flex items-center gap-2">
<Popover>
<PopoverTrigger as-child>
<ScrollablePopover align="end" content-class="w-80" :enable-scrollable="true">
<template #trigger>
<Button class="w-7 h-7 rounded-md" size="icon" variant="outline">
<Icon icon="lucide:settings-2" class="w-4 h-4" />
</Button>
</PopoverTrigger>
<PopoverContent align="end" class="p-0 w-80">
<ChatConfig
v-model:system-prompt="systemPrompt"
:temperature="temperature"
:context-length="contextLength"
:max-tokens="maxTokens"
:artifacts="artifacts"
:thinking-budget="thinkingBudget"
:model-id="chatStore.chatConfig.modelId"
:provider-id="chatStore.chatConfig.providerId"
@update:temperature="updateTemperature"
@update:context-length="updateContextLength"
@update:max-tokens="updateMaxTokens"
@update:artifacts="updateArtifacts"
@update:thinking-budget="updateThinkingBudget"
/>
</PopoverContent>
</Popover>
</template>
<ChatConfig
v-model:system-prompt="systemPrompt"
:temperature="temperature"
:context-length="contextLength"
:max-tokens="maxTokens"
:artifacts="artifacts"
:thinking-budget="thinkingBudget"
:model-id="chatStore.chatConfig.modelId"
:provider-id="chatStore.chatConfig.providerId"
@update:temperature="updateTemperature"
@update:context-length="updateContextLength"
@update:max-tokens="updateMaxTokens"
@update:artifacts="updateArtifacts"
@update:thinking-budget="updateThinkingBudget"
/>
</ScrollablePopover>
</div>
</div>
</template>
Expand All @@ -77,6 +75,7 @@ import { Icon } from '@iconify/vue'
import { Button } from '@/components/ui/button'
import { Badge } from '@/components/ui/badge'
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
import ScrollablePopover from './ScrollablePopover.vue'
import ChatConfig from './ChatConfig.vue'
import ModelSelect from './ModelSelect.vue'
import ModelIcon from './icons/ModelIcon.vue'
Expand Down
4 changes: 3 additions & 1 deletion src/renderer/src/components/ui/popover/PopoverContent.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ defineOptions({
})

const props = withDefaults(
defineProps<PopoverContentProps & { class?: HTMLAttributes['class'] }>(),
defineProps<PopoverContentProps & {
class?: HTMLAttributes['class']
}>(),
{
align: 'center',
sideOffset: 4
Expand Down