-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
fix: supplement datepicker component #4943
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on the addition of the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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: 0
🧹 Outside diff range and nitpick comments (1)
apps/web-ele/src/adapter/component/index.ts (1)
83-83
: Consider enhancing DatePicker registration to handle edge casesWhile the basic registration addresses the component availability, consider wrapping the DatePicker to handle the reported issues with datetimerange type and name attribute warnings. You might want to add validation or transformation logic similar to how Input and Select components use
withDefaultPlaceholder
.Example enhancement:
- DatePicker: ElDatePicker, + DatePicker: (props, { attrs, slots }) => { + // Handle datetimerange type + if (props?.type === 'datetimerange' && typeof props.name === 'string') { + // Convert string name to array for datetimerange + attrs = { ...attrs, name: [props.name + '_start', props.name + '_end'] }; + } + return h(ElDatePicker, { ...props, ...attrs }, slots); + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/web-ele/src/adapter/component/index.ts
(2 hunks)package.json
(1 hunks)packages/@core/base/shared/package.json
(0 hunks)pnpm-workspace.yaml
(6 hunks)
💤 Files with no reviewable changes (1)
- packages/@core/base/shared/package.json
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (6)
apps/web-ele/src/adapter/component/index.ts (3)
18-18
: LGTM: ElDatePicker import added correctly
The ElDatePicker component is properly imported from element-plus, which is necessary for the component registration.
Line range hint 44-58
: LGTM: DatePicker type properly added to ComponentType
The DatePicker component type is correctly added to the ComponentType union, ensuring proper type checking when using the datepicker in forms.
Line range hint 89-90
: Verify component registration in related files
The DatePicker is now registered in the global shared state. Let's verify if there are any other files that need updates to properly use this component.
✅ Verification successful
Component registration appears to be self-contained
The component registration in the global shared state is part of the Element Plus UI framework adapter initialization. Since:
- No direct DatePicker configurations were found in the form adapters
- The component registration follows the same pattern across different UI framework adapters (web-ele, web-antd, web-naive)
- The setComponents call is part of the framework's initialization process
This suggests the registration is a standard setup step that doesn't require additional file updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for DatePicker usage in form configurations and related files
echo "Searching for DatePicker usage patterns..."
rg -l "type.*['\"]DatePicker['\"]" --type ts
echo "Checking for potential form adapter configurations..."
rg -l "FormComponentType.*DatePicker" --type ts
echo "Looking for existing datepicker implementations that might need updates..."
rg -l "ElDatePicker" --type vue
Length of output: 531
Script:
#!/bin/bash
# Search for DatePicker usage with broader patterns and file types
echo "Searching for DatePicker usage patterns..."
rg -l "DatePicker" --type-add 'frontend:*.{ts,tsx,js,jsx,vue}' --type frontend
echo "Checking component registration patterns..."
rg -l "setComponents.*components" --type-add 'frontend:*.{ts,tsx,js,jsx,vue}' --type frontend
echo "Looking for form configuration files..."
fd -e ts -e js "form" --type f
Length of output: 1728
Script:
#!/bin/bash
# Check the content of form adapter files to understand DatePicker integration
echo "Checking form adapter implementations..."
rg -A 5 "DatePicker" apps/web-ele/src/adapter/form.ts
echo "Checking other UI framework adapters for DatePicker..."
rg -A 5 "DatePicker" apps/web-antd/src/adapter/form.ts apps/web-naive/src/adapter/form.ts
echo "Examining component registration pattern in web-ele..."
rg -A 10 "setComponents" apps/web-ele/src/adapter/component/index.ts
Length of output: 828
pnpm-workspace.yaml (3)
18-22
: Verify tooling updates compatibility.
Several development tools have received updates:
- Build tools (turbo, rollup)
- Linters (eslint plugins, stylelint)
- Testing frameworks (playwright)
- Code formatting (prettier)
While these are mostly patch version updates, please ensure your CI/CD pipeline passes with these changes.
Also applies to: 26-26, 33-34, 46-46, 51-52, 87-87, 90-90, 92-93, 98-98, 109-109, 124-124, 129-129, 132-132, 135-135, 138-138, 141-141, 150-150, 155-155
39-39
: Verify Vue ecosystem updates compatibility.
Several Vue-related packages have been updated:
@tanstack/vue-query
: ^5.60.5 → ^5.61.3@vueuse/core
: ^11.2.0 → ^11.3.0@vueuse/integrations
: ^11.2.0 → ^11.3.0
While these are minor version bumps, please ensure they don't introduce any breaking changes that could affect the datepicker component.
Also applies to: 60-61
✅ Verification successful
Vue ecosystem updates are safe to proceed
Based on the changelogs:
@tanstack/vue-query
v5.61.x changes are primarily bug fixes and improvements to other packages in the monorepo, with no breaking changes affecting Vue Query@vueuse/core
and@vueuse/integrations
v11.3.0 includes only new features and bug fixes, with no breaking changes
The updates are safe and won't impact the datepicker component functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in the updated Vue packages
# Check changelogs for breaking changes
gh api repos/TanStack/query/releases | jq -r '.[] | select(.tag_name | contains("v5.61")) | .body'
gh api repos/vueuse/vueuse/releases | jq -r '.[] | select(.tag_name | contains("v11.3")) | .body'
Length of output: 5130
175-176
: Review UI component library version jumps.
Significant version updates in UI libraries:
vxe-pc-ui
: ^4.2.55 → ^4.3.4vxe-table
: ^4.8.14 → ^4.9.5
These updates include minor version bumps which could potentially affect component behavior.
Description
fixed #4936
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
ElDatePicker
component, enhancing the component library with date selection functionality.Bug Fixes
Chores
pnpm@9.14.2
.