-
Notifications
You must be signed in to change notification settings - Fork 277
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(date-panel): [date-panel,date-picker] DatePicker support using only date panel #2778
Conversation
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 204 files out of 287 files are above the max files limit of 75. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis pull request introduces a comprehensive implementation of a new DatePanel component across multiple files. The changes span from creating demo examples, test cases, and documentation to modifying the core component implementation. The DatePanel provides enhanced date selection functionality with features like custom formatting, shortcuts, disabled dates, read-only mode, and custom week numbering. The implementation covers both PC and mobile-first modes, with extensive configuration options and robust event handling. Changes
Possibly related PRs
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 (
|
Walkthrough: This PR introduces a new feature to support using only the date panel in the Tiny Vue library. It includes the addition of new components, properties, events, and tests to enhance the functionality of the date panel. Changes:
|
examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue
Outdated
Show resolved
Hide resolved
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: 14
🔭 Outside diff range comments (1)
packages/renderless/src/date-panel/index.ts (1)
Line range hint
274-290
: Check for argument mismatch inapi.panelEmit
call withinhandleDatePick
In
handleDatePick
,api.panelEmit
is called as follows:api.panelEmit(state.date, t, props)However, according to the definition,
panelEmit
is defined as:export const panelEmit = ({ state, emit, t, props }) => (value, ...args) => { ... }Passing
t
andprops
as arguments may not match the expected parameters. This could lead to unexpected behavior or errors.Review the parameters passed to
api.panelEmit
to ensure they align with its expected signature. If additional context is needed withinpanelEmit
, consider adjusting its definition or how arguments are passed.
♻️ Duplicate comments (2)
examples/sites/demos/pc/app/date-panel/shortcuts.vue (1)
18-18
:⚠️ Potential issueRemove debug console.info statement.
Remove or replace the console.info statement with proper logging before merging.
examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue (1)
18-18
:⚠️ Potential issueRemove debug console.info statement.
Remove or replace the console.info statement with proper logging before merging.
🧹 Nitpick comments (25)
examples/sites/demos/pc/app/date-panel/format.vue (3)
3-4
: Enhance value display with descriptive labelsThe raw value display could be more user-friendly. Consider adding labels to distinguish between the two formats.
- <div>{{ value }}</div> - <div>{{ value1 }}</div> + <div>Format yyyy-MM-dd: {{ value || 'No date selected' }}</div> + <div>Format yyyy\MM\dd: {{ value1 || 'No date selected' }}</div>
14-15
: Consider more descriptive variable namesThe variable names could better reflect their respective date formats for improved clarity.
-const value = ref('') -const value1 = ref('') +const dateWithDashes = ref('') +const dateWithBackslashes = ref('')
18-22
: Consider responsive designThe fixed width might not work well on smaller screens. Consider using responsive units or media queries.
.demo-date-panel-wrap { - width: 560px; + width: 100%; + max-width: 560px; }examples/sites/demos/pc/app/date-panel/readonly.vue (3)
1-6
: Enhance template with formatting and accessibility improvementsConsider the following improvements:
- Format the displayed date value for better readability
- Use Vue's boolean prop syntax
:readonly="true"
- Add ARIA labels for better accessibility
<template> <div class="demo-date-panel-wrap"> - <div>{{ value }}</div> - <tiny-date-panel v-model="value" readonly></tiny-date-panel> + <div>Selected date: {{ formatDate(value) }}</div> + <tiny-date-panel + v-model="value" + :readonly="true" + aria-label="Read-only date panel" + ></tiny-date-panel> </div> </template>
8-13
: Add type safety and date formatting utilityConsider adding TypeScript support and a date formatting utility for better maintainability and developer experience.
-<script setup> +<script setup lang="ts"> import { ref } from 'vue' import { TinyDatePanel } from '@opentiny/vue' +import { format } from 'date-fns' -const value = ref('2025-01-05') +const value = ref<string>('2025-01-05') + +const formatDate = (date: string): string => { + return format(new Date(date), 'MMMM dd, yyyy') +} </script>
15-19
: Consider responsive design improvementsThe fixed width might not work well on smaller screens. Consider using responsive design patterns.
<style scoped lang="less"> .demo-date-panel-wrap { - width: 560px; + width: 100%; + max-width: 560px; + padding: 0 16px; + box-sizing: border-box; } </style>examples/sites/demos/pc/app/date-panel/basic-usage-composition-api.vue (2)
2-5
: Add ARIA attributes for better accessibilityThe value display and date panel should have proper ARIA attributes for screen readers.
<div class="demo-date-panel-wrap"> - <div>{{ value }}</div> + <div role="status" aria-live="polite">{{ value }}</div> <tiny-date-panel v-model="value"></tiny-date-panel> </div>
12-12
: Consider adding TypeScript type definitionsFor better type safety and developer experience, consider adding type definitions for the reactive value.
-const value = ref('') +const value = ref<string>('')examples/sites/demos/pc/app/date-panel/basic-usage.vue (1)
12-13
: Remove extra newline to fix eslint errorRemove the extra newline to comply with eslint rules.
import { TinyDatePanel } from '@opentiny/vue' - const value = ref('')
🧰 Tools
🪛 eslint
[error] 12-13: Delete
⏎
(prettier/prettier)
examples/sites/demos/pc/app/date-panel/disabled-date-composition-api.vue (1)
12-12
: Add validation for date string formatThe value is initialized with a date string but lacks validation.
-const value = ref('2025-01-02') +const value = ref<string>('2025-01-02') + +const validateDateString = (dateStr: string): boolean => { + const date = new Date(dateStr) + return !isNaN(date.getTime()) +} + +// Validate initial value +if (!validateDateString(value.value)) { + console.warn('Invalid date string format') +}examples/sites/demos/pc/app/date-panel/format-composition-api.vue (2)
14-15
: Initialize date values with valid formatted stringsEmpty strings as initial values may not properly demonstrate the formatting functionality.
Consider initializing with formatted dates:
-const value = ref('') -const value1 = ref('') +const value = ref('2025-01-01') +const value1 = ref('2025\01\01')
4-4
: Document format patterns in the templateThe format patterns could be clearer to users viewing the demo.
Add comments to explain the format patterns:
- <tiny-date-panel v-model="value" format="yyyy-MM-dd"></tiny-date-panel> + <!-- Standard ISO format: 2025-01-01 --> + <tiny-date-panel v-model="value" format="yyyy-MM-dd"></tiny-date-panel> - <tiny-date-panel v-model="value1" format="yyyy\MM\dd"></tiny-date-panel> + <!-- Windows-style format: 2025\01\01 --> + <tiny-date-panel v-model="value1" format="yyyy\MM\dd"></tiny-date-panel>Also applies to: 6-6
examples/sites/demos/pc/app/date-panel/event.vue (2)
3-4
: Consider adding ARIA attributes for better accessibilityThe value display could benefit from proper ARIA labeling to improve screen reader support.
- <div>{{ value }}</div> + <div aria-label="Selected date" role="status">{{ value }}</div>
14-16
: Localize the message stringThe hardcoded Chinese message should be moved to a localization system for better internationalization support.
Consider using a translation system:
- TinyModal.message({ message: '触发 面板选中 事件,组件绑定值为:' + value, status: 'info' }) + TinyModal.message({ message: t('datepanel.event.selectionMessage', { value }), status: 'info' })examples/sites/demos/pc/app/date-panel/event.spec.ts (2)
3-5
: Add test description in EnglishConsider using English for test descriptions to maintain consistency with international development practices.
-test('[DatePanel] 测试事件', async ({ page }) => { +test('[DatePanel] Test event handling', async ({ page }) => {
7-15
: Enhance test robustness and readabilityThe test could benefit from several improvements:
- Add test data isolation
- Use test utilities for common operations
- Add more assertions for component state
Consider refactoring to:
const setupDatePanel = async (page) => { await page.goto('date-panel#event') return { selectYear: async (year) => { await page.getByRole('button', { name: `${year} 年` }).click() await expect(page.getByRole('cell', { name: year })).toBeVisible() }, // ... other utility functions } } test('[DatePanel] Test event handling', async ({ page }) => { const datePanel = await setupDatePanel(page) await datePanel.selectYear('2025') // ... rest of the test using utility functions })examples/sites/demos/pc/app/date-panel/custom-week.spec.ts (1)
3-5
: Add test description in EnglishSimilar to the event test, consider using English for test descriptions.
-test('[DatePanel] 测试周次序号', async ({ page }) => { +test('[DatePanel] Test week number display', async ({ page }) => {examples/sites/demos/pc/app/date-panel/custom-weeks-composition-api.vue (1)
20-23
: Add documentation and type safety to formatWeeks function.The
formatWeeks
function would benefit from TypeScript types and JSDoc documentation explaining its parameters and return value.-const formatWeeks = (customWeeks, weekFirstDays) => { +/** + * Formats the week number and stores the first day of each week + * @param {number} customWeeks - The week number to format + * @param {Date[]} weekFirstDays - Array of dates representing first days of weeks + * @returns {string} Formatted week string with 'w' suffix + */ +const formatWeeks = (customWeeks: number, weekFirstDays: Date[]): string => { eachWeekFirstDay.value = weekFirstDays return customWeeks + 'w' }examples/sites/demos/pc/app/date-panel/basic-usage.spec.ts (1)
3-22
: Well-structured test with clear steps and assertions.The test effectively covers the basic date selection flow. Consider adding test cases for:
- Edge cases (e.g., month/year transitions)
- Keyboard navigation
- Accessibility attributes
examples/sites/demos/pc/app/date-panel/shortcuts.vue (1)
25-27
: Simplify date calculations using Date methods.The date calculations can be simplified using Date's built-in methods.
-date.setTime(date.getTime() - 3600 * 1000 * 24) +date.setDate(date.getDate() - 1) -date.setTime(date.getTime() - 3600 * 1000 * 24 * 7) +date.setDate(date.getDate() - 7)Also applies to: 33-35
examples/sites/demos/pc/app/date-panel/webdoc/date-panel.js (1)
Line range hint
115-118
: Add missing English descriptions for format options.The format options are missing English descriptions. Please add translations for better internationalization support.
For example:
name: 'a', desc: { 'zh-CN': 'am/pm', - 'en-US': '' + 'en-US': 'am/pm (12-hour clock)' }Also applies to: 124-127, 134-136, 142-144, 150-152, 158-160, 166-168, 174-176, 182-184
packages/vue/src/picker/src/pc.vue (1)
141-141
: Consider performance implications of v-show.Using
v-show
instead ofv-bind:visible
keeps the picker panel in the DOM, which might impact initial render time. Consider usingv-if
if the picker is rarely opened or the panel is complex.examples/sites/demos/pc/app/date-panel/custom-weeks.vue (2)
20-23
: Enhance theformatWeeks
function with input validation.The function should validate its inputs and handle edge cases.
Apply this diff to improve the function:
-const formatWeeks = (customWeeks, weekFirstDays) => { - eachWeekFirstDay.value = weekFirstDays - return customWeeks + 'w' -} +const formatWeeks = (customWeeks, weekFirstDays) => { + if (!Array.isArray(weekFirstDays)) { + console.warn('weekFirstDays should be an array') + return customWeeks + } + eachWeekFirstDay.value = weekFirstDays + return typeof customWeeks === 'number' ? `${customWeeks}w` : customWeeks +}
4-10
: Consider adding documentation for the component props.Add comments to explain the purpose of each prop and its expected values.
Add the following comments above the component:
+<!-- + @property {Function} format-weeks - Function to format week numbers + @property {Boolean} show-week-number - Whether to display week numbers + @property {Number} first-day-of-week - First day of the week (1 for Monday) +--> <tiny-date-panel ref="datePanel" v-model="value" :format-weeks="formatWeeks" :show-week-number="true" :first-day-of-week="1" ></tiny-date-panel>packages/theme/src/date-panel/index.less (1)
31-34
: Consider removing redundant position style.The
.has-time
modifier setsposition: relative
on the body wrapper, but this property is already defined in the base class. Consider removing the redundant style:&.has-time { - .@{picker-panel-prefix-cls}__body-wrapper { - position: relative; - } }Also applies to: 44-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
examples/sites/demos/apis/date-panel.js
(1 hunks)examples/sites/demos/pc/app/date-panel/basic-usage-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/basic-usage.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/basic-usage.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/custom-week.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/custom-weeks-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/custom-weeks.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/event-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/event.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/event.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/format-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/format.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/format.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/webdoc/date-panel.cn.md
(1 hunks)examples/sites/demos/pc/app/date-panel/webdoc/date-panel.en.md
(1 hunks)examples/sites/demos/pc/app/date-panel/webdoc/date-panel.js
(1 hunks)examples/sites/demos/pc/menus.js
(1 hunks)packages/renderless/src/date-panel/index.ts
(4 hunks)packages/renderless/src/date-panel/vue.ts
(8 hunks)packages/theme/src/date-panel/index.less
(1 hunks)packages/vue/src/date-panel/src/index.ts
(1 hunks)packages/vue/src/date-panel/src/pc.vue
(4 hunks)packages/vue/src/picker/src/pc.vue
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/sites/demos/pc/app/date-panel/webdoc/date-panel.cn.md
- examples/sites/demos/pc/app/date-panel/webdoc/date-panel.en.md
🧰 Additional context used
🪛 GitHub Actions: E2E Test PR--feat(date-panel): [date-panel] Support using only date panel
examples/sites/demos/pc/app/date-panel/format.spec.ts
[error] 8-8: Test failure: Timed out waiting for element with text '-01-15' to be visible after 8000ms
[warning] Slow test file: Took 24.0s to execute
examples/sites/demos/pc/app/date-panel/readonly.spec.ts
[error] 13-13: Test failure: CSS property 'background-color' expected to be '#1476ff' but received 'rgb(20, 118, 255)'
[warning] Slow test file: Took 24.3s to execute
🪛 eslint
examples/sites/demos/pc/app/date-panel/basic-usage.vue
[error] 12-13: Delete ⏎
(prettier/prettier)
🔇 Additional comments (14)
examples/sites/demos/pc/app/date-panel/format.vue (2)
10-12
: LGTM!The script setup and imports follow Vue 3 best practices.
5-6
: Verify backslash format handlingThe second DatePanel uses backslashes in the format string. Ensure that the component properly handles escaped characters in format strings.
✅ Verification successful
Backslash format is intentionally demonstrated
The date-panel component uses the fecha library for date formatting, where backslashes in format strings are valid separators. The example demonstrates two common date separator styles:
- Standard dash separator (yyyy-MM-dd)
- Backslash separator (yyyy\MM\dd)
Both formats are valid and intentionally shown as usage examples.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other format string implementations to verify consistency rg -l "format=['\"].*\\\\.*['\"]" --type vueLength of output: 76
Script:
#!/bin/bash # Find DatePanel component implementation fd "date-panel" --type f # Search for format patterns with backslashes rg "format=['\"].*\\.*['\"]" # Search for format property definition in component rg "format.*prop" -A 5Length of output: 35491
Script:
#!/bin/bash # Find date-panel format implementation ast-grep --pattern 'format.*=.*props.format' # Search for date format handling in date-panel files rg -A 5 "formatDate.*format" packages/renderless/src/date-panel/Length of output: 573
Script:
#!/bin/bash # Search for formatDate function implementation rg -A 10 "export const formatDate = " packages/renderless/src/ # Look for date format utility files fd "date|format" --type f packages/renderless/src/commonLength of output: 1143
examples/sites/demos/pc/app/date-panel/readonly.vue (1)
1-19
: Verify date panel functionalityThe implementation aligns with the PR objectives of using the date panel independently. Please verify:
- The date panel displays correctly in read-only mode
- The value updates are properly reflected in the UI
- The component works without any time-related dependencies
✅ Verification successful
Implementation verified as correct
The date panel component is properly isolated and implements the read-only functionality correctly. The implementation shows no unwanted coupling with time-related components, maintaining clean separation of concerns. The example demonstrates the intended standalone usage of the date panel component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the date panel is not dependent on time components rg -l "time" examples/sites/demos/pc/app/date-panel/Length of output: 52
Script:
#!/bin/bash # Check the date panel component implementation ast-grep --pattern 'export const TinyDatePanel = $_' # Look for any potential datetime coupling rg -l "datetime" examples/sites/demos/pc/app/date-panel/ # Check the component's props and structure ast-grep --pattern 'const props = $_'Length of output: 2556
Script:
#!/bin/bash # Find date panel component files fd -t f "date-panel" -x echo '{}' # Look for date panel implementation rg -l "DatePanel" --type vue --type ts --type js # Check for any date-related props or configuration ast-grep --pattern 'props: { $$$ }'Length of output: 474
packages/renderless/src/date-panel/index.ts (2)
168-176
: Well-implementedpanelEmit
function for improved event handlingThe new
panelEmit
function effectively updates the state and emits the required events, streamlining the event emission process and enhancing code modularity.
217-226
: Ensure consistent handling ofprops.shortcuts
inhandleShortcutClick
In the
$emit
function withinpicker
, the conditionif (props.shortcuts?.length)
is used to choose betweenapi.handleDatePick
andapi.doPick
. To prevent potential issues whenprops.shortcuts
is undefined or empty, ensure this logic handles all cases correctly.Consider verifying the behavior when
props.shortcuts
is undefined, null, or an empty array, and adjust the condition if necessary.examples/sites/demos/pc/app/date-panel/readonly-composition-api.vue (1)
1-19
: Component correctly demonstratesreadonly
usage with appropriate bindingsThe component is well-structured, effectively demonstrating how to use the
tiny-date-panel
component in read-only mode with the Composition API. Thevalue
is properly initialized and bound to the date panel.examples/sites/demos/pc/app/date-panel/disabled-date.vue (1)
12-15
: Consider adding validation for the initial valueThe initial value '2025-01-02' is valid according to the
disabledDate
function, but there's no validation to ensure future initial values will respect the disabled date constraints.Consider adding validation or documenting that initial values should be validated against
disabledDate
:const value = ref('2025-01-02') const disabledDate = (val) => { return val.getFullYear() < 2025 } +// Validate initial value +const validateInitialValue = () => { + const date = new Date(value.value) + if (disabledDate(date)) { + console.warn('Initial value does not respect disabled date constraints') + } +} +validateInitialValue()examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue (1)
25-27
: Simplify date calculations using Date methods.The date calculations can be simplified using Date's built-in methods.
-date.setTime(date.getTime() - 3600 * 1000 * 24) +date.setDate(date.getDate() - 1) -date.setTime(date.getTime() - 3600 * 1000 * 24 * 7) +date.setDate(date.getDate() - 7)Also applies to: 33-35
packages/vue/src/picker/src/pc.vue (1)
Line range hint
4-6
: Verify test coverage for new props.Please ensure test coverage exists for the new props (
firstDayOfWeek
,nowClick
,modelValue
, etc.) in the DatePanel component.packages/renderless/src/date-panel/vue.ts (3)
Line range hint
91-144
: LGTM! Props are properly used for state initialization.The function correctly initializes state properties using props with appropriate fallback values, ensuring safe initialization.
145-166
: LGTM! Watchers are properly implemented.The watchers for
props.modelValue
andprops.format
are correctly set up with immediate execution and proper state updates.
Line range hint
213-245
: LGTM! API methods are properly initialized with props.The API methods are correctly initialized with props, ensuring proper access to component properties.
examples/sites/demos/pc/menus.js (1)
133-133
: LGTM! Menu item is properly added.The DatePanel menu item is correctly placed in the Form section with consistent naming.
packages/theme/src/date-panel/index.less (1)
31-34
: LGTM! Clean implementation of the body wrapper styles.The new
.@{picker-panel-prefix-cls}__body-wrapper
class follows good practices:
- Uses correct BEM naming convention
- Leverages CSS custom properties for theming
- Maintains consistency with existing styles
examples/sites/demos/pc/app/date-panel/disabled-date-composition-api.vue
Show resolved
Hide resolved
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: 14
🔭 Outside diff range comments (1)
packages/renderless/src/date-panel/index.ts (1)
Line range hint
274-290
: Add null check forvalue
inhandleDatePick
In the
handleDatePick
function, there is no check to ensure thatvalue
is a valid date object before proceeding. This could lead to runtime errors ifvalue
is null or undefined. Add a check to confirmvalue
is valid.Apply this diff to add a null check:
export const handleDatePick = ({ api, state, t, props }) => (value) => { if (props.readonly) { return } + if (!value || !isDate(value)) { + console.warn('Invalid date value:', value) + return + } if (state.selectionMode === DATEPICKER.Day) { let newDate = state.value
🧹 Nitpick comments (45)
examples/sites/demos/pc/app/date-panel/readonly.vue (3)
2-5
: Enhance accessibility and user experience.Consider the following improvements:
- Add ARIA labels for better screen reader support
- Format the displayed date value for better readability
- Add loading and error states handling
Here's a suggested implementation:
<div class="demo-date-panel-wrap"> - <div>{{ value }}</div> + <div role="status" aria-label="Selected date"> + {{ new Date(value).toLocaleDateString() }} + </div> <tiny-date-panel v-model="value" readonly></tiny-date-panel> </div>
8-13
: Add TypeScript support and input validation.The current implementation could benefit from type safety and validation:
- Consider using TypeScript for better type safety
- Add validation for the date value
- Use current date as default instead of a hardcoded future date
Here's a suggested implementation:
-<script setup> +<script setup lang="ts"> import { ref } from 'vue' import { TinyDatePanel } from '@opentiny/vue' -const value = ref('2025-01-05') +const value = ref<string>(new Date().toISOString().split('T')[0])
15-19
: Improve responsive design support.The current fixed width might not work well on different screen sizes. Consider:
- Using relative units or max-width
- Adding media queries for mobile devices
- Supporting RTL layouts
Here's a suggested implementation:
<style scoped lang="less"> .demo-date-panel-wrap { - width: 560px; + max-width: 560px; + width: 100%; + + @media (max-width: 768px) { + max-width: 100%; + } } </style>examples/sites/demos/pc/app/date-panel/format.vue (5)
3-4
: Add ARIA labels for better accessibility.The displayed date values should have appropriate ARIA labels to improve screen reader support.
- <div>{{ value }}</div> - <div>{{ value1 }}</div> + <div aria-label="Formatted date (yyyy-MM-dd)">{{ value }}</div> + <div aria-label="Formatted date (yyyy\MM\dd)">{{ value1 }}</div>
5-6
: Document the backslash format usage.The second date panel uses backslashes in the format. Consider adding a comment to explain if this is intended for Windows-style dates or has a specific purpose.
<tiny-date-panel v-model="value" format="yyyy-MM-dd"></tiny-date-panel> + <!-- Windows-style date format with backslashes --> <tiny-date-panel v-model="value1" format="yyyy\MM\dd"></tiny-date-panel>
10-13
: Consider adopting TypeScript for better type safety.Adding TypeScript would provide better type checking for the component props and reactive values.
-<script setup> +<script setup lang="ts"> import { ref } from 'vue' -import { TinyDatePanel } from '@opentiny/vue' +import type { Ref } from 'vue' +import { TinyDatePanel } from '@opentiny/vue'
14-15
: Enhance reactive state initialization and typing.Consider initializing with today's date and adding type annotations for better maintainability.
-const value = ref('') -const value1 = ref('') +const value: Ref<string> = ref(new Date().toISOString().split('T')[0]) +const value1: Ref<string> = ref(new Date().toISOString().split('T')[0])
19-21
: Improve responsive design.Consider using relative units and media queries for better responsiveness across different screen sizes.
.demo-date-panel-wrap { - width: 560px; + width: 100%; + max-width: 560px; + margin: 0 auto; }examples/sites/demos/pc/app/date-panel/event-composition-api.vue (3)
3-3
: Consider formatting the displayed date valueThe raw value display might not be user-friendly. Consider formatting the date using a date formatting library or Vue filter for better readability.
- <div>{{ value }}</div> + <div>{{ value ? new Date(value).toLocaleDateString() : 'No date selected' }}</div>
14-16
: Improve event handler implementationTwo suggestions for improvement:
- Consider internationalizing the message text for better global accessibility
- Avoid parameter shadowing with the reactive reference name
-const handleSelectChange = (value) => { - TinyModal.message({ message: '触发 面板选中 事件,组件绑定值为:' + value, status: 'info' }) +const handleSelectChange = (selectedDate) => { + TinyModal.message({ message: `Panel selection changed. Selected value: ${selectedDate}`, status: 'info' })
20-22
: Consider using responsive widthA fixed width of 560px might not be optimal for all screen sizes. Consider using relative units or media queries for better responsiveness.
.demo-date-panel-wrap { - width: 560px; + width: 100%; + max-width: 560px;examples/sites/demos/pc/app/date-panel/format.spec.ts (1)
1-12
: Optimize test performanceThe test file is reported as slow (24.1s). Consider these optimizations:
- Use
test.describe.parallel()
to run tests in parallel- Minimize page navigations
- Use more specific selectors instead of nth-based selection
import { test, expect } from '@playwright/test' +test.describe.parallel('DatePanel Format Tests', () => { + test.beforeAll(async ({ browser }) => { + // Set up shared context + const context = await browser.newContext() + const page = await context.newPage() + await page.goto('date-panel#format') + return { page } + }) + test('[DatePanel] 测试格式化', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) - await page.goto('date-panel#format') await page.getByText('15', { exact: true }).nth(1).click() await expect(page.getByText('-01-15')).toBeVisible() await page.getByText('8', { exact: true }).nth(2).click() await expect(page.getByText('\\01\\08')).toBeVisible() }) +})🧰 Tools
🪛 GitHub Actions: E2E Test PR--feat(date-panel): [date-panel] Support using only date panel
[error] 8-8: Test failure: Timed out waiting for element with text '-01-15' to be visible after 8000ms
[warning] Slow test file: Took 24.1s to execute
examples/sites/demos/pc/app/date-panel/readonly-composition-api.vue (4)
3-4
: Enhance date display formatting and accessibility.The raw date value display could be improved with proper formatting for better user experience.
Consider applying these improvements:
- <div>{{ value }}</div> + <div role="status" aria-label="Selected date"> + {{ new Date(value).toLocaleDateString() }} + </div>
4-4
: Make boolean prop more explicit.The readonly prop using shorthand might not be immediately clear to all developers.
Consider being more explicit:
- <tiny-date-panel v-model="value" readonly></tiny-date-panel> + <tiny-date-panel v-model="value" :readonly="true"></tiny-date-panel>
16-18
: Consider responsive design.The fixed width of 560px might not be optimal for all viewport sizes.
Consider using more responsive styling:
.demo-date-panel-wrap { - width: 560px; + max-width: 560px; + width: 100%; }
8-13
: Add component documentation.As this is a demo component, it would be helpful to add comments explaining the purpose and usage.
Consider adding documentation:
<script setup> import { ref } from 'vue' import { TinyDatePanel } from '@opentiny/vue' + /** + * Demo component showcasing TinyDatePanel in readonly mode + * This example demonstrates: + * - Basic date panel usage + * - Readonly mode implementation + * - Two-way binding with v-model + */ const value = ref('2025-01-05') </script>examples/sites/demos/pc/app/date-panel/format-composition-api.vue (4)
3-4
: Add ARIA labels for better accessibility.The div displaying the selected value should have appropriate ARIA attributes for screen readers.
- <div>{{ value }}</div> + <div aria-live="polite" aria-label="Selected date">{{ value }}</div>
5-6
: Consider using forward slashes in the format string.The format string "yyyy\MM\dd" uses backslashes which might be confusing. Consider using forward slashes for better readability and consistency with common date formats.
- <tiny-date-panel v-model="value1" format="yyyy\MM\dd"></tiny-date-panel> + <tiny-date-panel v-model="value1" format="yyyy/MM/dd"></tiny-date-panel>
14-15
: Consider adding TypeScript type annotations and default values.For better type safety and initial state, consider adding type annotations and default formatted dates.
-const value = ref('') -const value1 = ref('') +const value = ref<string>(new Date().toISOString().split('T')[0]) +const value1 = ref<string>(new Date().toLocaleDateString().replace(/\//g, '\\'))
19-24
: Consider making the layout more responsive.The fixed width of 560px might not work well on smaller screens. Consider using responsive units and media queries.
.demo-date-panel-wrap { - width: 560px; + width: 100%; + max-width: 560px; .label { margin-top: 18px; } }packages/renderless/src/date-panel/index.ts (2)
168-176
: RefactorpanelEmit
to reuse existing emit logicThe
panelEmit
function duplicates some functionality ofcusEmit
. Consider refactoringpanelEmit
to callcusEmit
internally or to abstract common code into a shared helper function to avoid code duplication and maintain consistency.
168-176
: Potential redundancy betweencusEmit
andpanelEmit
Both
cusEmit
andpanelEmit
handle emitting events after date selection. Review whether both functions are necessary or if their logic can be consolidated to simplify the codebase.Also applies to: 274-290
examples/sites/demos/pc/app/date-panel/basic-usage-composition-api.vue (3)
4-4
: Bindvalue
usingmodelValue
for clarityWhile using
v-model
is acceptable, explicitly using:modelValue
and@update:modelValue
can enhance clarity, especially when dealing with multiple bindings.- <tiny-date-panel v-model="value"></tiny-date-panel> + <tiny-date-panel :modelValue="value" @update:modelValue="value = $event"></tiny-date-panel>
12-12
: Initializevalue
with a meaningful defaultCurrently,
value
is initialized as an empty string. Consider initializing it with the current date or a specific default date to improve user experience.const value = ref('') +// Initialize with the current date +value.value = new Date()
16-23
: Ensure styles are encapsulated and adaptiveThe scoped styles set a fixed width of 560px, which may not be responsive on smaller screens. Consider using relative units or media queries to enhance responsiveness.
.demo-date-panel-wrap { - width: 560px; + max-width: 100%; + width: 560px;examples/sites/demos/pc/app/date-panel/basic-usage.vue (3)
3-4
: Consider adding validation and formatting for the displayed valueThe raw value display might not be user-friendly. Consider adding validation and proper date formatting.
- <div>{{ value }}</div> + <div>Selected date: {{ value ? new Date(value).toLocaleDateString() : 'No date selected' }}</div>
12-13
: Remove extra blank lineThere's an unnecessary blank line that should be removed as per the linting rules.
🧰 Tools
🪛 eslint
[error] 12-13: Delete
⏎
(prettier/prettier)
13-13
: Consider adding type definition for the value refAdding TypeScript type definitions would improve code maintainability and IDE support.
-const value = ref('') +const value = ref<string>('')🧰 Tools
🪛 eslint
[error] 12-13: Delete
⏎
(prettier/prettier)
examples/sites/demos/pc/app/date-panel/disabled-date-composition-api.vue (1)
13-15
: Improve type safety and maintainability of the disabled date logicConsider these improvements:
- Add TypeScript type for the parameter
- Extract the year as a constant
- Add JSDoc documentation
+const MINIMUM_ALLOWED_YEAR = 2025 + +/** + * Determines if a date should be disabled + * @param {Date} val - The date to check + * @returns {boolean} True if the date should be disabled + */ -const disabledDate = (val) => { - return val.getFullYear() < 2025 +const disabledDate = (val: Date): boolean => { + return val.getFullYear() < MINIMUM_ALLOWED_YEAR }examples/sites/demos/pc/app/date-panel/disabled-date.spec.ts (1)
3-3
: Consider using English for test descriptionsFor better international collaboration, consider using English in test descriptions.
-test('[DatePanel] 测试时间禁用', async ({ page }) => { +test('[DatePanel] test date disabling functionality', async ({ page }) => {examples/sites/demos/pc/app/date-panel/readonly.spec.ts (1)
3-14
: Optimize test performance.The test is taking 24s to execute, which is significantly slow. Consider:
- Using
page.waitForSelector()
instead of implicit waits- Combining related assertions
- Reducing navigation overhead
test('[DatePanel] 测试只读', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('date-panel#readonly') + + // Wait for the date panel to be ready + await page.waitForSelector('text=21') + await page.getByText('21', { exact: true }).click() - await expect(page.getByText('-01-05')).toBeVisible() - await page.getByLabel('上个月').click() await page.getByText('18').click() await page.getByLabel('下个月').click() - await expect(page.getByText('-01-05')).toBeVisible() - await expect(page.getByText('5', { exact: true }).first()).toHaveCSS('background-color', '#1476ff') + + // Combine assertions + await Promise.all([ + expect(page.getByText('-01-05')).toBeVisible(), + expect(page.getByText('5', { exact: true }).first()).toHaveCSS('background-color', 'rgb(20, 118, 255)') + ]) })🧰 Tools
🪛 GitHub Actions: E2E Test PR--feat(date-panel): [date-panel] Support using only date panel
[error] 13-13: Test failure: CSS property 'background-color' value mismatch. Expected '#1476ff' but received 'rgb(20, 118, 255)'
[warning] Slow test file: Took 24.0s to execute
examples/sites/demos/pc/app/date-panel/event.spec.ts (1)
3-16
: Enhance test robustness and coverage.
- Hard-coded text assertions may be fragile against UI changes
- Missing error scenarios (e.g., invalid selections)
Consider:
- Using data-testid attributes for more stable selectors
- Adding test cases for error scenarios
test('[DatePanel] 测试事件', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('date-panel#event') + // Test invalid selection + await test.step('invalid selection', async () => { + // Add test for invalid date selection + }) + + // Test valid selection + await test.step('valid selection', async () => { await page.getByRole('button', { name: '2025 年' }).click() await expect(page.getByRole('cell', { name: '2025' })).toBeVisible() await page.getByText('2025').click() await expect(page.getByRole('cell', { name: '一月', exact: true })).toBeVisible() await page.getByText('六月').click() await page.getByRole('cell', { name: '11' }).locator('div').click() - await expect(page.getByText('触发 面板选中 事件,组件绑定值为:2025-06-')).toBeVisible() + await expect(page.getByTestId('event-message')).toHaveText('触发 面板选中 事件,组件绑定值为:2025-06-') + }) })examples/sites/demos/pc/app/date-panel/custom-week.spec.ts (1)
7-11
: Improve test organization and performance.
- Test is slow (16.5s execution time)
- Comments should be in English for international collaboration
- Missing verification of week calculation logic
test('[DatePanel] 测试周次序号', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('date-panel#custom-weeks') - // 选择年份月份日期 + // Select year and month + await test.step('year and month selection', async () => { await page.getByRole('button', { name: '2025 年' }).click() await page.getByText('2026').click() await page.getByText('三月').click() await expect(page.getByText('12w')).toBeVisible() + }) - // 选择下个月 + // Navigate to next month + await test.step('month navigation', async () => { await page.getByLabel('下个月').click() await expect(page.getByText('17w')).toBeVisible() + }) - // 选择日期 + // Select date + await test.step('date selection', async () => { await page.getByText('23', { exact: true }).click() await expect(page.getByText('-04-23')).toBeVisible() + + // Verify week calculation + const weekNumber = await page.evaluate(() => { + // Add week calculation verification logic + }) + expect(weekNumber).toBe(17) + }) })Also applies to: 13-15, 17-19
🧰 Tools
🪛 GitHub Actions: E2E Test PR--feat(date-panel): [date-panel] Support using only date panel
[warning] Slow test file: Took 16.5s to execute
examples/sites/demos/pc/app/date-panel/custom-weeks-composition-api.vue (2)
14-24
: Enhance type safety and documentation.
- Add TypeScript type definitions
- Remove unused
datePanel
ref- Add JSDoc documentation
<script setup> import { ref } from 'vue' import { TinyDatePanel } from '@opentiny/vue' +import type { WeekFirstDay } from '@opentiny/vue' -const value = ref('') -const eachWeekFirstDay = ref([]) +const value = ref<string>('') +const eachWeekFirstDay = ref<WeekFirstDay[]>([]) + +/** + * Formats the week number by appending 'w' and stores the first day of each week + * @param {number} customWeeks - The week number to format + * @param {WeekFirstDay[]} weekFirstDays - Array of first days for each week + * @returns {string} Formatted week number + */ const formatWeeks = (customWeeks, weekFirstDays) => { eachWeekFirstDay.value = weekFirstDays return customWeeks + 'w' } </script>
4-10
: Remove unused ref attribute.The
ref="datePanel"
attribute is not being used in the component.<tiny-date-panel - ref="datePanel" v-model="value" :format-weeks="formatWeeks" :show-week-number="true" :first-day-of-week="1" ></tiny-date-panel>
examples/sites/demos/pc/app/date-panel/basic-usage.spec.ts (1)
3-22
: Test implementation looks good but execution is slowThe test comprehensively covers the date selection workflow using role-based selectors, which is great for accessibility. However, the test execution time (15.9s) is concerning.
Consider optimizing the test by:
- Reducing wait times between actions
- Using
waitFor
with appropriate timeout values instead of relying on default timeouts- Combining related assertions where possible
Also, consider internationalizing the Chinese comments for better maintainability in a global context.
🧰 Tools
🪛 GitHub Actions: E2E Test PR--feat(date-panel): [date-panel] Support using only date panel
[warning] Slow test file: Took 15.9s to execute
examples/sites/demos/pc/app/date-panel/webdoc/date-panel.js (2)
12-13
: Format HTML tags in descriptions consistently.The HTML tags in descriptions could be better formatted for consistency and readability:
- Line 12: Single-line HTML without proper spacing
- Lines 24-26: Multi-line HTML with inconsistent indentation
- Line 50: Duplicate paragraph tags
Apply these formatting improvements:
- 'zh-CN': '<p>支持只使用日期面板、日期区间面板。</p>', + 'zh-CN': '<p>支持只使用日期面板、日期区间面板。</p>', - 'zh-CN': '<p>通过设置 <code>disabledDate</code> 属性,可以实现部分禁用,此时只能选择一部分日期。</p>', + 'zh-CN': '<p>通过设置 <code>disabledDate</code> 属性,可以实现部分禁用,此时只能选择一部分日期。</p>', - 'en-US': '<p><p>You can set shortcut options by specifying the <code>shortcuts</code> array</ p></p>' + 'en-US': '<p>You can set shortcut options by specifying the <code>shortcuts</code> array</p>'Also applies to: 24-27, 49-51
62-63
: Add missing English description.The English description for the readonly demo is empty.
Add the missing translation:
- 'en-US': '' + 'en-US': '<p>Set readonly mode through the <code>readonly</code> property.</p>'examples/sites/demos/apis/date-panel.js (2)
115-118
: Add missing English translations for date format descriptions.Several format options lack English translations, which is important for international developers.
Add English translations for all format options. For example:
- 'en-US': '' + 'en-US': 'am/pm'Also applies to: 124-127, 134-136, 142-144, 150-152, 158-160, 166-168, 174-176, 182-184
195-210
: Improve theIPickerOptions
interface documentation.The interface could benefit from better documentation and type safety:
- Comments should be in English for consistency
- The
shortcuts
array type could be more specificConsider this improvement:
interface IPickerOptions { - // 每周的第一天是星期几,默认值是7,也就是星期天 + /** First day of the week, 7 represents Sunday (default) */ firstDayOfWeek: number - // 实现部分禁用,此时只能选择一部分日期 + /** Callback function to disable specific dates */ disabledDate: (time: Date) => boolean - // 选中日期后执行的回调,需要与 daterange 或 datetimerange 类型配合使用才生效 + /** Callback after date selection (only works with daterange/datetimerange) */ onPick: (range: { minDate: Date, maxDate: Date }) => void - // 快捷选项 + /** Shortcut options for quick date selection */ shortcuts: Array<{ text: string onClick: (picker: { $emit: (type: string, date: Date) => void }) => void type: 'startFrom' | 'EndAt' startDate: Date endDate: Date }> }packages/renderless/src/date-panel/vue.ts (1)
91-106
: Initialize state properties with proper type checking.The state initialization could be more robust with type checking and default values.
Consider adding type checks:
const initState = ({ reactive, computed, api, i18n, designConfig, props }) => { const state = reactive({ - popperClass: props.popperClass || '', + popperClass: typeof props.popperClass === 'string' ? props.popperClass : '', date: new Date(), value: '', defaultValue: null, defaultTime: null, - showTime: props.type === 'datetimerange' || false, + showTime: Boolean(props.type === 'datetimerange'), selectionMode: DATEPICKER.Day, - shortcuts: props.shortcuts || [], + shortcuts: Array.isArray(props.shortcuts) ? props.shortcuts : [], visible: false, currentView: DATEPICKER.Date, - disabledDate: props.disabledDate || '', + disabledDate: typeof props.disabledDate === 'function' ? props.disabledDate : null, cellClassName: '', selectableRange: [], - firstDayOfWeek: props.firstDayOfWeek || 7, + firstDayOfWeek: Number(props.firstDayOfWeek) || 7,examples/sites/demos/pc/app/date-panel/shortcuts.vue (2)
18-19
: Remove console.info statement.The console.info statement in the shortcut handler appears to be debugging code that should be removed.
onClick(picker) { const date = new Date() - console.info(picker) picker.$emit('pick', date) }
15-36
: Consider internationalizing shortcut labels.The shortcut labels are hardcoded in Chinese ('今天', '昨天', '一周前'). Consider using i18n for better internationalization support.
const shortcuts = [ { - text: '今天', + text: t('datepanel.shortcuts.today'), onClick(picker) { const date = new Date() picker.$emit('pick', date) } }, { - text: '昨天', + text: t('datepanel.shortcuts.yesterday'), onClick(picker) { const date = new Date() date.setTime(date.getTime() - 3600 * 1000 * 24) picker.$emit('pick', date) } }, { - text: '一周前', + text: t('datepanel.shortcuts.lastWeek'), onClick(picker) { const date = new Date() date.setTime(date.getTime() - 3600 * 1000 * 24 * 7) picker.$emit('pick', date) } } ]examples/sites/demos/pc/app/date-panel/custom-weeks.vue (1)
18-23
: Improve state management and documentation.The
eachWeekFirstDay
ref is updated but never used in the component. Additionally, theformatWeeks
function lacks documentation for its parameters and return value.Consider:
- Remove unused ref or document its purpose
- Add JSDoc comments explaining the function
-const eachWeekFirstDay = ref([]) -const formatWeeks = (customWeeks, weekFirstDays) => { +/** + * Formats the week number with a 'w' suffix + * @param {number} customWeeks - The week number to format + * @param {Date[]} weekFirstDays - Array of first days of each week + * @returns {string} Formatted week number with 'w' suffix + */ +const formatWeeks = (customWeeks, weekFirstDays) => { - eachWeekFirstDay.value = weekFirstDays return customWeeks + 'w' }packages/theme/src/date-panel/index.less (1)
31-34
: Consider consolidating duplicate styles.The new
.body-wrapper
class has a duplicateposition: relative
property that's also defined under the.has-time
selector. Consider consolidating these styles to avoid redundancy:&__body-wrapper { - position: relative; border: solid 1px var(--tv-DatePanel-border-color-divider); } &.has-sidebar.has-time { width: 434px; } &.has-sidebar { width: 398px; } &.has-time { - .@{picker-panel-prefix-cls}__body-wrapper { - position: relative; - } }Then, move the
position: relative
to the main.body-wrapper
class:&__body-wrapper { + position: relative; border: solid 1px var(--tv-DatePanel-border-color-divider); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
examples/sites/demos/apis/date-panel.js
(1 hunks)examples/sites/demos/pc/app/date-panel/basic-usage-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/basic-usage.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/basic-usage.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/custom-week.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/custom-weeks-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/custom-weeks.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/event-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/event.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/event.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/format-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/format.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/format.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/webdoc/date-panel.cn.md
(1 hunks)examples/sites/demos/pc/app/date-panel/webdoc/date-panel.en.md
(1 hunks)examples/sites/demos/pc/app/date-panel/webdoc/date-panel.js
(1 hunks)examples/sites/demos/pc/menus.js
(1 hunks)packages/renderless/src/date-panel/index.ts
(4 hunks)packages/renderless/src/date-panel/vue.ts
(8 hunks)packages/theme/src/date-panel/index.less
(1 hunks)packages/vue/src/date-panel/src/index.ts
(1 hunks)packages/vue/src/date-panel/src/pc.vue
(4 hunks)packages/vue/src/picker/src/pc.vue
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/sites/demos/pc/app/date-panel/webdoc/date-panel.cn.md
- examples/sites/demos/pc/app/date-panel/webdoc/date-panel.en.md
🧰 Additional context used
🪛 GitHub Actions: E2E Test PR--feat(date-panel): [date-panel] Support using only date panel
examples/sites/demos/pc/app/date-panel/basic-usage.spec.ts
[warning] Slow test file: Took 15.9s to execute
examples/sites/demos/pc/app/date-panel/format.spec.ts
[error] 8-8: Test failure: Timed out waiting for element with text '-01-15' to be visible after 8000ms
[warning] Slow test file: Took 24.1s to execute
examples/sites/demos/pc/app/date-panel/custom-week.spec.ts
[warning] Slow test file: Took 16.5s to execute
examples/sites/demos/pc/app/date-panel/readonly.spec.ts
[error] 13-13: Test failure: CSS property 'background-color' value mismatch. Expected '#1476ff' but received 'rgb(20, 118, 255)'
[warning] Slow test file: Took 24.0s to execute
🪛 eslint
examples/sites/demos/pc/app/date-panel/basic-usage.vue
[error] 12-13: Delete ⏎
(prettier/prettier)
🔇 Additional comments (13)
examples/sites/demos/pc/app/date-panel/readonly-composition-api.vue (1)
12-12
: Validate the initial date value.The hardcoded date should be validated to ensure it's a valid date string.
Consider adding validation:
- const value = ref('2025-01-05') + const value = ref(new Date('2025-01-05').toISOString().split('T')[0])Let's verify the TinyDatePanel component's exported interface:
examples/sites/demos/pc/app/date-panel/format-composition-api.vue (1)
1-25
: Verify integration with date panel requirements.The component successfully demonstrates the date panel formatting feature as intended in the PR objectives. However, let's verify the integration with other date panel features and documentation.
✅ Verification successful
Format feature implementation verified successfully
The date panel format feature is correctly implemented and consistent across both API styles. The documentation properly covers the format attribute functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent format handling across other date panel demos rg -l "tiny-date-panel.*format=" "examples/sites/demos/pc/app/date-panel/" # Verify documentation coverage for the format feature rg -l "format.*属性" "examples/sites/demos/pc/app/date-panel/webdoc/"Length of output: 318
Script:
#!/bin/bash # View the content of the Options API version cat "examples/sites/demos/pc/app/date-panel/format.vue" # Check documentation for format patterns cat "examples/sites/demos/pc/app/date-panel/webdoc/date-panel.js"Length of output: 3847
packages/renderless/src/date-panel/index.ts (1)
217-226
: Ensure consistent use ofhandleDatePick
anddoPick
inhandleShortcutClick
The conditional check for
props.shortcuts?.length
may not cover all scenarios where shortcuts are used. Verify that the logic correctly determines when to callhandleDatePick
versusdoPick
, and ensure that theprops.shortcuts
check aligns with how shortcuts are implemented.If needed, adjust the condition to accurately reflect when
handleDatePick
should be used.packages/vue/src/picker/src/pc.vue (1)
141-141
: Consider performance implications of using v-show.The change from
:visible
tov-show
means the panel will remain in the DOM when hidden. While this can improve performance for frequently toggled elements by avoiding re-renders, it increases initial page load and memory usage.Let's verify the frequency of panel toggling to confirm if this change is beneficial:
✅ Verification successful
Using v-show is the correct approach here
The picker panel is frequently toggled during normal usage (focus, selection, keyboard events, clicks). Using
v-show
is more efficient as it avoids expensive DOM operations for this frequently toggled component. The initial DOM cost is justified by the performance benefits during interaction.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for panel visibility toggle patterns in the codebase # Search for methods that might toggle panel visibility rg -A 5 "handleFocus|handleClose|handlePick" --type vueLength of output: 85
Script:
#!/bin/bash # Search for visibility-related patterns in Vue files rg -l "\.pickerVisible|handleFocus|handleClose|handlePick" # Get more context around the visibility state usage rg -B 3 -A 3 "pickerVisible" # Look for state management related to picker visibility ast-grep --pattern 'state.pickerVisible = $_'Length of output: 19240
packages/vue/src/date-panel/src/pc.vue (5)
28-28
: LGTM! Performance optimization for sidebar rendering.The change from
v-show
tov-if
withstate.shortcuts?.length
is a good optimization. It ensures the sidebar is only rendered in the DOM when shortcuts are available, reducing memory usage.
101-103
: LGTM! Improved code design with generic handler.The change to use
showHeaderPicker('Year')
instead of a specificshowYearPicker
method improves code maintainability by using a more generic approach.
105-105
: LGTM! Consistent use of generic handler.The change maintains consistency with the year picker by using the same generic
showHeaderPicker
approach.
147-147
: LGTM! Added readonly support.The addition of the readonly prop provides better control over the date table's interactivity.
333-333
: LGTM! Updated event names and added v-model support.The changes properly support v-model with 'update:modelValue' and provide a more specific event name with 'select-panel-change'.
examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue (1)
1-49
: Same issues as shortcuts.vue apply here.This file is identical to shortcuts.vue and has the same concerns regarding console.info and internationalization.
examples/sites/demos/pc/menus.js (1)
133-133
: LGTM! Menu item properly placed.The DatePanel menu item is correctly added to the form components section, maintaining alphabetical order and consistent structure with other entries.
examples/sites/demos/pc/app/date-panel/custom-weeks.vue (1)
4-10
: Consider adding validation for first-day-of-week.The
first-day-of-week
prop is hardcoded to 1 (Monday). Consider adding validation to ensure it's within the valid range (0-6).Run this script to check if the component validates the prop:
packages/theme/src/date-panel/index.less (1)
33-33
: LGTM! Consistent use of theming variables.The border styling correctly uses the
--tv-DatePanel-border-color-divider
CSS variable, maintaining consistency with the project's theming system.
@discreted66 点击日期时,控制台会报一个警告: |
@discreted66 E2E测试用例有两个报错。另外在标题把 date-picker 也加上,date-picker 是依赖 date-panel 的,需要确保 date-picker 的功能不受影响。 |
@discreted66 The E2E test case has two errors. In addition, add date-picker to the title. Date-picker depends on date-panel. You need to ensure that the function of date-picker is not affected. |
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
♻️ Duplicate comments (5)
examples/sites/demos/pc/app/date-panel/format.spec.ts (3)
7-8
:⚠️ Potential issueAdd explicit wait for panel visibility.
The test is failing with visibility timeout. Add explicit wait for the panel to be visible before interacting with it.
Apply this diff to fix the visibility timeout:
- await page.locator('#format').getByText('15').first().click() - await expect(page.getByText('-01-15')).toBeVisible() + // Wait for panel to be ready + await expect(page.getByTestId('date-panel')).toBeVisible({ timeout: 10000 }) + await page.getByTestId('date-15').click() + await expect(page.getByTestId('formatted-date')).toBeVisible({ + timeout: 10000, + message: 'Formatted date should be visible after selection' + })
10-11
:⚠️ Potential issueUse data-testid for reliable element selection.
Using text-based selectors with
nth
is fragile. Use data-testid attributes for more reliable element selection.Apply this diff to improve selector reliability:
- await page.locator('#format').getByText('8', { exact: true }).nth(2).click() - await expect(page.getByText('/01/08')).toBeVisible() + await page.getByTestId('date-8').click() + await expect(page.getByTestId('formatted-date-2')).toBeVisible({ + timeout: 10000, + message: 'Second formatted date should be visible after selection' + })
3-4
: 🛠️ Refactor suggestionAdd retry logic for flaky tests.
The test is failing intermittently in CI. Add retry logic to handle potential flakiness.
Apply this diff to add retry logic:
+test.describe.configure({ retries: 2 }); + test('[DatePanel] 测试格式化', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull())packages/vue/src/date-panel/src/pc.vue (1)
324-327
:⚠️ Potential issueFix the type of
disabledDate
prop.The
disabledDate
prop is incorrectly typed as Boolean. It should be a Function that takes a Date parameter and returns a boolean value.Apply this diff to fix the prop type:
- disabledDate: { - type: Boolean, - default: false - }, + disabledDate: { + type: Function, + default: null + },examples/sites/demos/apis/date-panel.js (1)
102-110
:⚠️ Potential issueFix incorrect demo reference for select-change event.
The pcDemo reference 'event' doesn't match the actual demo ID.
Apply this diff to fix the demo reference:
- pcDemo: 'event' + pcDemo: 'events'
🧹 Nitpick comments (3)
examples/sites/demos/apis/date-panel.js (1)
9-18
: Improve the disabled-date prop description.The current description "Configuration section disabled" is unclear. It should better explain the purpose and usage of the disabled-date function.
Apply this diff to improve the description:
desc: { - 'zh-CN': '配置部分禁用', - 'en-US': 'Configuration section disabled.' + 'zh-CN': '禁用日期的判断函数,参数为当前日期,返回true表示禁用', + 'en-US': 'Function that determines if a date should be disabled. Takes a Date object as parameter and returns true if the date should be disabled.' },packages/renderless/src/date-panel/index.ts (2)
217-226
: Improve code documentation clarity.Consider translating the Chinese comment to English for better maintainability and global collaboration:
- // 面板直接使用快捷选项 + // Direct usage of shortcuts in the panel
Line range hint
274-290
: Consider consolidating emit calls.The function currently calls both
cusEmit
andpanelEmit
, which might lead to duplicate events. Consider consolidating these into a single emit strategy to maintain clearer event handling.- api.cusEmit(state.date, state.showTime) - api.panelEmit(state.date, t, props) + api.panelEmit(state.date, t, props)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
examples/sites/demos/apis/date-panel.js
(1 hunks)examples/sites/demos/pc/app/date-panel/event-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/event.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/format-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/format.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/format.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/readonly.spec.ts
(1 hunks)packages/renderless/src/date-panel/index.ts
(4 hunks)packages/vue/src/date-panel/src/index.ts
(1 hunks)packages/vue/src/date-panel/src/mobile-first.vue
(1 hunks)packages/vue/src/date-panel/src/pc.vue
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- examples/sites/demos/pc/app/date-panel/format.vue
- examples/sites/demos/pc/app/date-panel/readonly.spec.ts
- examples/sites/demos/pc/app/date-panel/format-composition-api.vue
- examples/sites/demos/pc/app/date-panel/event.vue
- examples/sites/demos/pc/app/date-panel/event-composition-api.vue
- packages/vue/src/date-panel/src/index.ts
🔇 Additional comments (5)
packages/vue/src/date-panel/src/mobile-first.vue (1)
90-90
: LGTM! Good refactoring.The consolidation of
showYearPicker
andshowMonthPicker
into a single parameterizedshowHeaderPicker
method reduces code duplication and improves maintainability.Also applies to: 98-98
packages/vue/src/date-panel/src/pc.vue (2)
101-101
: LGTM! Good refactoring.The consolidation of
showYearPicker
andshowMonthPicker
into a single parameterizedshowHeaderPicker
method is consistent with the changes in mobile-first.vue.Also applies to: 105-105
333-333
:⚠️ Potential issueRevert the event name change.
The event name should remain as
select-change
based on the team's feedback.Apply this diff to revert the event name:
- emits: ['pick', 'select-change', 'update:modelValue', 'dodestroy'], + emits: ['pick', 'select-change', 'dodestroy'],Likely invalid or redundant comment.
packages/renderless/src/date-panel/index.ts (2)
168-176
: LGTM! Well-structured event emission implementation.The function follows Vue's v-model convention and maintains state consistency.
178-185
: Validate type parameter to prevent runtime errors.The function needs validation for the
type
parameter before accessingDATEPICKER[type]
to prevent potential runtime errors.
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 (4)
examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue (4)
3-3
: Add date formatting to improve value display.The raw date value might not be user-friendly. Consider formatting the date using a date formatting library or the
Intl.DateTimeFormat
API.- <div class="value">{{ value }}</div> + <div class="value">{{ value ? new Intl.DateTimeFormat('zh-CN').format(value) : '请选择日期' }}</div>
2-5
: Enhance accessibility and internationalization.Consider adding:
- ARIA labels for better screen reader support
- Internationalized text for better global support
- <div class="demo-date-panel-wrap"> - <div class="value">{{ value }}</div> - <tiny-date-panel v-model="value" :shortcuts="shortcuts"></tiny-date-panel> + <div class="demo-date-panel-wrap" role="group" aria-label="日期选择"> + <div class="value" aria-live="polite">{{ value }}</div> + <tiny-date-panel + v-model="value" + :shortcuts="shortcuts" + aria-label="日期面板"> + </tiny-date-panel>
24-26
: Improve date calculations and add constants for magic numbers.The date calculations use magic numbers and could be simplified. Consider:
- Using constants for time calculations
- Adding timezone handling
- Using Date methods instead of setTime
+const MS_PER_DAY = 24 * 60 * 60 * 1000; + +const getDateOffset = (days) => { + const date = new Date(); + date.setDate(date.getDate() - days); + return date; +}; + const shortcuts = [ { text: '今天', onClick(picker) { - const date = new Date() + const date = getDateOffset(0) picker.$emit('pick', date) } }, { text: '昨天', onClick(picker) { - const date = new Date() - date.setTime(date.getTime() - 3600 * 1000 * 24) + const date = getDateOffset(1) picker.$emit('pick', date) } }, { text: '一周前', onClick(picker) { - const date = new Date() - date.setTime(date.getTime() - 3600 * 1000 * 24 * 7) + const date = getDateOffset(7) picker.$emit('pick', date) } } ]Also applies to: 32-34
41-47
: Improve responsive design and spacing.The current styling uses a fixed width which might not be responsive on different screen sizes. Consider:
- Using relative units or max-width
- Adding comprehensive spacing
.demo-date-panel-wrap { - width: 560px; + max-width: 560px; + width: 100%; & > * { - margin-top: 12px; + margin: 12px 0; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/sites/demos/pc/app/date-panel/shortcuts-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.spec.ts
(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.vue
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/sites/demos/pc/app/date-panel/shortcuts.spec.ts
- examples/sites/demos/pc/app/date-panel/shortcuts.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
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: 2
🧹 Nitpick comments (1)
packages/vue/src/date-panel/__tests__/date-panel.test.ts (1)
14-15
: Enhance assertions with more specific checks.The current assertions are too generic. Add more specific checks for the date panel's structure and functionality.
-expect(wrapper.classes()).toContain('tiny-picker-panel') -expect(wrapper.find('td .current').exists()).toBe(true) +// Check panel structure +expect(wrapper.find('.tiny-picker-panel')).toBeTruthy() +expect(wrapper.find('.month-header')).toBeTruthy() +expect(wrapper.find('.week-header')).toBeTruthy() + +// Verify selected date +const currentCell = wrapper.find('td .current') +expect(currentCell.exists()).toBe(true) +expect(currentCell.text()).toBe('14') + +// Verify month and year display +expect(wrapper.find('.month-header').text()).toContain('January 2025')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue/src/date-panel/__tests__/date-panel.test.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Unit Test PR--feat(date-panel): [date-panel] DatePicker support using only date panel
packages/vue/src/date-panel/__tests__/date-panel.test.ts
[error] 9-9: Invalid value used as weak map key in test 'PC Mode > static'. Error occurred during component mounting.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
packages/vue/src/date-panel/__tests__/date-panel.test.ts (1)
1-7
: Verify component import and mounting setup.The pipeline failure indicates an issue with the component mounting. This could be related to how the component is being imported or initialized.
Run this script to check the component's export structure:
✅ Verification successful
Import and component structure are correctly implemented
The DatePanel component is properly exported and the import statement in the test file is correct. The component follows the standard Vue plugin structure with proper installation support and theme integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the component's export structure ast-grep --pattern 'export { $_ as DatePanel }'Length of output: 49
Script:
#!/bin/bash # Find the date-panel component file fd -t f "date-panel" --exclude "*.test.ts" # Check for any export patterns ast-grep --pattern 'export default $_' # Check named exports with DatePanel ast-grep --pattern 'export const DatePanel = $_' # Check package.json for the module path fd package.json --exec cat {} \; | grep -A 5 -B 5 "vue-date-panel"Length of output: 66286
Script:
#!/bin/bash # Find the date-panel component source file fd -t f "date-panel" -e vue -e ts -e js --exclude "*.test.ts" --exclude "*.type.ts" # Check for any exports in date-panel related files rg -l "export.*DatePanel" --type ts --type js --type vue # Check for default exports in date-panel files rg -l "export default" --type ts --type js --type vue | grep -i "date-panel"Length of output: 375
Script:
#!/bin/bash # Find date-panel files in packages directory fd -t f "date-panel" packages/ # Search for exports without file type restrictions rg "export.*DatePanel" packages/ # Check package structure fd -t f "package.json" packages/ --exec cat {} \; | grep -A 2 -B 2 "date-panel"Length of output: 916
Script:
#!/bin/bash # Check the component implementation cat packages/vue/src/date-panel/index.ts # Verify if there are any other exports rg "export" packages/vue/src/date-panel/ --type tsLength of output: 1226
198bde0
to
bd3801b
Compare
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: 2
♻️ Duplicate comments (2)
packages/vue/src/date-panel/__tests__/date-panel.test.ts (2)
6-8
: 🛠️ Refactor suggestionAdd test cleanup to prevent potential side effects.
Add an afterEach hook to clean up mounted components after each test.
describe('PC Mode', () => { const mount = mountPcMode + afterEach(() => { + // Clean up any mounted components after each test + wrapper.unmount() + })
9-19
: 🛠️ Refactor suggestionExpand test coverage for DatePanel functionality.
The current test case only verifies basic rendering. Add test cases for core functionality:
- Date selection and value updates
- Month/year navigation
- Disabled dates
- Format validation
- Event emissions
test('static', () => { const wrapper = mount(DatePanel, { props: { modelValue: ref('2025-01-14'), format: 'yyyy-MM-dd' } }) expect(wrapper.classes()).toContain('tiny-picker-panel') expect(wrapper.find('td .current').exists()).toBe(true) }) +test('date selection', async () => { + const wrapper = mount(DatePanel, { + props: { + modelValue: ref('2025-01-14'), + format: 'yyyy-MM-dd' + } + }) + await wrapper.find('td:not(.disabled)').trigger('click') + expect(wrapper.emitted('update:modelValue')).toBeTruthy() +}) + +test('month navigation', async () => { + const wrapper = mount(DatePanel, { + props: { + modelValue: ref('2025-01-14'), + format: 'yyyy-MM-dd' + } + }) + await wrapper.find('.prev-month').trigger('click') + expect(wrapper.find('.month-header').text()).toContain('December') +})🧰 Tools
🪛 GitHub Actions: Unit Test PR--feat(date-panel): [date-panel,date-picker] DatePicker support using only date panel
[error] 10-10: Invalid value used as weak map key in PC Mode test. Error occurs during component mounting in test setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue/src/date-panel/__tests__/date-panel.test.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Unit Test PR--feat(date-panel): [date-panel,date-picker] DatePicker support using only date panel
packages/vue/src/date-panel/__tests__/date-panel.test.ts
[error] 10-10: Invalid value used as weak map key in PC Mode test. Error occurs during component mounting in test setup.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
packages/vue/src/date-panel/__tests__/date-panel.test.ts (1)
1-5
: LGTM! Import statements are well-organized.The necessary testing utilities and component dependencies are properly imported.
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 (1)
packages/vue/src/date-panel/__tests__/date-panel.test.tsx (1)
1-19
: Consider restructuring the test suite for better organization and coverage.The current test suite structure could be improved to better organize different test scenarios:
- Group related test cases using nested
describe
blocks- Add separate sections for:
- Basic rendering
- Date formatting
- User interactions
- Edge cases
- Error scenarios
- Consider using shared fixtures or factory functions for common setup
Would you like me to provide a template for the restructured test suite?
🧰 Tools
🪛 GitHub Actions: Unit Test PR--feat(date-panel): [date-panel,date-picker] DatePicker support using only date panel
[error] 10-10: TypeError: Invalid value used as weak map key in PC Mode static test
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/sites/demos/pc/app/date-picker/align.spec.ts
(3 hunks)packages/vue/src/date-panel/__tests__/date-panel.test.tsx
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Unit Test PR--feat(date-panel): [date-panel,date-picker] DatePicker support using only date panel
packages/vue/src/date-panel/__tests__/date-panel.test.tsx
[error] 10-10: TypeError: Invalid value used as weak map key in PC Mode static test
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (4)
examples/sites/demos/pc/app/date-picker/align.spec.ts (2)
15-16
: LGTM: Robust alignment verification logic.The alignment verification using x-placement attributes is well-implemented:
- Handles both bottom and top placements
- Uses appropriate regex patterns for flexible matching
- Verifies all three alignment modes (left, center, right)
Also applies to: 25-26, 35-36
11-11
: 🛠️ Refactor suggestionReplace fixed timeouts with proper wait conditions.
Using arbitrary timeouts (
waitForTimeout
) is an anti-pattern in E2E testing as it can make tests flaky and slow. Instead, wait for specific conditions using Playwright's built-in wait methods.Consider this approach:
- await page.waitForTimeout(400) + await leftDatePanelDom.waitFor({ state: 'attached' })Also investigate if the need for increased timeouts indicates potential performance issues with the DatePanel rendering.
Let's check for similar patterns across other test files:
Also applies to: 14-14, 24-24, 34-34
packages/vue/src/date-panel/__tests__/date-panel.test.tsx (2)
1-4
: LGTM! Import statements are well-organized.The necessary testing utilities, test runners, and component dependencies are properly imported.
6-8
: Verify mountPcMode implementation due to pipeline failure.The test suite setup looks correct, but there's a pipeline failure indicating a "TypeError: Invalid value used as weak map key". This might be related to how
mountPcMode
is implemented or used.Let's check the implementation of
mountPcMode
:
da12813
to
25d66a7
Compare
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
♻️ Duplicate comments (1)
packages/vue/src/date-panel/src/index.ts (1)
21-24
: 🛠️ Refactor suggestionAdd validation for firstDayOfWeek prop.
The firstDayOfWeek prop should be validated to ensure it's within valid range (1-7).
firstDayOfWeek: { type: Number, default: 7, + validator: (value) => value >= 1 && value <= 7 },
🧹 Nitpick comments (4)
packages/vue/src/date-panel/src/index.ts (3)
25-27
: Add default value and JSDoc for nowClick prop.The nowClick prop should have a default value and documentation for its expected signature.
nowClick: { type: Function, + default: null, + /** + * Callback when clicking the "now" button + * @param {Date} date - The current date + * @returns {void} + */ },
32-35
: Add default format and documentation.The format prop should have a default date format string and documentation for supported patterns.
format: { type: String, - default: '' + default: 'yyyy-MM-dd', + /** + * Date format string + * @example 'yyyy-MM-dd', 'dd/MM/yyyy', 'MM/dd/yyyy' + */ },
40-43
: Specify shortcuts array type and add documentation.The shortcuts prop should specify the expected item type structure.
shortcuts: { - type: Array, + type: Array as PropType<Array<{ text: string; value: Date | (() => Date) }>>, default: () => [], + /** + * Array of shortcut options + * @example [{ text: 'Today', value: new Date() }] + */ },packages/vue/src/date-panel/src/pc.vue (1)
296-326
: Add JSDoc comments for the new props.The new props enhance the component's configurability, but they lack documentation. Consider adding JSDoc comments to describe:
- Expected format for the modelValue
- Available format string options
- Structure of shortcut items
- Expected signature and usage of the disabledDate function
Example documentation:
+ /** + * First day of the week (1: Monday, 7: Sunday) + * @default 7 + */ firstDayOfWeek: { type: Number, default: 7 },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/sites/demos/pc/app/date-picker/align.spec.ts
(3 hunks)packages/vue/src/date-panel/__tests__/date-panel.test.tsx
(1 hunks)packages/vue/src/date-panel/src/index.ts
(1 hunks)packages/vue/src/date-panel/src/pc.vue
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/sites/demos/pc/app/date-picker/align.spec.ts
- packages/vue/src/date-panel/tests/date-panel.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (6)
packages/vue/src/date-panel/src/index.ts (3)
36-39
: LGTM!The readonly prop is correctly defined with appropriate type and default value.
48-50
: LGTM!The popperClass prop is correctly defined with appropriate type and default value.
44-47
: 🛠️ Refactor suggestionImprove disabledDate function signature and default value.
The disabledDate function should return a boolean and have proper documentation.
disabledDate: { type: Function, - default: () => null + default: () => false, + /** + * Function to determine if a date should be disabled + * @param {Date} date - The date to check + * @returns {boolean} - Returns true if the date should be disabled + */ },Likely invalid or redundant comment.
packages/vue/src/date-panel/src/pc.vue (3)
Line range hint
28-40
: LGTM! Improved performance and safety.The change from
v-show
tov-if
with optional chaining is a good optimization. It prevents unnecessary DOM rendering when there are no shortcuts and safely handles undefined values.
Line range hint
101-109
: LGTM! Good consolidation of picker methods.The consolidation of show methods into
showHeaderPicker
improves code maintainability while preserving accessibility attributes.
332-332
: Verify the impact of the event name change.The event name change from 'select-change' to 'select-panel-change' might affect existing event listeners in parent components.
Let's verify the usage of this event in the codebase:
Caution No docstrings were generated. |
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
Summary by CodeRabbit
Based on the comprehensive changes, here are the updated release notes:
New Features
firstDayOfWeek
,modelValue
,format
,readonly
,shortcuts
,disabledDate
, andpopperClass
.Enhancements
Bug Fixes
Documentation