-
Notifications
You must be signed in to change notification settings - Fork 516
feat: replace font size slider with button group for better UX #788
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
WalkthroughReplaced the font-size slider in DisplaySettings.vue with a five-button selector bound to fontSizeLevel via a new fontSizeOptions array. Removed the computed fontSizeClass. Updated font-size labels across multiple locales (en, fa, fr, ja, ko, ru, zh-*) to new wording while keeping keys unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DisplaySettings as DisplaySettings.vue
participant SettingsStore as SettingsStore
User->>DisplaySettings: Click size button (e.g., XL)
DisplaySettings->>SettingsStore: set fontSizeLevel(index)
SettingsStore-->>DisplaySettings: state updated (fontSizeLevel)
Note over DisplaySettings: Computes active button from fontSizeLevel
DisplaySettings-->>User: UI re-renders with selected button
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
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/settings/DisplaySettings.vue (1)
35-36
: Remove hardcoded fallbacks and non-English logs; rely on i18n and English-only dev text
- Per repo rules, all user-facing strings must use i18n; don't OR-fallback to hardcoded Chinese.
- Dev logs/comments must be English.
Apply this diff:
- <span class="text-sm font-medium">{{ - t('settings.common.notifications') || '系统通知' - }}</span> + <span class="text-sm font-medium">{{ t('settings.common.notifications') }}</span>- <span class="text-sm font-medium">{{ - t('settings.common.contentProtection') || '投屏保护' - }}</span> + <span class="text-sm font-medium">{{ t('settings.common.contentProtection') }}</span>- <DialogTitle>{{ - t('settings.common.contentProtectionDialogTitle') || '确认切换投屏保护' - }}</DialogTitle> + <DialogTitle>{{ t('settings.common.contentProtectionDialogTitle') }}</DialogTitle>- { value: 'system', label: t('common.languageSystem') || '跟随系统' }, // 使用i18n key 或 默认值 + { value: 'system', label: t('common.languageSystem') },- console.log('准备切换投屏保护状态:', value) + console.log('DisplaySettings: toggling content protection', value)Follow-up (optional but recommended): move all language labels in languageOptions to i18n keys (e.g., common.localeNames.en-US, common.localeNames.zh-CN, …) instead of hardcoding.
Also applies to: 79-80, 118-119, 182-182, 220-220
🧹 Nitpick comments (7)
src/renderer/src/i18n/ko-KR/settings.json (1)
740-745
: Polish Korean labels for more natural UI phrasing.Current nouns/adjectives (“작음/큼”) feel less conventional in settings. Typical Korean UIs use adverbs or short phrases: “작게/보통/크게/아주 크게/가장 크게”. Consider this tweak for clarity while retaining XL/XXL hints if desired.
Proposed diff:
- "text-sm": "작음", - "text-base": "보통", - "text-lg": "큼", - "text-xl": "XL", - "text-2xl": "XXL", + "text-sm": "작게", + "text-base": "보통", + "text-lg": "크게", + "text-xl": "아주 크게 (XL)", + "text-2xl": "가장 크게 (XXL)",src/renderer/src/components/settings/DisplaySettings.vue (4)
60-71
: Button group: improve accessibility and key stability
- Use a stable key instead of the loop index.
- Expose pressed state to assistive tech with aria-pressed.
- Group the buttons for SR users via role="group" and an aria-label.
- Minor: set type="button" to avoid implicit submit behavior.
Apply this diff:
- <div class="flex flex-wrap items-center gap-1.5 pl-6"> + <div + class="flex flex-wrap items-center gap-1.5 pl-6" + role="group" + :aria-label="t('settings.display.fontSize')" + > <Button v-for="(sizeOption, index) in fontSizeOptions" - :key="index" + :key="sizeOption" :variant="fontSizeLevel === index ? 'default' : 'outline'" size="sm" - class="px-2 py-1.5 text-xs flex-shrink-0" + class="px-2 py-1.5 text-xs flex-shrink-0" + :aria-pressed="fontSizeLevel === index" + :id="`font-size-${sizeOption}`" + type="button" @click="fontSizeLevel = index" > {{ t('settings.display.' + sizeOption) }} </Button> </div>
4-4
: Convert developer comments to English per codebase rulesCurrent inline comments are Chinese-only. Please translate to concise English for consistency.
Example (no functional change needed):
- 语言选择 → Language selection
- 系统通知设置 → System notifications
- 字体大小设置 → Font size
- 投屏保护开关 → Screen capture protection
- 悬浮按钮开关 → Floating button
- 投屏保护切换确认对话框 → Content protection toggle dialog
Also applies to: 29-29, 51-51, 74-74, 91-91, 113-113
46-48
: RTL/LTR padding: prefer direction-aware utilitiesThese use fixed left padding (pl-6). For RTL locales, this indents the wrong side. Consider Tailwind direction variants.
Apply this diff if direction variants are enabled:
- <div class="pl-6 text-xs text-muted-foreground"> + <div class="ltr:pl-6 rtl:pr-6 text-xs text-muted-foreground"> {{ t('settings.common.notificationsDesc') }} </div>- <div class="pl-6 text-xs text-muted-foreground"> + <div class="ltr:pl-6 rtl:pr-6 text-xs text-muted-foreground"> {{ t('settings.display.floatingButtonDesc') }} </div>Also applies to: 106-108
200-201
: Strengthen typing forfontSizeOptions
and plan a token-based migrationTo improve type safety, mark the options tuple as
const
. If you decide to migrate from an index to a token (e.g.'text-lg'
), be aware this impacts several parts of the codebase.Minimal typing diff:
-const fontSizeOptions = ['text-sm', 'text-base', 'text-lg', 'text-xl', 'text-2xl'] +const fontSizeOptions = ['text-sm', 'text-base', 'text-lg', 'text-xl', 'text-2xl'] as constFiles to update if migrating to token-based API:
- src/renderer/src/components/settings/DisplaySettings.vue
- Remove index coupling in
v-for="(sizeOption, index) in fontSizeOptions"
- Change computed and setter from numeric
fontSizeLevel
to stringfontSizeToken
- Update
@click="fontSizeLevel = index"
to e.g.@click="updateFontSize(sizeOption)"
- src/renderer/src/App.vue
- Update calls to
settingsStore.updateFontSizeLevel(currentLevel ± 1)
tosettingsStore.updateFontSize(token)
- src/main/presenter/configPresenter/index.ts
- Adjust event key checks (
if (key === 'fontSizeLevel')
) and the payload to use token values- src/renderer/src/stores/settings.ts
- Rename
fontSizeLevel: ref<number>
tofontSizeToken: ref<FontSize>
- Replace
FONT_SIZE_CLASSES[level]
lookup with identity mapping or explicit record- Change
updateFontSizeLevel(level: number)
signature toupdateFontSize(token: FontSize)
and persist that token- Update all references to
fontSizeLevel.value
andupdateFontSizeLevel
Before proceeding with the migration, manually verify that all downstream components and IPC handlers are updated to consume the new token-based API.
src/renderer/src/i18n/en-US/settings.json (1)
741-744
: Font-size labels read well in EnglishSmall/Medium/Large/X-Large/XX-Large is clear and consistent with common UI wording. No key changes required.
Optional: If you prefer spelled-out adjectives, consider "Extra Large" and "Extra Extra Large". Not required.
src/renderer/src/i18n/ja-JP/settings.json (1)
740-744
: JA font-size wording: consider 標準 for text-baseCurrent: 小 / 中 / 大 / XL / XXL. Many Japanese UIs use 標準 for the default/medium size. “中” is understandable but a bit telegraphic in settings.
Suggested tweak:
- "text-base": "中", + "text-base": "標準",If product copy prefers the compact 小/中/大 trio, feel free to keep as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
src/renderer/src/components/settings/DisplaySettings.vue
(2 hunks)src/renderer/src/i18n/en-US/settings.json
(1 hunks)src/renderer/src/i18n/fa-IR/settings.json
(1 hunks)src/renderer/src/i18n/fr-FR/settings.json
(1 hunks)src/renderer/src/i18n/ja-JP/settings.json
(1 hunks)src/renderer/src/i18n/ko-KR/settings.json
(1 hunks)src/renderer/src/i18n/ru-RU/settings.json
(1 hunks)src/renderer/src/i18n/zh-CN/settings.json
(1 hunks)src/renderer/src/i18n/zh-HK/settings.json
(1 hunks)src/renderer/src/i18n/zh-TW/settings.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
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/i18n/zh-CN/settings.json
src/renderer/src/i18n/zh-TW/settings.json
src/renderer/src/i18n/ja-JP/settings.json
src/renderer/src/i18n/fa-IR/settings.json
src/renderer/src/i18n/zh-HK/settings.json
src/renderer/src/i18n/ru-RU/settings.json
src/renderer/src/i18n/en-US/settings.json
src/renderer/src/i18n/ko-KR/settings.json
src/renderer/src/i18n/fr-FR/settings.json
src/renderer/src/components/settings/DisplaySettings.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/src/components/settings/DisplaySettings.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/src/**/*.vue
: Use Composition API for all Vue 3 components
Use Tailwind CSS with scoped styles for styling
Organize components by feature in src/renderer/src/
Follow existing component patterns in src/renderer/src/ when creating new UI components
Use Composition API with proper TypeScript typing for new UI components
Implement responsive design with Tailwind CSS for new UI components
Add proper error handling and loading states for new UI componentsUse scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/settings/DisplaySettings.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/src/**/*.{ts,tsx,vue}
: Use Pinia for frontend state management
Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Files:
src/renderer/src/components/settings/DisplaySettings.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/settings/DisplaySettings.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/settings/DisplaySettings.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/settings/DisplaySettings.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/settings/DisplaySettings.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.
Files:
src/renderer/src/components/settings/DisplaySettings.vue
🧠 Learnings (3)
📚 Learning: 2025-07-21T01:46:30.354Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-07-21T01:46:30.354Z
Learning: Applies to src/renderer/src/i18n/*.json : Maintain consistent structure across all translation files
Applied to files:
src/renderer/src/i18n/zh-CN/settings.json
src/renderer/src/i18n/zh-TW/settings.json
src/renderer/src/i18n/ja-JP/settings.json
src/renderer/src/i18n/zh-HK/settings.json
src/renderer/src/i18n/ru-RU/settings.json
src/renderer/src/i18n/ko-KR/settings.json
📚 Learning: 2025-07-21T01:46:30.354Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-07-21T01:46:30.354Z
Learning: Applies to src/renderer/src/i18n/common.json : Shared translation keys must be placed in 'src/renderer/src/i18n/common.json'
Applied to files:
src/renderer/src/i18n/zh-CN/settings.json
src/renderer/src/i18n/ru-RU/settings.json
📚 Learning: 2025-07-21T01:46:30.354Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-07-21T01:46:30.354Z
Learning: Applies to src/renderer/src/**/* : Ensure all user-visible text in the renderer uses the translation system
Applied to files:
src/renderer/src/i18n/zh-CN/settings.json
src/renderer/src/i18n/ja-JP/settings.json
src/renderer/src/i18n/zh-HK/settings.json
⏰ 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 (6)
src/renderer/src/i18n/zh-CN/settings.json (1)
741-741
: LGTM: “text-base” → “标准” aligns with zh-HK/zh-TW and the new button-group UI.Key names remain stable and the label reads naturally in Simplified Chinese. No further changes needed here.
src/renderer/src/i18n/zh-HK/settings.json (1)
741-745
: LGTM: Updated font-size labels read naturally; “標準” for text-base matches zh-TW.The five options remain present and consistent with DisplaySettings.vue expectations. Good to go.
src/renderer/src/i18n/ru-RU/settings.json (1)
740-745
: LGTM: Clear, concise Russian labels; consistent with other locales’ XL/XXL convention.“Малый/Средний/Большой/XL/XXL” are unambiguous in a compact control. No issues spotted.
src/renderer/src/i18n/zh-TW/settings.json (1)
741-745
: LGTM: “標準/大/特大/超大” are idiomatic; keys unchanged.Matches the button group’s five discrete sizes and keeps parity with zh-HK/zh-CN.
src/renderer/src/i18n/fr-FR/settings.json (1)
741-744
: FR font-size labels look good“Moyen / Grand / XL / XXL” is concise and idiomatic for button labels. Keys remain stable, which is important for the new button group.
src/renderer/src/i18n/fa-IR/settings.json (1)
741-744
: FA font-size labels are consistent with UI“کوچک / متوسط / بزرگ / XL / XXL” reads clearly. Retaining Latin XL/XXL is acceptable and common in Persian UIs.
replace font size slider with button group for better UX
Summary by CodeRabbit