-
Notifications
You must be signed in to change notification settings - Fork 8.3k
add global font size adjustment #6909
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
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughAdds a global, user-adjustable base font size preference (15–22px). The setting is stored in theme preferences, exposed as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PreferencesUI as Preferences UI
participant ThemeConfig as Theme Config
participant CSSEngine as CSS Engine
participant Components as UI Components
User->>PreferencesUI: change font size (15–22px)
PreferencesUI->>ThemeConfig: update themeFontSize model
ThemeConfig->>CSSEngine: set --font-size-base (e.g., "16px")
CSSEngine->>Components: styles recompute (calc(var(--font-size-base)))
Components-->>User: render scaled UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/@core/ui-kit/menu-ui/src/components/menu.vue (1)
839-853: Inconsistent hardcoded font sizes in submenu collapse state.Lines 841 and 849 use hardcoded font sizes (
20pxand12px) instead of the CSS variable approach used throughout the rest of this PR. This creates an inconsistency where submenu items won't scale with the global font size setting.Apply this diff to align with the variable-based approach:
.#{$namespace}-menu__icon { display: block; - font-size: 20px !important; + font-size: calc(var(--font-size-base, 16px) * 1.25) !important; transition: all 0.25s ease; } .#{$namespace}-sub-menu-content__title { display: inline-flex; flex-shrink: 0; margin-top: 8px; margin-bottom: 0; - font-size: 12px; + font-size: calc(var(--font-size-base, 16px) * 0.75); font-weight: 400; line-height: normal; transition: all 0.25s ease; }
🧹 Nitpick comments (1)
packages/effects/layouts/src/widgets/preferences/blocks/theme/font-size.vue (1)
27-37: Consider removing the redundant watcher.The
NumberFieldcomponent already handles min/max validation through its:minand:maxprops (lines 45-46), making the manual clamping watcher redundant. The component should prevent out-of-range values natively.Apply this diff to simplify the component:
-import { watch } from 'vue'; - import { $t } from '@vben/locales'; import { @@ -21,19 +19,6 @@ const modelValue = defineModel<number>({ const min = 15; const max = 22; const step = 1; - -// 限制输入值在 min 和 max 之间 -watch( - modelValue, - (newValue) => { - if (newValue < min) { - modelValue.value = min; - } else if (newValue > max) { - modelValue.value = max; - } - }, - { immediate: true }, -);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
package.json(1 hunks)packages/@core/base/design/src/css/global.css(1 hunks)packages/@core/preferences/src/config.ts(1 hunks)packages/@core/preferences/src/types.ts(1 hunks)packages/@core/preferences/src/update-css-variables.ts(1 hunks)packages/@core/ui-kit/menu-ui/src/components/menu.vue(3 hunks)packages/@core/ui-kit/menu-ui/src/components/normal-menu/normal-menu.vue(2 hunks)packages/effects/layouts/src/widgets/preferences/blocks/index.ts(1 hunks)packages/effects/layouts/src/widgets/preferences/blocks/theme/font-size.vue(1 hunks)packages/effects/layouts/src/widgets/preferences/preferences-drawer.vue(3 hunks)packages/locales/src/langs/en-US/preferences.json(1 hunks)packages/locales/src/langs/zh-CN/preferences.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-15T04:29:13.944Z
Learnt from: mynetfan
Repo: vbenjs/vue-vben-admin PR: 5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Applied to files:
packages/@core/ui-kit/menu-ui/src/components/menu.vuepackages/@core/ui-kit/menu-ui/src/components/normal-menu/normal-menu.vue
📚 Learning: 2025-02-08T07:36:14.722Z
Learnt from: dingdayu
Repo: vbenjs/vue-vben-admin PR: 5500
File: packages/@core/ui-kit/tabs-ui/src/components/tabs-chrome/tabs.vue:51-51
Timestamp: 2025-02-08T07:36:14.722Z
Learning: The project uses changesets for version management and changelog generation. Breaking changes should be documented by creating a new changeset using `pnpm changeset` command, selecting affected packages, choosing "major" as the bump type, and providing a detailed description of the change and its impact.
Applied to files:
package.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). (6)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (12)
packages/locales/src/langs/zh-CN/preferences.json (1)
123-124: LGTM!The Chinese localization keys for font size are well-structured and provide clear labels and helpful tooltips for users.
packages/locales/src/langs/en-US/preferences.json (1)
123-124: LGTM!The English localization keys are clear and consistent with the Chinese translations, providing good user guidance.
packages/@core/preferences/src/update-css-variables.ts (1)
70-76: LGTM!The CSS variable update logic for font size is well-implemented and follows the same pattern as the existing radius handling. The guard check and unit handling are correct.
packages/@core/preferences/src/types.ts (1)
242-243: LGTM!The fontSize field is properly typed and documented with clear unit specification. The addition is non-breaking and aligns with the theme preference structure.
packages/effects/layouts/src/widgets/preferences/blocks/index.ts (1)
18-18: LGTM!The FontSize export is cleanly added and follows the existing pattern for theme-related components. The placement between ColorMode and Radius is logical and maintains alphabetical ordering within the theme section.
packages/@core/preferences/src/config.ts (1)
119-119: Validation constraints are properly enforced in the UI component.The fontSize component at
packages/effects/layouts/src/widgets/preferences/blocks/theme/font-size.vueimplements the 15-22px range constraints through:
min = 15andmax = 22constants- A watch function that clamps values to stay within bounds
- NumberField component using
:minand:maxpropsThe default value of 16px is appropriate and within the defined range. The validation is complete and correct.
packages/@core/base/design/src/css/global.css (1)
17-19: Original review comment's premise is unsupported by codebase analysis.The review assumes a change from Tailwind's
text-[100%]utility, but searches found no such pattern in the codebase. The currentfont-size: var(--font-size-base, 16px)on line 19 is integrating an already-established CSS variable infrastructure (defined in design-tokens since 16px baseline) with the existing preferences system that dynamically updates--font-size-base. The fallback value matches the system's configured default (fontSize: 16 in config). No breaking changes detected—this enables rather than replaces existing functionality.Likely an incorrect or invalid review comment.
package.json (1)
103-103: Verify the pnpm update intentionality and prepare for breaking changes.The bump from pnpm 10.14.0 to 10.21.0 introduces breaking/semi-breaking changes including lifecycle script blocking and peer-dependency resolution behavior shifts. Confirm whether this update is intentional (especially if unrelated to the font size feature) and ensure:
- Breaking changes are documented in a changeset (per project standards)
- Configuration updates are needed (e.g.,
pnpm.onlyBuiltDependenciesfor dependencies requiring lifecycle scripts)- Dependency resolution behavior changes are accounted for
packages/effects/layouts/src/widgets/preferences/preferences-drawer.vue (1)
46-46: LGTM! Clean integration of the font size control.The FontSize component is properly imported, the model follows the existing naming convention, and it's logically placed in the Appearance tab alongside radius and other theme controls.
Also applies to: 89-89, 333-335
packages/@core/ui-kit/menu-ui/src/components/normal-menu/normal-menu.vue (1)
105-105: LGTM! Consistent variable-based scaling.The font sizes now scale properly with
--font-size-base. The multipliers (1.25 for icons, 0.75 for text) maintain the original proportions while enabling dynamic scaling.Also applies to: 149-149, 156-156
packages/effects/layouts/src/widgets/preferences/blocks/theme/font-size.vue (1)
22-24: Font size range is well-chosen.The 15-22px range provides reasonable flexibility while maintaining readability and usability across the interface.
packages/@core/ui-kit/menu-ui/src/components/menu.vue (1)
447-447: LGTM! Variable-based font sizing properly implemented.The CSS variables enable dynamic font scaling across menu items, icons, and text with appropriate fallbacks and multipliers.
Also applies to: 461-461, 755-755, 763-763
|
This feature is nice to have |
#6908
添加全局字体大小调整,可实时查看效果
Summary by CodeRabbit
New Features
Chores