-
Notifications
You must be signed in to change notification settings - Fork 297
feat(config-provider): config-provider adds theme configuration function #3353
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
WalkthroughThis update introduces customizable theme support and swipeable tabs to the component library. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigProvider
participant TinyThemeTool
User->>ConfigProvider: Set theme prop (object with data)
ConfigProvider->>ConfigProvider: Watch theme prop
alt theme is object with data
ConfigProvider->>TinyThemeTool: changeTheme(theme)
else theme missing data key
ConfigProvider->>User: Console warning (missing data property)
else theme not object
ConfigProvider->>User: Console warning (theme must be object)
end
sequenceDiagram
participant User
participant TabsComponent
User->>TabsComponent: Enable swipeable prop (mobile-first)
TabsComponent->>TabsComponent: Render tabs with swipe gestures enabled
User->>TabsComponent: Swipe tab content area
TabsComponent->>TabsComponent: Switch active tab on swipe
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
examples/sites/demos/apis/tabs.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/apis/config-provider.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/mobile-first/app/tabs/webdoc/tabs.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Walkthrough: This PR introduces a new theme configuration feature to the config-provider component in the Tiny Vue library. It allows users to customize theme colors using a specified format. Additionally, a swipeable feature is added to tabs, enabling sliding content switching. Changes:
|
} | ||
|
||
if (isObject(theme.value) && !hasAnyKey(theme.value, ['data'])) { | ||
console.warn(`configProvider组件的theme属性对象请配置data属性。e.g { data: {'tv-base-color-brand': '#000'}}`) |
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.
The warning message here should be more descriptive to guide the user on how to properly configure the theme
property. Consider providing a more detailed example or linking to documentation.
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
🧹 Nitpick comments (8)
examples/sites/demos/pc/app/config-provider/theme-composition-api.vue (2)
15-17
: Consider initializing theme with default valuesThe initial theme object has an empty data property. Consider initializing it with default values to make the initial state more explicit.
const theme = ref({ - data: {} + data: { + 'tv-base-color-brand': '#191919' + } })
36-41
: Add more documentation for CSS selectorsThe CSS selectors target
.tiny-config-provider
which is applied by the component, but it's not immediately clear to users. Consider adding a comment to explain this relationship.<style scoped> +/* Styles applied to the TinyConfigProvider component for demonstration purposes */ .tiny-config-provider { padding: 1em; border: 1px solid #ccc; border-radius: 1em; } </style>
packages/vue/src/config-provider/src/index.vue (3)
10-32
: Consider moving hasAnyKey to a utility fileThe
hasAnyKey
utility function is well-implemented but would be better placed in a shared utility file since it's a generic function that could be useful elsewhere.Consider moving this function to your utilities and importing it instead:
-/** - * 检查对象是否具有任何一个指定的键 - * @param obj 需要检查的对象 - * @param keys 需要检查的键的数组 - * @return 如果对象具有任何一个指定的键,返回true,否则返回false - */ -const hasAnyKey = (obj: any, keys: string[]): boolean => { - if (obj == null) { - return false - } - - if (keys.length === 0) { - return false - } - - for (const key of keys) { - if (Object.prototype.hasOwnProperty.call(obj, key)) { - return true - } - } - - return false -} +import { hasAnyKey } from '@opentiny/utils'
98-101
: Consider prefetching theme toolCreating a new instance of
TinyThemeTool
on every theme change might be inefficient. Consider creating it once and reusing it.+const themeTool = new TinyThemeTool() hooks.watch( () => theme.value, () => { if (isObject(theme.value) && hasAnyKey(theme.value, ['data'])) { - const themeTool = new TinyThemeTool() themeTool.changeTheme(theme.value) }
103-109
: Enhance warning messages with examplesYour warning messages are helpful, but adding code examples would make them even more useful.
-console.warn(`configProvider组件的theme属性对象请配置data属性。e.g { data: {'tv-base-color-brand': '#000'}}`) +console.warn(`configProvider组件的theme属性对象请配置data属性。例如: { data: {'tv-base-color-brand': '#000'} }`) -console.warn(`configProvider组件的theme属性请配置对象格式数据`) +console.warn(`configProvider组件的theme属性请配置对象格式数据, 例如: { data: { 'tv-base-color-brand': '#000' } }`)examples/sites/demos/pc/app/config-provider/theme.vue (2)
21-23
: Consider initializing theme with default valuesThe initial theme object has an empty data property. Consider initializing it with default values to make the initial state more explicit.
theme: { - data: {} + data: { + 'tv-base-color-brand': '#191919' + } }
46-50
: Add more documentation for CSS selectorsThe CSS selectors target
.tiny-config-provider
which is applied by the component, but it's not immediately clear to users. Consider adding a comment to explain this relationship.<style scoped> +/* Styles applied to the TinyConfigProvider component for demonstration purposes */ .tiny-config-provider { padding: 1em; border: 1px solid #ccc; border-radius: 1em; } </style>
examples/sites/demos/apis/config-provider.js (1)
50-51
: Fix English documentation formatThere's a small format issue in the English description of the theme property.
desc: { 'zh-CN': '自定义主题色,格式:{data:{"tv-base-color-brand":"#595959",....}}', - 'en-US': 'Customized theme color, in {data:"tv-base-color-brand":"#595959",....} format.' + 'en-US': 'Customized theme color, in {data:{"tv-base-color-brand":"#595959",....}} format.' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/theme-saas/src/svgs/go-back.svg
is excluded by!**/*.svg
📒 Files selected for processing (11)
examples/sites/demos/apis/config-provider.js
(1 hunks)examples/sites/demos/apis/tabs.js
(1 hunks)examples/sites/demos/mobile-first/app/tabs/swipeable.vue
(1 hunks)examples/sites/demos/mobile-first/app/tabs/webdoc/tabs.js
(1 hunks)examples/sites/demos/pc/app/config-provider/theme-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/config-provider/theme.vue
(1 hunks)examples/sites/demos/pc/app/config-provider/webdoc/config-provider.js
(1 hunks)packages/utils/src/common/index.ts
(1 hunks)packages/vue/src/config-provider/package.json
(1 hunks)packages/vue/src/config-provider/src/index.vue
(3 hunks)packages/vue/src/config-provider/src/props.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/vue/src/config-provider/src/props.ts
[error] 23-23: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (13)
examples/sites/demos/apis/tabs.js (1)
141-151
: Looks good - new swipeable property well-documentedThe new
swipeable
boolean property is added with clear descriptions in both Chinese and English, consistent default value, and correctly specifies mobile-first mode only. The property documentation follows the existing structure pattern.examples/sites/demos/mobile-first/app/tabs/webdoc/tabs.js (1)
32-44
: New swipeable demo entry properly structuredThe demo entry for the swipeable tabs feature is well-structured with bilingual names and descriptions that accurately explain the feature's requirements (disabled optimization and lazy loading). The code references the correct implementation file.
examples/sites/demos/mobile-first/app/tabs/swipeable.vue (2)
1-34
: Well-implemented swipeable tabs templateThe template correctly implements the swipeable feature by:
- Disabling optimization with
:optimized="false"
as required- Enabling the swipeable mode with the
swipeable
attribute- Creating a good demonstration with horizontally scrollable content in the first tab
The class
select-none
is a good addition to prevent text selection during swipe gestures.
36-50
: Component implementation follows Vue best practicesThe component correctly imports and registers the necessary components from
@opentiny/vue
and manages tab state through reactive data. The implementation is clean and follows Vue best practices.packages/utils/src/common/index.ts (1)
312-334
: Well-implemented utility function for checking object keys.The
hasAnyKey
function provides a useful utility to check if an object contains at least one key from a given array. The implementation correctly handles edge cases:
- Null/undefined objects
- Empty key arrays
- Safe property checking using
Object.prototype.hasOwnProperty.call
The function is also well-documented with clear JSDoc comments explaining its purpose and parameters.
packages/vue/src/config-provider/package.json (1)
18-19
: Appropriate dependency addition for theme functionality.Adding the
@opentiny/utils
dependency is correctly aligned with the new theme customization feature, as it will provide access to the newly addedhasAnyKey
utility function used for theme object validation.examples/sites/demos/pc/app/config-provider/webdoc/config-provider.js (1)
47-57
: Good documentation for the new theme customization feature.The new demo entry for theme customization is well-structured with bilingual support and clear descriptions of the functionality. This properly documents the new
theme
property added to the config-provider component.examples/sites/demos/pc/app/config-provider/theme-composition-api.vue (1)
1-43
: Well-structured composition API example for theme demonstrationThis is a clear demonstration of the new theme configuration feature using Vue's composition API. The code is well-organized with reactive theme management and two theme-switching functions.
packages/vue/src/config-provider/src/index.vue (3)
7-8
: The new imports look goodProperly importing the required dependencies for theme functionality.
71-76
: Theme prop definition looks goodThe theme prop is properly defined as an object with null default.
93-114
: Well-implemented theme functionality with validationsGood job implementing the theme functionality with proper validation and error handling. The warnings provide helpful guidance to users when they provide invalid values.
examples/sites/demos/pc/app/config-provider/theme.vue (1)
1-52
: Well-structured options API example for theme demonstrationThis is a clear demonstration of the new theme configuration feature using Vue's options API. The code is well-organized with a reactive theme object and methods to change themes.
examples/sites/demos/apis/config-provider.js (1)
42-55
: Well-documented theme propertyThe API documentation for the new theme property is comprehensive, including type, default value, stability information, descriptions in multiple languages, and a demo reference.
@@ -20,4 +20,5 @@ export interface ConfigProviderProps { | |||
enable?: boolean | |||
name?: string | |||
} | |||
theme?: Object | null |
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.
🛠️ Refactor suggestion
Avoid using Object
as a type and define a specific interface instead.
Using Object
as a type is overly broad and provides little type safety. Consider creating a specific interface that defines the expected structure of the theme object, which would provide better documentation and type checking.
+ export interface ThemeConfig {
+ data: Record<string, string>;
+ // Add any other expected properties
+ }
export interface ConfigProviderProps {
breakPoints: breakPoint
direction: 'rtl' | 'ltr'
globalPrefix?: string
tag: {
enable?: boolean
name?: string
}
- theme?: Object | null
+ theme?: ThemeConfig | null
}
📝 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.
theme?: Object | null | |
export interface ThemeConfig { | |
data: Record<string, string> | |
// Add any other expected properties | |
} | |
export interface ConfigProviderProps { | |
breakPoints: breakPoint | |
direction: 'rtl' | 'ltr' | |
globalPrefix?: string | |
tag: { | |
enable?: boolean | |
name?: string | |
} | |
theme?: ThemeConfig | null | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
config-provider添加配置主题功能
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
theme
property to the configuration provider component, enabling users to customize theme colors dynamically.theme
property for theme customization.swipeable
property to the tabs component, allowing users to enable swipe-based tab switching on mobile.Documentation