-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
feat: form component IconPicker
#5002
Conversation
|
WalkthroughThe changes in this pull request primarily involve the integration of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
apps/web-antd/src/adapter/component/index.tsOops! Something went wrong! :( ESLint: 9.16.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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
🧹 Outside diff range and nitpick comments (3)
playground/src/views/demos/features/icons/index.vue (1)
77-103
: Consider adding ARIA labels for accessibility.The implementation demonstrates good component reuse and clear UI organization. However, consider enhancing accessibility by adding ARIA labels to the IconPicker components.
-<IconPicker class="w-[200px]" /> +<IconPicker class="w-[200px]" aria-label="Select an icon" /> -<IconPicker class="w-[200px]" prefix="svg" /> +<IconPicker class="w-[200px]" prefix="svg" aria-label="Select an SVG icon" />playground/src/views/examples/form/basic.vue (1)
56-60
: Consider adding validation rules for the icon field.While the IconPicker is correctly integrated into the form schema, consider adding validation rules to ensure a valid icon is selected.
{ component: 'IconPicker', fieldName: 'icon', label: '图标', + rules: 'required', + componentProps: { + placeholder: '请选择图标', + }, },docs/src/components/common-ui/vben-form.md (1)
90-90
: Consider adding IconPicker usage examples in documentation.While the IconPicker is correctly added to the component types and adapter initialization, consider adding a dedicated example section demonstrating its usage in forms.
Add a new example section after the "基础用法" section:
## IconPicker 使用示例 使用 `IconPicker` 组件在表单中选择图标。 <DemoPreview dir="demos/vben-form/icon-picker" />Also applies to: 152-152, 170-170
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (18)
apps/web-antd/src/adapter/component/index.ts
(3 hunks)apps/web-antd/src/locales/langs/en-US/components.json
(1 hunks)apps/web-antd/src/locales/langs/zh-CN/components.json
(1 hunks)apps/web-ele/src/adapter/component/index.ts
(3 hunks)apps/web-ele/src/locales/langs/en-US/components.json
(1 hunks)apps/web-ele/src/locales/langs/zh-CN/components.json
(1 hunks)apps/web-naive/src/adapter/component/index.ts
(3 hunks)apps/web-naive/src/locales/langs/en-US/components.json
(1 hunks)apps/web-naive/src/locales/langs/zh-CN/components.json
(1 hunks)docs/src/components/common-ui/vben-form.md
(3 hunks)packages/effects/common-ui/src/components/icon-picker/icon-picker.vue
(2 hunks)playground/src/adapter/component/index.ts
(3 hunks)playground/src/locales/langs/en-US/components.json
(1 hunks)playground/src/locales/langs/zh-CN/components.json
(1 hunks)playground/src/views/demos/features/icons/icon-picker.vue
(0 hunks)playground/src/views/demos/features/icons/icons.data.ts
(0 hunks)playground/src/views/demos/features/icons/index.vue
(3 hunks)playground/src/views/examples/form/basic.vue
(1 hunks)
💤 Files with no reviewable changes (2)
- playground/src/views/demos/features/icons/icons.data.ts
- playground/src/views/demos/features/icons/icon-picker.vue
✅ Files skipped from review due to trivial changes (8)
- playground/src/locales/langs/en-US/components.json
- apps/web-ele/src/locales/langs/zh-CN/components.json
- playground/src/locales/langs/zh-CN/components.json
- apps/web-antd/src/locales/langs/en-US/components.json
- apps/web-ele/src/locales/langs/en-US/components.json
- apps/web-antd/src/locales/langs/zh-CN/components.json
- apps/web-naive/src/locales/langs/en-US/components.json
- apps/web-naive/src/locales/langs/zh-CN/components.json
🔇 Additional comments (17)
packages/effects/common-ui/src/components/icon-picker/icon-picker.vue (4)
117-117
: Verify that v-model:open
is the correct binding for VbenPopover
Ensure that v-model:open
is the correct prop for two-way binding with VbenPopover
. If the component uses a different prop like v-model:visible
, adjust the binding accordingly.
51-66
: Efficient handling of icon lists with error management
The currentList
computed property efficiently loads icons based on the prefix
prop and includes proper error handling for missing icons.
68-72
: Effective filtering of icons with debounced keyword search
The implementation of the showList
computed property correctly filters icons based on the debounced keyword input, enhancing performance and user experience.
44-45
:
Incorrect use of defineModel
; use defineProps
and defineEmits
for v-model
The defineModel
function is not a standard Vue Composition API method. To properly implement v-model
in Vue 3, you should use defineProps
to define modelValue
and defineEmits
to emit update:modelValue
when the value changes.
Apply this diff to fix the issue:
-const modelValue = defineModel({ default: '', type: String });
+const props = defineProps({
+ modelValue: {
+ type: String,
+ default: '',
+ },
+});
+
+const emit = defineEmits(['update:modelValue']);
Then, update references to modelValue
accordingly:
In line 80:
-currentSelect.value = modelValue.value;
+currentSelect.value = props.modelValue;
In line 92:
-modelValue.value = icon;
+emit('update:modelValue', icon);
Likely invalid or redundant comment.
apps/web-naive/src/adapter/component/index.ts (3)
11-11
: Correctly imported IconPicker
component
The IconPicker
component is correctly imported from @vben/common-ui
.
49-49
: Properly updated ComponentType
to include IconPicker
IconPicker
is appropriately added to the ComponentType
union type.
79-79
: Registered IconPicker
in component adapter
The IconPicker
is correctly registered in the components
object within initComponentAdapter()
.
apps/web-ele/src/adapter/component/index.ts (3)
11-11
: Correctly imported IconPicker
component
The IconPicker
component is correctly imported from @vben/common-ui
.
48-48
: Properly updated ComponentType
to include IconPicker
IconPicker
is appropriately added to the ComponentType
union type.
77-77
: Registered IconPicker
in component adapter
The IconPicker
is correctly registered in the components
object within initComponentAdapter()
.
playground/src/adapter/component/index.ts (3)
11-11
: Correctly imported IconPicker
component
The IconPicker
component is correctly imported from @vben/common-ui
.
57-57
: Properly updated ComponentType
to include IconPicker
IconPicker
is appropriately added to the ComponentType
union type.
91-91
: Registered IconPicker
in component adapter
The IconPicker
is correctly registered in the components
object within initComponentAdapter()
.
apps/web-antd/src/adapter/component/index.ts (3)
11-11
: Correctly imported IconPicker
component
The IconPicker
component is correctly imported from @vben/common-ui
.
57-57
: Properly updated ComponentType
to include IconPicker
IconPicker
is appropriately added to the ComponentType
union type.
91-91
: Registered IconPicker
in component adapter
The IconPicker
is correctly registered in the components
object within initComponentAdapter()
.
playground/src/views/demos/features/icons/index.vue (1)
2-4
: LGTM: Script setup follows Vue 3 best practices.
The implementation correctly uses Vue 3's Composition API with proper imports and reactive state management.
Also applies to: 24-24
Description
将图标选择器抽离成与独立组件,并提供给form使用。 close #4999
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
Release Notes
New Features
IconPicker
component across various modules, enhancing the component library.IconPicker
in both English and Chinese, improving accessibility for users.IconPicker
into forms, allowing users to select icons directly within form fields.Documentation
IconPicker
.Bug Fixes
IconPicker
component for better user experience.