feat: add default placeholder for ApiSelect
#5078
Conversation
|
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 WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
playground/src/adapter/component/index.ts (1)
Line range hint
88-95: Consider using withDefaultPlaceholder HOC for consistencyOther select components in the file use the
withDefaultPlaceholderHOC for handling placeholders. Consider refactoring ApiSelect and ApiTreeSelect to use the same pattern for better consistency and maintainability.Example refactor:
-ApiSelect: (props, { attrs, slots }) => { +ApiSelect: withDefaultPlaceholder((props, { attrs, slots }) => { return h( ApiSelect, { - placeholder: $t('ui.placeholder.select'), ...props, ...attrs, component: Select, loadingSlot: 'suffixIcon', modelPropName: 'value', visibleEvent: 'onVisibleChange', }, slots, ); -}, +}, 'select'),Also applies to: 103-111
apps/web-antd/src/adapter/component/index.ts (1)
Inconsistent dropdown visibility event names found in Ant Design adapter
The verification confirms there is an inconsistency in the event naming within the Ant Design adapter (
apps/web-antd/src/adapter/component/index.ts):
- Line 92: Uses
onDropdownVisibleChange- Line 110: Uses
onVisibleChangeWhile other adapters consistently use
onVisibleChange:
- Playground adapter:
onVisibleChange- Naive UI adapter:
onVisibleChange- Element Plus adapter:
onVisibleChange🔗 Analysis chain
Line range hint
91-91: Verify event name consistency across adaptersThe ApiSelect component uses
onDropdownVisibleChangehere butonVisibleChangein the playground adapter. This inconsistency might cause issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for inconsistent visible event names across adapters echo "Checking visible event names in ApiSelect components:" rg -n "visibleEvent.*['\"]on\w+Change['\"]" apps/*/src/adapter/component/index.ts playground/src/adapter/component/index.tsLength of output: 983
apps/web-ele/src/adapter/component/index.ts (2)
Line range hint
77-85: Consider reordering props to allow placeholder override.The current implementation might prevent users from overriding the default placeholder since it's defined before spreading props. Consider reordering to allow proper prop override.
Here's the suggested change:
return h( ApiSelect, { - placeholder: $t('ui.placeholder.select'), ...props, ...attrs, + placeholder: props?.placeholder ?? $t('ui.placeholder.select'), component: ElSelectV2, loadingSlot: 'loading', visibleEvent: 'onVisibleChange', }, slots, );
Line range hint
91-101: Apply the same placeholder override fix for consistency.For consistency with ApiSelect, the placeholder implementation should follow the same pattern to allow proper prop override.
Here's the suggested change:
return h( ApiSelect, { - placeholder: $t('ui.placeholder.select'), ...props, ...attrs, + placeholder: props?.placeholder ?? $t('ui.placeholder.select'), component: ElTreeSelect, props: { label: 'label', children: 'children' }, nodeKey: 'value', loadingSlot: 'loading', optionsPropName: 'data', visibleEvent: 'onVisibleChange', }, slots, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/web-antd/src/adapter/component/index.ts(2 hunks)apps/web-ele/src/adapter/component/index.ts(2 hunks)apps/web-ele/src/views/demos/form/basic.vue(0 hunks)apps/web-naive/src/adapter/component/index.ts(2 hunks)apps/web-naive/src/views/demos/form/basic.vue(0 hunks)playground/src/adapter/component/index.ts(2 hunks)playground/src/views/examples/form/basic.vue(0 hunks)
💤 Files with no reviewable changes (3)
- playground/src/views/examples/form/basic.vue
- apps/web-naive/src/views/demos/form/basic.vue
- apps/web-ele/src/views/demos/form/basic.vue
🔇 Additional comments (5)
apps/web-antd/src/adapter/component/index.ts (1)
87-87:
Fix props ordering to ensure default placeholder works correctly
Same issue as in playground adapter: placeholder property can be overridden by props.
Apply the same fix as suggested for the playground adapter.
Also applies to: 102-102
apps/web-naive/src/adapter/component/index.ts (2)
Line range hint 89-97: LGTM! Proper handling of Naive UI specific configuration
The implementation correctly handles Naive UI specific props like nodeKey, keyField, and appropriate slot names.
75-75:
Fix props ordering to ensure default placeholder works correctly
Same issue as in other adapters: placeholder property can be overridden by props.
Apply the same fix as suggested for the playground adapter.
Also applies to: 88-88
apps/web-ele/src/adapter/component/index.ts (2)
Line range hint 1-30: Well-structured component adapter implementation!
The adapter pattern implementation is clean and maintainable, with good type safety and consistent component wrapping approach. The withDefaultPlaceholder utility function is particularly well-designed for reuse.
Line range hint 77-101: Verify consistent implementation across adapters.
Let's verify that the placeholder implementation is consistent across all adapter files and that form components are properly handling the new default placeholders.
✅ Verification successful
Placeholder implementation is consistent across all adapter variants
The verification shows that the placeholder implementation using $t('ui.placeholder.select') is consistently implemented across all adapter variants:
- web-antd adapter
- web-ele adapter
- web-naive adapter
- playground adapter
The implementation is uniform for both ApiSelect and ApiTreeSelect components in each adapter, and the components are properly used in form components across different views.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for placeholder implementation across adapter files
echo "Checking placeholder implementation in adapter files..."
rg -A 5 "placeholder.*ui\.placeholder\.select" "apps/*/src/adapter/component/"
# Check form component usage
echo "Checking form component usage..."
rg -A 5 "ApiSelect|ApiTreeSelect" "apps/*/src/views/"
Length of output: 460
Script:
#!/bin/bash
# First, let's find the adapter files
echo "Finding adapter files..."
fd "component" --type d
# Then check for placeholder implementation
echo "Checking placeholder implementation in adapter files..."
rg -A 5 "placeholder.*ui\.placeholder\.select"
# Check form component usage
echo "Checking form component usage..."
rg -A 5 "ApiSelect|ApiTreeSelect"
Length of output: 18218
Description
给
ApiSelect和ApiTreeSelect添加默认的placeholderclose #5076
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
ApiSelectandApiTreeSelectcomponents across multiple applications.ApiSelectandApiTreeSelectcomponents in demo forms, affecting user guidance.