-
Notifications
You must be signed in to change notification settings - Fork 266
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(time-picker): [time-picker] Optimize and modify component style issues #2452
Conversation
WalkthroughThis pull request introduces a series of modifications primarily focused on the styling of various date picker components across multiple Vue files. The key change is the increase in width for the Changes
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: 6
🧹 Outside diff range and nitpick comments (39)
examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue (1)
18-18
: Consider adding a comment about the standardized width.To help maintain consistency in future updates, consider adding a comment explaining that this width is standardized across date picker components.
.demo-date-picker-wrap { + // Standardized width for all date picker components width: 360px;
examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (1)
Line range hint
7-20
: Consider using consistent component naming.While the code works, there's a minor inconsistency between the component registration name
TinyDatePicker
and its usage astiny-date-picker
in the template. Consider using kebab-case consistently in the template or updating the component registration to match the usage.- TinyDatePicker: DatePicker + 'tiny-date-picker': DatePickerexamples/sites/demos/pc/app/date-picker/unlink-panels.vue (2)
Line range hint
1-15
: Consider enhancing documentation with internationalized comments.The template contains Chinese comments. For better maintainability and international collaboration, consider using English comments or implementing i18n for documentation.
- <p>默认启用面板联动:</p> + <p>Default (with linked panels):</p> - <p>关闭面板联动:</p> + <p>Disabled panel linkage:</p>
Line range hint
6-8
: Consider enhancing accessibility attributes.The date picker implementation could benefit from additional accessibility attributes to improve user experience for screen readers.
<div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" type="daterange"></tiny-date-picker> + <tiny-date-picker + v-model="value" + type="daterange" + aria-label="Select date range with linked panels" + ></tiny-date-picker> </div> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" type="daterange" unlink-panels></tiny-date-picker> + <tiny-date-picker + v-model="value" + type="daterange" + unlink-panels + aria-label="Select date range with independent panels" + ></tiny-date-picker> </div>Also applies to: 12-14
examples/sites/demos/pc/app/date-picker/step.vue (2)
Line range hint
1-15
: Consider implementing i18n for text contentThe template contains hardcoded Chinese text ("步长" and "箭头控制"). Consider using i18n translations to support internationalization.
- <p>步长:</p> + <p>{{ $t('datePicker.step') }}</p> - <p>箭头控制:</p> + <p>{{ $t('datePicker.arrowControl') }}</p>
Line range hint
18-33
: Consider adding prop validation for the step valuesWhile the current step values are reasonable, adding validation can prevent potential issues with invalid inputs.
data() { return { step: { hour: 2, minute: 5 }, value: '' } } +computed: { + validatedStep() { + return { + hour: Math.max(1, Math.min(24, this.step.hour)), + minute: Math.max(1, Math.min(60, this.step.minute)) + } + } +}Then use
:step="validatedStep"
in the template.examples/sites/demos/pc/app/date-picker/clear.vue (2)
Line range hint
1-21
: Consider improving semantic structure and spacing.The template structure is clear but could be enhanced for better maintainability:
- Replace
<br />
tags with CSS margins for spacing- Use semantic grouping for each demo section
- Use more specific class names
Consider this structure:
<template> <div> - <p>默认显示清除按钮:</p> - <br /> - <div class="demo-date-picker-wrap"> + <section class="date-picker-demo"> + <h3 class="demo-title">默认显示清除按钮:</h3> + <div class="date-picker-container"> <tiny-date-picker v-model="value"></tiny-date-picker> - </div> - <br /> + </div> + </section> <!-- Similar changes for other sections --> </div> </template>Add corresponding CSS:
.date-picker-demo { margin-bottom: 24px; } .demo-title { margin-bottom: 16px; }
Line range hint
23-38
: Consider improvements to data initialization and naming conventions.A few suggestions to enhance the implementation:
- The hard-coded date might not be ideal for all use cases
- The
IconClose
variable name should follow Vue's component naming conventionConsider these improvements:
data() { return { - value: new Date('2023-05-24'), - IconClose: iconClose() + value: new Date(), + clearIcon: iconClose() } }Then update the template:
-<tiny-date-picker v-model="value" :clear-icon="IconClose"></tiny-date-picker> +<tiny-date-picker v-model="value" :clear-icon="clearIcon"></tiny-date-picker>examples/sites/demos/pc/app/time-picker/basic-usage.spec.ts (1)
15-16
: Documentation update needed for time format change.The time format now includes seconds in all test cases. This change should be documented in the component's documentation to inform users about the new default behavior.
#!/bin/bash # Check if documentation has been updated rg -l "time.*format.*:20" --type mdexamples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (1)
34-34
: Consider adding a comment explaining the width choice.To improve maintainability, consider adding a comment explaining why 360px was chosen as the standard width for date-picker components.
.demo-date-picker-wrap { + // Standard width for date-picker components to accommodate datetime range inputs width: 360px; & > * {
examples/sites/demos/pc/app/date-picker/basic-usage.vue (1)
Line range hint
33-38
: Consider adding responsive design support.While the width increase improves the desktop experience, consider making the component more responsive for different screen sizes.
Here's a suggested enhancement:
.demo-date-picker-wrap { - width: 360px; + width: 360px; + max-width: 100%; + + @media (max-width: 480px) { + width: 100%; + } & > * { margin-top: 12px; } }examples/sites/demos/pc/app/date-picker/format-composition-api.vue (1)
38-41
: Consider documenting the width requirement.To maintain consistency across the codebase and help other developers, consider adding a comment explaining why 360px was chosen as the standard width for date picker wrappers.
.demo-date-picker-wrap { + // Standard width for date picker components to ensure consistent layout width: 360px; & > * {
examples/sites/demos/pc/app/date-picker/now-composition-api.vue (4)
Line range hint
3-19
: Consider implementing internationalization for text content.The template contains hardcoded Chinese text that should be internationalized for better maintainability and multi-language support:
- "插槽式"
- "函数式"
- "此刻(服务器时间)"
Line range hint
21-24
: Consider using a constant or configuration for default dates.The initial date values are hardcoded. Consider extracting these to a constant or configuration to improve maintainability.
+const DEFAULT_DATE = '2020-11-11 10:10:11' -const value1 = ref('2020-11-11 10:10:11') -const value2 = ref('2020-11-11 10:10:11') +const value1 = ref(DEFAULT_DATE) +const value2 = ref(DEFAULT_DATE)
Line range hint
26-31
: Improve Promise implementation innowClick
.The current implementation has several issues:
- Unnecessary
setTimeout
with 0ms delay- No error handling
- Inconsistent return type (Date object) compared to the string format used elsewhere
Consider this improved implementation:
const nowClick = () => { - return new Promise((resolve) => { - setTimeout(() => { - resolve(new Date(2024, 9, 8, 18, 18, 18)) - }, 0) - }) + return Promise.resolve('2024-10-08 18:18:18') + .catch(error => { + console.error('Failed to get current time:', error) + throw error + }) }
Line range hint
33-35
: Add error handling to thenow
method.The direct value assignment could potentially cause reactivity issues if the value is invalid.
const now = () => { - value1.value = '2018-12-11 18:18:18' + try { + const date = '2018-12-11 18:18:18' + if (!isValidDate(date)) { + throw new Error('Invalid date format') + } + value1.value = date + } catch (error) { + console.error('Failed to set current time:', error) + } } + +function isValidDate(dateString) { + const date = new Date(dateString) + return date instanceof Date && !isNaN(date) +}examples/sites/demos/pc/app/date-picker/timezone-composition-api.vue (2)
Line range hint
1-23
: Enhance template structure and accessibilityConsider improving the template structure with these recommendations:
- Replace
<p>
tags with semantic heading elements (e.g.,<h3>
)- Add ARIA labels to the date pickers for better accessibility
- Replace
<br />
tags with CSS margins for spacing<template> <div> - <p>开启时区选择</p> - <br /> + <h3>开启时区选择</h3> <div class="demo-date-picker-wrap"> <tiny-date-picker + aria-label="Timezone selection date picker" v-model="timezoneValue" type="datetime" :show-timezone="true" :timezone-data="tzData" default-timezone="Etc/GMT+12" @select-change="selectChange" ></tiny-date-picker> </div> - <br /> - <p>默认时间</p> - <br /> + <h3>默认时间</h3>
Line range hint
33-39
: Consider improving timezone selection feedbackThe current modal message only shows the timezone name. Consider enhancing it with more context, such as the offset and current time in that timezone.
function selectChange(tz) { - Modal.message({ message: `当前值为 ${tz.tz.name}`, status: 'info' }) + Modal.message({ + message: `Selected timezone: ${tz.tz.name} (UTC${tz.tz.offset >= 0 ? '+' : ''}${tz.tz.offset})`, + status: 'info' + }) }examples/sites/demos/pc/app/date-picker/now.vue (3)
Line range hint
1-19
: Enhance accessibility and internationalization.Consider the following improvements to the template structure:
- Add ARIA labels to the date pickers
- Use i18n for the Chinese text
- Use semantic HTML for the title sections
<template> <div> - <div class="demo-date-picker"> - <div class="title">插槽式:</div> - <tiny-date-picker v-model="value1" type="datetime"> + <div class="demo-date-picker" role="region"> + <h3 class="title">{{ t('slotBased') }}</h3> + <tiny-date-picker + v-model="value1" + type="datetime" + aria-label="Date and time picker with custom now button"> <template #now> - <div class="nowclass" @click="now">此刻(服务器时间)</div> + <div class="nowclass" @click="now" role="button">{{ t('currentServerTime') }}</div> </template> </tiny-date-picker> </div> - <div class="demo-date-picker"> - <div class="title">函数式:</div> - <tiny-date-picker v-model="value2" type="datetime" :now-click="nowClick"> </tiny-date-picker> + <div class="demo-date-picker" role="region"> + <h3 class="title">{{ t('functionBased') }}</h3> + <tiny-date-picker + v-model="value2" + type="datetime" + :now-click="nowClick" + aria-label="Date and time picker with function-based now feature"> + </tiny-date-picker> </div> </div> </template>
Line range hint
32-43
: Improve date handling consistency and flexibility.The current implementation has several potential issues:
- Hardcoded dates in both methods could lead to maintenance issues
- Inconsistent date handling between methods (string vs Date object)
- Unnecessary setTimeout with 0ms delay
methods: { now() { - this.value1 = '2018-12-11 18:18:18' + this.value1 = new Date().toISOString().slice(0, 19).replace('T', ' ') }, nowClick() { return new Promise((resolve) => { - setTimeout(() => { - resolve(new Date(2024, 9, 8, 18, 18, 18)) - }, 0) + resolve(new Date()) }) } }
Line range hint
48-63
: Enhance style maintainability and user experience.Consider using CSS custom properties and adding hover states for interactive elements.
<style scoped> +:root { + --date-picker-width: 360px; + --primary-color: #0067d1; + --hover-color: #1a7be0; +} + .demo-date-picker { - width: 360px; + width: var(--date-picker-width); } .nowclass { display: inline-flex; height: 28px; align-items: center; - color: #0067d1; + color: var(--primary-color); cursor: pointer; + transition: color 0.2s ease; +} +.nowclass:hover { + color: var(--hover-color); } .title { font-weight: bold; padding: 10px 0; } </style>examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (3)
Line range hint
1-29
: Improve template accessibility and structure.
- Add ARIA labels to improve accessibility
- Replace
<br />
tags with CSS margins for better maintainability- Add type annotations to v-model binding
Consider applying these improvements:
<template> <div> - <p>默认隐藏周次序号:</p> - <br /> + <p class="demo-description">默认隐藏周次序号:</p> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value"></tiny-date-picker> + <tiny-date-picker + v-model="value" + aria-label="Date picker without week numbers" + ></tiny-date-picker> </div> - <br /> - <p>显示周次序号:</p> - <br /> + <p class="demo-description">显示周次序号:</p> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" show-week-number></tiny-date-picker> + <tiny-date-picker + v-model="value" + show-week-number + aria-label="Date picker with week numbers" + ></tiny-date-picker> </div>Add these styles:
.demo-description { margin: 1rem 0; }
Line range hint
31-45
: Enhance type safety and error handling in script section.The current implementation could benefit from TypeScript types and proper validation.
Consider applying these improvements:
<script setup lang="ts"> import { ref } from 'vue' import { DatePicker as TinyDatePicker } from '@opentiny/vue' -const eachWeekFirstDay = ref([]) -const value = ref('') +const eachWeekFirstDay = ref<number[]>([]) +const value = ref<string>('') const pickerOptions = ref({ firstDayOfWeek: 1 }) -function formatWeeks(customWeeks, weekFirstDays) { +function formatWeeks(customWeeks: number, weekFirstDays: number[]): string { + if (!Array.isArray(weekFirstDays) || weekFirstDays.some(day => day < 0 || day > 6)) { + console.warn('Invalid weekFirstDays provided') + return String(customWeeks) + } eachWeekFirstDay.value = weekFirstDays return customWeeks + 'w' }
Line range hint
1-53
: Add component documentation.Consider adding JSDoc comments to document:
- Component purpose and usage
- Available props and their types
- Examples for each feature (default, week numbers, custom format)
Add documentation at the top of the component:
+<!-- + * @description A date picker component demonstrating various week number display options + * @props {boolean} show-week-number - Shows/hides week numbers + * @props {Function} format-weeks - Customizes week number format + * @props {Object} picker-options - Configures picker behavior + --> <template>examples/sites/demos/pc/app/date-picker/timezone.vue (2)
Line range hint
1-23
: LGTM! Consider adding prop documentation.The template structure is well-organized with clear sections. Each date picker instance serves a distinct purpose with appropriate configuration.
Consider adding comments to document the purpose of key props:
show-timezone
timezone-data
default-timezone
isutc8
Line range hint
26-47
: Consider making the reference date dynamic.The
referenceDate
is currently hardcoded to a specific date ('2020-10-28'). This could become outdated over time.Consider using a dynamic default date:
- referenceDate: '2020-10-28T00:00:00.000+0800' + referenceDate: new Date().toISOString().replace('Z', '+0800')examples/sites/demos/pc/app/date-picker/custom-weeks.vue (1)
Line range hint
42-47
: Consider adding input validation to formatWeeks method.The method currently assumes valid inputs. Consider adding type checking and validation for robustness.
formatWeeks(customWeeks, weekFirstDays) { + if (typeof customWeeks !== 'string' && typeof customWeeks !== 'number') { + console.warn('customWeeks should be a string or number') + return customWeeks + } + if (!Array.isArray(weekFirstDays)) { + console.warn('weekFirstDays should be an array') + return customWeeks + } this.eachWeekFirstDay = weekFirstDays return customWeeks + 'w' }examples/sites/demos/pc/app/date-picker/disabled.vue (2)
Line range hint
1-22
: Consider enhancing accessibility and internationalization.The template could benefit from the following improvements:
- Add ARIA labels to improve screen reader support
- Consider using i18n for placeholder texts instead of hardcoded Chinese characters
Example improvement:
- <tiny-date-picker v-model="disabledValue" disabled placeholder="请选择日期"></tiny-date-picker> + <tiny-date-picker + v-model="disabledValue" + disabled + placeholder="请选择日期" + aria-label="Disabled date picker" + :placeholder="t('datePicker.selectDate')" + ></tiny-date-picker>
Line range hint
37-46
: Improve date handling flexibility.The current implementation has hardcoded dates in the
pickerOptions
and initial values. Consider:
- Moving date restrictions to computed properties or constants
- Using dynamic initial dates relative to the current date
Example improvement:
data() { + const TODAY = new Date() + const MIN_DATE = new Date('2023-08-01') return { - disabledValue: '2020/10/29', + disabledValue: TODAY.toISOString().split('T')[0], value: '', dateRangeValue: '', - readonlyValue: '2020/10/29', + readonlyValue: TODAY.toISOString().split('T')[0], editableValue: '', pickerOptions: { disabledDate(time) { - return time.getTime() > Date.now() || time.getTime() < new Date('2023-08-01').getTime() + return time.getTime() > TODAY.getTime() || time.getTime() < MIN_DATE.getTime() } } } }packages/theme/src/time-panel/vars.less (1)
Line range hint
41-41
: Address the pending color confirmation comment.There's an existing comment indicating that the confirm button color was temporarily changed to match the link button color (
#1476ff
) and needs confirmation. Consider following up on this design decision to ensure it's intentionally left as is.examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue (2)
Line range hint
9-19
: Consider enabling validate-event for better user feedbackThe date and time pickers have
validate-event="false"
while having validation rules. This means validation only occurs on form submission rather than providing immediate feedback. Consider enabling validate-event for a better user experience.<tiny-date-picker - :validate-event="false" type="date" placeholder="选择日期" v-model="ruleForm.date1" ></tiny-date-picker> ... <tiny-time-picker - :validate-event="false" placeholder="选择时间" v-model="ruleForm.date2" ></tiny-time-picker>
Line range hint
39-43
: Initialize missing date2 field in ruleFormThe
date2
field has validation rules but is not initialized in the ruleForm object. This could lead to reactivity issues.const ruleForm = ref({ name: '', - date1: '' + date1: '', + date2: '' })examples/sites/demos/pc/app/time-picker/format.spec.ts (2)
15-15
: LGTM! Consider applying the same fix consistently.The addition of
.first()
makes the selector more precise and less prone to ambiguity when multiple elements match. However, there are similar selectors in the file that could benefit from the same improvement.Consider applying the same pattern to other similar selectors, for example:
- await page.getByRole('listitem').filter({ hasText: '09 PM' }).locator('span').click() + await page.getByRole('listitem').filter({ hasText: '09 PM' }).locator('span').first().click()
Line range hint
2-50
: Replace hardcoded waits with proper wait conditions.The test uses multiple
waitForTimeout
calls which can make tests flaky. Instead, use Playwright's built-in wait conditions that are more reliable.Consider replacing the timeouts with proper wait conditions:
- await page.waitForTimeout(100) + await expect(page.getByRole('listitem').filter({ hasText: '08 pm' })).toBeVisible()This makes the tests more reliable and faster as they won't wait the full timeout when elements are ready sooner.
packages/theme/src/time-panel/index.less (1)
Line range hint
89-89
: Address or remove the TODO comment.There's a TODO comment questioning whether the borderless scenario is needed: "待确认有没有这种无边框场景,没有的话去掉" (Translation: "To confirm if there is such a borderless scenario, if not, remove it").
Would you like me to:
- Help investigate if this borderless scenario is still needed?
- Create a GitHub issue to track this technical debt?
packages/theme/src/time-spinner/index.less (1)
57-59
: LGTM! Consider adding RTL support.The flex layout implementation improves alignment control. However, for better internationalization support, consider adding RTL (Right-to-Left) language support.
.@{time-spinner-prefix-cls}__list { display: flex; flex-direction: column; align-items: center; + [dir="rtl"] & { + align-items: center; /* RTL languages maintain center alignment */ + }packages/theme/src/picker/index.less (1)
Line range hint
251-251
: Consider addressing the TODO comment about scrollbar evaluation.There's a TODO comment indicating that the scrollbar implementation needs evaluation. Since this PR is focusing on component style optimizations, it would be a good opportunity to assess if this scrollbar styling is still needed.
Would you like me to help evaluate the scrollbar usage across the codebase and create a GitHub issue to track this cleanup task?
packages/vue/src/picker/src/pc.vue (1)
Line range hint
131-134
: Consider refactoring display-only mode implementation.The display-only mode logic is spread across multiple conditions and components. Consider extracting this into a separate component or computed property for better maintainability.
Consider creating a dedicated display-only component:
<!-- DisplayOnlyContent.vue --> <template> <tiny-tooltip v-if="isDisplayOnly" class="tiny-range-editor-display-only" :content="tooltipContent" effect="light" placement="top" @mouseenter.native="onEnter" > <span>{{ formattedValue }}</span> </tiny-tooltip> </template>This would:
- Improve code organization
- Make the logic more reusable
- Simplify the main component's template
Also applies to: 139-148
examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (1)
Line range hint
1-200
: Consider improving documentation structure.While reviewing the entire file, I noticed some opportunities for improvement:
- Many demos lack English descriptions (e.g., 'event' demo)
- Some descriptions mix HTML tags with markdown code blocks
- Inconsistent spacing around code tags across descriptions
Consider:
- Implementing a documentation linter to enforce consistent formatting
- Using a unified documentation format (either HTML or markdown, not mixed)
- Adding missing English translations for better internationalization
- Creating a template for new demo entries to ensure consistency
Would you like me to help create these documentation guidelines or set up a linter configuration?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (49)
- examples/sites/demos/pc/app/date-picker/align-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/align.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/basic-usage.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/clear-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/clear.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-suffix-icon-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-weeks.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/default-value.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/disabled.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/events-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/date-picker/events.vue (2 hunks)
- examples/sites/demos/pc/app/date-picker/filter-mode-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/filter-mode.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/format-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/format.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/label-inside-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/label-inside.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/multiple-dates.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/now-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/now.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/size-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/size.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/step-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/step.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/timezone-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/timezone.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/unlink-panels-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/unlink-panels.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/validate-event.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/basic-usage.spec.ts (1 hunks)
- examples/sites/demos/pc/app/time-picker/basic-usage.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/format.spec.ts (1 hunks)
- examples/sites/demos/pc/app/time-picker/is-range-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/is-range.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (2 hunks)
- packages/theme/src/picker/index.less (2 hunks)
- packages/theme/src/time-panel/index.less (1 hunks)
- packages/theme/src/time-panel/vars.less (1 hunks)
- packages/theme/src/time-spinner/index.less (2 hunks)
- packages/vue/src/picker/src/pc.vue (1 hunks)
- packages/vue/src/time/src/pc.vue (0 hunks)
💤 Files with no reviewable changes (1)
- packages/vue/src/time/src/pc.vue
✅ Files skipped from review due to trivial changes (14)
- examples/sites/demos/pc/app/date-picker/align-composition-api.vue
- examples/sites/demos/pc/app/date-picker/align.vue
- examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue
- examples/sites/demos/pc/app/date-picker/clear-composition-api.vue
- examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue
- examples/sites/demos/pc/app/date-picker/filter-mode-composition-api.vue
- examples/sites/demos/pc/app/date-picker/filter-mode.vue
- examples/sites/demos/pc/app/date-picker/label-inside-composition-api.vue
- examples/sites/demos/pc/app/date-picker/multiple-dates.vue
- examples/sites/demos/pc/app/date-picker/step-composition-api.vue
- examples/sites/demos/pc/app/date-picker/unlink-panels-composition-api.vue
- examples/sites/demos/pc/app/date-picker/validate-event.vue
- examples/sites/demos/pc/app/time-picker/is-range-composition-api.vue
- examples/sites/demos/pc/app/time-picker/is-range.vue
🔇 Additional comments (46)
examples/sites/demos/pc/app/date-picker/custom-suffix-icon-composition-api.vue (2)
Line range hint
1-24
: Implementation looks good!The component follows Vue 3 best practices with Composition API and proper icon integration.
18-18
: Verify consistent width across all date picker demos.The width change to 360px appears to be part of a standardization effort.
#!/bin/bash # Description: Verify that all date picker demos use the consistent width of 360px # Expected: All .demo-date-picker-wrap classes should have width: 360px # Search for width declarations in date-picker demo files rg -g "examples/sites/demos/pc/app/date-picker/*.vue" "demo-date-picker-wrap.*width:" -A 1examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue (1)
18-18
: LGTM! Width adjustment aligns with standardization effort.The width increase to 360px is consistent with the broader initiative to standardize date picker component widths across the application.
examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (2)
Line range hint
1-5
: LGTM! Template structure follows Vue best practices.The template is well-structured with proper component usage and prop bindings.
25-32
: Verify consistent width changes across related files.The width change to 360px appears to be part of a broader standardization effort. Let's verify this change is consistent across all related files.
✅ Verification successful
Width changes are consistently applied across all date-picker demos
The verification confirms that all date-picker demo files consistently use
width: 360px
for the.demo-date-picker-wrap
class. Time-picker demos use a different width (280px) which is expected as they are a separate component with different requirements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify width consistency across date-picker demo files # Expected: All demo files should use 360px width # Search for width declarations in demo-date-picker-wrap class rg -l "demo-date-picker-wrap" | xargs rg "width:.*px"Length of output: 4897
examples/sites/demos/pc/app/date-picker/label-inside.vue (2)
Line range hint
1-31
: Implementation looks good!The component maintains a clean and simple implementation with proper component registration, data management, and consistent styling.
26-26
: Verify consistent width across date picker components.The width adjustment to 360px aligns with the PR's style optimization objective and matches changes made in other files.
Let's verify this width is consistently applied across all date picker demos:
✅ Verification successful
Width is consistently set to 360px across all date picker demos
The verification confirms that all date picker demo files in the repository consistently use a width of 360px for their wrapper elements. This includes both regular and composition API variants of each demo, maintaining a uniform appearance throughout the component examples.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent width styling across date picker demos # Expected: All date picker demos should use 360px width # Search for width declarations in date picker demo files rg -g '*.vue' -B 2 -A 2 'width:.*px' 'examples/sites/demos/pc/app/date-picker/'Length of output: 15094
examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (1)
19-19
: Verify if adding seconds is intentional.The default time value now includes 20 seconds (18:40:20) instead of just hours and minutes (18:40:00). This might affect user experience as time pickers typically deal with hour:minute precision by default.
✅ Verification successful
Adding seconds to default time value is consistent across demos
The verification shows that both the basic usage demo (Vue Options API) and the composition API version consistently use the same default time value with seconds (18:40:20). This appears to be an intentional pattern for demo consistency, not a regression. The test file uses a simpler format (18:40) as it's focused on testing component behavior rather than demonstrating the full capabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if other time-picker demos or tests expect this precision rg -l "new Date\([^)]+, \d+\)" --type vueLength of output: 71
Script:
#!/bin/bash # Search for Date initialization patterns in all files rg "new Date\([^)]+, \d+\)" -l # Also check for any test files related to time-picker rg "time-picker.*test" -lLength of output: 3155
Script:
#!/bin/bash # Let's check the content of the time picker test file and other basic usage examples cat packages/vue/src/time-picker/__tests__/time-picker.test.tsx cat examples/sites/demos/pc/app/time-picker/basic-usage.vueLength of output: 1809
examples/sites/demos/pc/app/date-picker/unlink-panels.vue (1)
35-35
: Verify the width adjustment impact across different viewport sizes.The width increase from 280px to 360px appears to be part of a consistent style update across date picker components. However, we should ensure this fixed width works well across different viewport sizes.
Consider using responsive width values or max-width constraints to ensure better adaptability across different screen sizes. For example:
.demo-date-picker-wrap { width: min(360px, 100%); max-width: 100%; }examples/sites/demos/pc/app/time-picker/basic-usage.vue (1)
24-24
: LGTM: Time initialization change includes seconds precision.The update to include seconds (
:20
) in the initial time value aligns with the test case updates mentioned in the summary.examples/sites/demos/pc/app/date-picker/step.vue (2)
39-39
: LGTM! Width adjustment is consistent with other filesThe width increase to 360px aligns with the PR's styling optimization objectives and maintains consistency across the component demos.
Line range hint
1-41
: Verify responsive behavior with new widthPlease ensure that the increased width (360px) works well across different viewport sizes and doesn't cause layout issues in the demo site.
✅ Verification successful
Based on the search results, I can make a definitive assessment about the width change in the date picker component:
Width change is consistent with existing patterns
The codebase shows that:
- The width of 360px is already used consistently across multiple date picker demos in the codebase
- No responsive issues or media queries were found that would be affected by this width
- The width is applied through a scoped class
.demo-date-picker-wrap
which ensures it only affects the demo container🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for responsive styling or media queries related to date-picker rg -t css -t vue "demo-date-picker|tiny-date-picker" -A 5 -B 5Length of output: 92
Script:
#!/bin/bash # Search for responsive styling or media queries related to date-picker with correct file type rg "demo-date-picker|tiny-date-picker" -g "*.{css,scss,vue}" -A 5 -B 5Length of output: 274059
Script:
#!/bin/bash # Find all date picker demo files to analyze width settings fd -e vue "date-picker" examples/sites/demos/Length of output: 109
Script:
#!/bin/bash # Search for any media queries in the codebase that might affect the date picker rg "@media.*demo-date-picker" -g "*.{css,scss,vue}"Length of output: 53
examples/sites/demos/pc/app/date-picker/clear.vue (1)
43-45
: Verify the width increase doesn't cause layout issues.The width increase from 280px to 360px looks good, but let's ensure it works well across all views and responsive breakpoints.
Let's check for other components that might be affected by this width:
✅ Verification successful
Based on the search results, I can now provide a final response:
Width increase from 280px to 360px is consistent with other date picker components
The width change from 280px to 360px in the date picker's clear demo is aligned with the standard width used across all date picker demos in the codebase. The search results show that all date picker components consistently use a width of 360px in their wrapper class
demo-date-picker-wrap
, including:
- Basic usage
- Date range
- Disabled state
- Format options
- Multiple dates
- Custom weeks
- And many other date picker variants
This standardization helps maintain UI consistency across the component's various use cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other components using the same class rg "demo-date-picker-wrap" --type vue # Check for any fixed widths that might conflict rg "width:\s*\d+px" --type vueLength of output: 126
Script:
#!/bin/bash # Search for other components using the same class without file type restriction rg "demo-date-picker-wrap" # Check for any fixed widths that might conflict rg "width:\s*\d+px"Length of output: 279106
Script:
#!/bin/bash # Search for demo-date-picker-wrap class usage and its width definitions rg -A 2 -B 2 "demo-date-picker-wrap" --type less --type vue # Search for date picker related width styles in theme files rg "DatePicker|Picker" --type less | grep -i "width"Length of output: 936
Script:
#!/bin/bash # Search without file type restriction to find all demo-date-picker-wrap usages rg -A 2 -B 2 "demo-date-picker-wrap" # Search for date picker width styles in all files rg "date-picker.*width|picker.*width" -iLength of output: 78892
examples/sites/demos/pc/app/time-picker/basic-usage.spec.ts (1)
8-9
: Verify the intentional addition of seconds to time values.The test assertions now expect times with seconds (
:20
). This appears to be a behavioral change rather than just a style modification. Please confirm if this is intentional and part of the component's new design.✅ Verification successful
The seconds value (
:20
) is an intentional change in the component's default time.Found evidence in
basic-usage.vue
that the component's initial value is explicitly set to include 20 seconds:value1: new Date(2016, 9, 10, 18, 40, 20)
. The test assertions inbasic-usage.spec.ts
are correctly synchronized with this default value.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related time format changes in component files rg -l "new Date\(.*?, \d+, \d+, \d+, \d+, 20\)" --type vueLength of output: 88
Script:
#!/bin/bash # Search for the related Vue file to check time format changes fd "basic-usage.vue" --type f # After finding the file, let's examine its content for time-related changes rg "value1.*=.*new Date" -A 3Length of output: 13388
Script:
#!/bin/bash # Let's check the specific basic-usage.vue file content cat examples/sites/demos/pc/app/time-picker/basic-usage.vue # Also check for any recent changes in the time-picker component rg "new Date.*18.*40.*20" --type ts --type vueLength of output: 802
examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (1)
34-34
: LGTM! Style change aligns with PR objectives.The width increase to 360px is consistent with the component-wide style optimization effort mentioned in the PR objectives.
examples/sites/demos/pc/app/date-picker/basic-usage.vue (1)
34-34
: Width adjustment aligns with component optimization goal.The increased width provides more space for the date-picker variants, potentially improving readability and user experience.
Let's verify the consistency of this width change across other date-picker demo files:
✅ Verification successful
Width change is consistently applied across all date-picker demo files
The verification confirms that all 36 date-picker demo files in the
examples/sites/demos/pc/app/date-picker/
directory consistently use the new width value of 360px. This includes both regular and composition API variants of the components, ensuring a uniform appearance across all demo implementations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify width consistency across date-picker demo files # Expected: All demo files should use the same width (360px) # Search for width declarations in date-picker demo files rg -l "demo-date-picker-wrap" examples/sites/demos/pc/app/date-picker/ | xargs rg "width:"Length of output: 2926
examples/sites/demos/pc/app/date-picker/default-value.vue (2)
42-42
: LGTM: Width adjustment aligns with PR objectives.The width increase to 360px is consistent with the style optimization goals across the time-picker components.
Line range hint
1-46
: Verify accessibility of date picker default values.The implementation looks good, but let's ensure the default time values are accessible for all users.
Consider adding ARIA labels or tooltips to indicate the default values to screen readers. For example:
- <tiny-date-picker v-model="value" :default-value="defaultDate"></tiny-date-picker> + <tiny-date-picker + v-model="value" + :default-value="defaultDate" + aria-label="Date picker with default date set to November 10, 2000" + ></tiny-date-picker>examples/sites/demos/pc/app/date-picker/format-composition-api.vue (1)
38-38
: LGTM! Verify width consistency across other date picker demos.The width increase to 360px improves the component's visual presentation. This change aligns with the PR's objective of optimizing the time-picker component style.
Let's verify the width consistency across other date picker demos:
✅ Verification successful
Width is consistently set to 360px across all date picker demos
The verification confirms that all date picker demos in the codebase use the same width of 360px for the
.demo-date-picker-wrap
class. This change maintains consistency across the entire component suite.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if all date picker demos use the same width # Expect: All .demo-date-picker-wrap classes should use width: 360px rg -g '*.vue' 'demo-date-picker-wrap' examples/sites/demos/pc/app/date-picker/Length of output: 10863
Script:
#!/bin/bash # Let's check the width property in all date picker demos rg -g '*.vue' 'width:.*px' examples/sites/demos/pc/app/date-picker/Length of output: 3051
examples/sites/demos/pc/app/date-picker/format.vue (3)
Line range hint
1-24
: LGTM! Well-structured template with clear format demonstrations.The template effectively demonstrates three distinct use cases of date formatting:
- Basic date format
- Combined date-time format
- Timestamp value format
The examples are well-organized and properly documented with descriptive labels.
Line range hint
26-39
: LGTM! Clean and minimal script implementation.The script section provides appropriate initial values for the demonstrations and follows Vue component best practices.
46-46
: Verify consistent width changes across related files.The width change from 280px to 360px appears to be part of a broader style standardization effort.
Let's verify the consistency of this width change across other date-picker demo files:
✅ Verification successful
Width changes are consistently applied across all date-picker demo files
The verification confirms that all date-picker demo files in the examples directory consistently use the width value of 360px, showing a uniform implementation of the style change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent width values in date-picker demo files # Expected: All .demo-date-picker-wrap classes should use 360px width # Search for width definitions in date-picker demo files rg -l "demo-date-picker-wrap" examples/sites/demos/pc/app/date-picker/ | xargs rg "width:"Length of output: 2926
examples/sites/demos/pc/app/date-picker/now-composition-api.vue (1)
41-41
: LGTM! Width adjustment aligns with PR objectives.The width change to 360px is consistent with the PR-wide styling updates for date picker components.
examples/sites/demos/pc/app/date-picker/timezone-composition-api.vue (2)
48-48
: LGTM: Style change is consistentThe width increase to 360px aligns with the PR objectives and maintains consistency across other date picker components.
Line range hint
35-35
: Verify the default reference dateThe hardcoded reference date ('2020-10-28') might become outdated. Consider using a dynamic date or documenting why this specific date is used.
examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (1)
51-51
: Verify responsive behavior with increased width.The width increase from 280px to 360px aligns with the PR objectives, but ensure this change maintains responsive design principles.
Let's check for responsive design considerations:
✅ Verification successful
Based on the search results, I can see that the width change from 280px to 360px is part of a consistent pattern across all date-picker demo files. The width is standardized to 360px for date-picker components while time-picker components maintain 280px width. This appears to be an intentional design decision to maintain consistency across the component library.
Width change is part of component standardization
The increased width of 360px is consistently applied across all date-picker examples and aligns with the component's design system. The change is appropriate because:
- All date-picker demos use the same 360px width
- Time-picker demos maintain 280px width, showing intentional differentiation
- The wrapper class
.demo-date-picker-wrap
includes proper margin spacing for stacked components🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for responsive design patterns or media queries related to date-picker rg -g '*.{vue,css,scss}' -A 5 'demo-date-picker-wrap'Length of output: 89603
examples/sites/demos/pc/app/date-picker/custom-weeks.vue (2)
Line range hint
1-29
: Well-structured template with clear progressive examples!The template demonstrates a logical progression from basic to advanced usage, with clear descriptions and consistent layout.
60-60
: Verify impact of increased width across all date picker implementations.The width increase from 280px to 360px is part of a broader change. Please ensure this change:
- Is documented in the component's style guide
- Doesn't cause layout issues in existing implementations
✅ Verification successful
Width change is consistently applied across all date picker demos
The width of 360px is uniformly applied across all date picker demo files (both regular and composition API versions). This appears to be an intentional standardization, separate from time picker demos which maintain 280px width.
- All 35 date picker demo files use the consistent width of 360px
- The change is systematically applied across the entire demo suite
- Time picker demos maintain their original 280px width, showing this is a date-picker specific change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of demo-date-picker-wrap to ensure consistent width rg -l "demo-date-picker-wrap" | xargs rg "width:.*px" # Search for potential documentation files that might need updating fd -e md -e mdx | xargs rg -l "date-picker.*width"Length of output: 4950
examples/sites/demos/pc/app/date-picker/disabled.vue (1)
Line range hint
60-64
: Verify consistent width across all date picker instances.The width change to 360px appears to be part of a broader styling update. Let's verify this change is applied consistently across all instances.
✅ Verification successful
Width change is part of a consistent design system update
The change from 280px to 360px is part of a broader design system update, as evidenced by:
- All date picker demos in the codebase now use 360px width
- The width is also defined as 360px in the core theme files (packages/theme/src/picker/index.less and packages/theme-saas/src/date-picker/index.less)
- The change is consistently applied across all date picker and time picker components
The width modification is correctly implemented and aligns with the design system standards.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of 280px width in date picker related files rg -g '*.vue' -g '*.less' -g '*.css' "280px.*demo-date-picker-wrap" || echo "No remaining instances found" # Search for all date picker width definitions to ensure consistency rg -g '*.vue' -g '*.less' -g '*.css' "width:.*demo-date-picker-wrap"Length of output: 207
Script:
#!/bin/bash # Search for demo-date-picker-wrap class and its width definitions rg -g '*.vue' -g '*.less' -g '*.css' "demo-date-picker-wrap" -B 2 -A 2 # Search for any width definitions in date picker related files rg -g '*.vue' -g '*.less' -g '*.css' "width:.*360px" -B 2 -A 2Length of output: 94520
examples/sites/demos/pc/app/date-picker/size-composition-api.vue (2)
55-55
: LGTM! Width adjustment improves date picker visibility.The increase in width from 280px to 360px provides better visual space for the date picker components, especially beneficial for the datetime and daterange variants which typically require more horizontal space to display their content properly.
Line range hint
1-61
: Excellent component structure and implementation.The component follows Vue best practices:
- Clean separation of template, script, and styles
- Proper use of Composition API with reactive references
- Consistent component naming with
Tiny
prefix- Well-organized template structure with logical grouping
examples/sites/demos/pc/app/date-picker/events-composition-api.vue (2)
3-4
: LGTM: Layout improvements enhance readabilityThe column width adjustments from span-4 to span-6 provide more horizontal space for the date picker components, improving the overall layout and user experience.
Also applies to: 8-8, 12-12
5-7
: Verify date picker width consistencyOther files in this PR use a
.demo-date-picker-wrap
class with a width of 360px. Consider whether this class should be applied to the date pickers in this file for consistency.Also applies to: 9-11, 13-15, 16-18
examples/sites/demos/pc/app/date-picker/size.vue (3)
Line range hint
1-65
: LGTM! Clean and focused style updates.The changes maintain the component's functionality while optimizing its presentation. The style updates are consistent with similar changes across other files in the component library.
61-61
: Verify width consistency with design system.The width increase from 280px to 360px appears to be part of a broader style optimization effort. However, we should ensure this change:
- Aligns with the design system's specifications
- Maintains proper responsive behavior
- Won't cause layout issues in parent containers
Let's verify the width consistency across the component library:
✅ Verification successful
Width change is consistently applied across all date-picker examples
The verification shows that the 360px width has been uniformly applied across all date-picker demo files (37 files in total), including both standard and composition API versions. No remaining instances of 280px were found, indicating a complete and consistent transition to the new width.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any inconsistent widths in date-picker related files # and verify if this new width is used consistently # Search for any remaining 280px widths in date-picker related files echo "Checking for any remaining 280px widths in date-picker files:" rg -l "280px" "examples/sites/demos/pc/app/date-picker" # Search for the new 360px width to ensure consistency echo "Verifying 360px width usage:" rg -l "360px" "examples/sites/demos/pc/app/date-picker"Length of output: 2841
Line range hint
60-65
: Verify impact of removed daterange style rule.The removal of the
.tiny-date-editor--daterange.tiny-input__inner
style rule suggests a move towards using default styles. Let's ensure this doesn't cause any visual regressions.Let's check if this style rule is being handled elsewhere:
✅ Verification successful
Based on the git history and code changes, I can see that the removed style rule
.tiny-date-editor--daterange.tiny-input__inner
was part of a broader theme optimization effort. The width for date range pickers is now consistently set to 360px across the codebase, as evidenced by multiple commits that standardize the width value.The removal of this style rule is intentional and safe.
The width is now properly handled by the parent
.demo-date-picker-wrap
class withwidth: 360px
and there are no regressions since:
- The width is consistently applied across all date range picker demos
- The change was part of multiple theme optimization commits
- The style was redundant since the width is now handled at the container level
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of daterange input styles # Check if the style is defined in other locations echo "Checking for daterange input styles across the codebase:" rg "tiny-date-editor--daterange" --type css --type less --type vue # Look for any style-related comments or discussions echo "Checking for related style documentation:" rg "date.*range.*style" --type mdLength of output: 2841
Script:
#!/bin/bash # Let's try with correct file type flags and check the theme files as well echo "Checking for daterange input styles across the codebase:" rg "tiny-date-editor--daterange" -t vue -t css -t scss -t less # Check the theme files specifically echo -e "\nChecking theme files:" fd -e scss -e css -e less . packages/theme | xargs rg "tiny-date-editor--daterange" # Check the base styles for date picker echo -e "\nChecking date picker base styles:" fd "date-picker" -e scss -e css -e less . packages/theme | xargs catLength of output: 508
Script:
#!/bin/bash # Let's check the actual content of the date picker component and its styles echo "Checking date picker component styles:" fd "date-picker" -t vue -t scss -t css -t less . packages | xargs cat # Also check for any recent changes to date picker styles echo -e "\nChecking recent changes to date picker styles:" git log -p --grep="date.*picker" --grep="daterange" -i -- "*.vue" "*.scss" "*.css" "*.less"Length of output: 425308
packages/theme/src/time-panel/vars.less (1)
32-32
: Verify the header margin adjustment against design specs.The reduction of bottom margin from 24px to 0px makes the component more compact, which aligns with the optimization goals. However, please ensure this spacing adjustment:
- Matches the design system specifications
- Maintains sufficient visual separation between the header and content sections
- Doesn't cause any layout shifts in different states (empty, error, etc.)
✅ Verification successful
Header margin change is consistent with component's spacing system
The margin adjustment aligns well with the component's spacing architecture:
- The panel uses a consistent spacing system based on
--tv-space-*
tokens- Footer has negative margins to align with padding:
--tv-TimePanel-footer-margin: 0 calc(-1 * var(--tv-space-xl, 16px))
- The header's reduced bottom margin (from 24px to 0) creates a more compact layout while maintaining the top spacing with
--tv-space-xl
The change is safe as it:
- Preserves the component's spacing token system
- Maintains visual hierarchy with adequate top margin
- Aligns with other spacing variables in the component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any other margin-related changes in time-picker components # that might need to be synchronized with this change # Look for other margin declarations in time-picker related files rg --type=less --type=vue "margin|spacing" packages/theme/src/time-panel/ rg --type=less --type=vue "margin|spacing" packages/time-picker/Length of output: 198
Script:
#!/bin/bash # Search for margin-related styles in Less files rg "margin" --type=less -A 2 -B 2 packages/theme/src/time-panel/ # Search for any visual hierarchy or spacing related variables in theme files rg "TimePanel.*spacing|TimePanel.*margin" --type=less packages/theme/src/Length of output: 4174
examples/sites/demos/pc/app/date-picker/events.vue (1)
3-4
: LGTM! Layout changes improve component visibility.The increased column width (span="6") provides more space for each date-picker component, improving the overall visual presentation while maintaining a balanced layout.
Also applies to: 8-8, 12-12
examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue (2)
Line range hint
87-91
: LGTM: Style changes align with component standardizationThe width adjustment to 360px and margin spacing are consistent with the standardization effort across date picker components.
87-87
: Verify width consistency across date-picker componentsLet's ensure the width change to 360px is consistently applied across all related components.
✅ Verification successful
Width standardization is consistently applied across date-picker components
The width of 360px is uniformly applied across all date-picker components in the codebase (36 files), maintaining consistency in the component's styling. Time-picker components use a different width (280px) as they represent a different component type with distinct styling requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for date-picker width definitions across all Vue files rg -l "demo-date-picker-wrap" | xargs rg "width:\s*\d+px"Length of output: 4901
packages/theme/src/time-panel/index.less (1)
42-42
: Verify the margin transfer from header-input to header-title.The margin property has been moved from the removed
&__header-input
class to&__header-title
. This change appears to be part of the style optimization, but we should ensure it doesn't affect the component's layout negatively.✅ Verification successful
The margin change is safe and correctly implemented
The verification shows that:
- The
--tv-TimePanel-header-margin
variable is properly defined invars.less
with a value ofvar(--tv-space-xl, 16px) 2px 0 2px
- The margin is correctly applied to the
&__header-title
class which handles the layout for the header section- No references to
header-input
class exist in the codebase, confirming its complete removal- The component structure in
index.ts
and the overall layout inindex.less
maintain proper styling hierarchy🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to header-input class # to ensure complete removal and verify no layout regressions # Search for any remaining references to header-input rg -l "header-input" --type=less --type=vue # Look for related test files that might need updates fd -e spec.js -e spec.ts -e test.js -e test.ts "time-panel|time-picker"Length of output: 145
Script:
#!/bin/bash # Let's try a more comprehensive search approach # Search for header-input in less files rg -l "header-input" --type=less # Search for header-input in all files to catch any Vue templates rg -l "header-input" # Look for any files containing TimePanel to understand the component structure rg -l "TimePanel" # Search for time-panel related files fd "time-panel"Length of output: 644
Script:
#!/bin/bash # Let's examine the specific changes and related files # Check the vars.less file for the margin variable definition cat packages/theme/src/time-panel/vars.less # Look at the time panel component implementation cat packages/vue/src/time-panel/src/index.ts # Check the current content of the time-panel less file cat packages/theme/src/time-panel/index.lessLength of output: 5386
packages/theme/src/picker/index.less (2)
118-127
: Improved close icon styling and positioning.The changes to the close icon enhance its consistency and visibility through:
- Standardized 16x16px dimensions
- Better vertical centering with transform
- Improved visibility with background and z-index
176-176
: Verify line height alignment with design system.The reduction in separator line height to 20px for small-sized editors appears reasonable, but please verify this matches the design system's specifications for small variant components.
Consider documenting these metrics in the component's design tokens or variables for better maintainability and consistency across the system.
packages/vue/src/picker/src/pc.vue (1)
Line range hint
1-3
: Track technical debt: Remove tiny-date-container class.The TODO comment indicates plans to remove the
tiny-date-container
class. This should be tracked to ensure it's not forgotten.Would you like me to:
- Create a GitHub issue to track this technical debt?
- Help identify all usages of this class in the codebase?
Let's check for other usages of this class:
examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (2)
25-25
: LGTM! Improved clarity in Chinese documentation.The addition of "通过" (meaning "through") in the Chinese description makes the explanation clearer and more consistent with other demo descriptions.
38-38
: Verify English translation accuracy.While the Chinese description has been improved, the English translation appears to have formatting and grammar issues:
- Missing spaces around code tags
- Inconsistent hyphen usage
- Awkward sentence structure
Let's check for similar translation issues in other files:
Consider updating the English translation to:
- "<p><code> is-range </code> whether the setting is range selection. Default is false, <code> range-separator </code>, and the range selection delimiter (optional) defaults to'-'.</p>" + "<p>Use <code>is-range</code> to enable range selection (defaults to false). The range separator can be customized using <code>range-separator</code> (defaults to '-').</p>"
examples/sites/demos/pc/app/date-picker/custom-suffix-icon-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: 4
🧹 Outside diff range and nitpick comments (40)
examples/sites/demos/pc/app/date-picker/filter-mode-composition-api.vue (2)
Line range hint
3-4
: Consider internationalizing the label text.The hardcoded Chinese labels "日期选择" and "日期范围选择" should be internationalized to support multiple languages.
- <tiny-date-picker v-model="value" shape="filter" label="日期选择"></tiny-date-picker> - <tiny-date-picker v-model="rangeValue" type="daterange" shape="filter" label="日期范围选择"></tiny-date-picker> + <tiny-date-picker v-model="value" shape="filter" :label="t('datePicker.singleDate')"></tiny-date-picker> + <tiny-date-picker v-model="rangeValue" type="daterange" shape="filter" :label="t('datePicker.dateRange')"></tiny-date-picker>
Line range hint
18-22
: Consider adding responsive width handling.The fixed width of 360px might not be optimal for all viewport sizes. Consider using relative units or media queries for better responsiveness.
.demo-date-picker-wrap { - width: 360px; + width: min(360px, 100%); + max-width: 100%; & > * { margin-top: 12px; } }examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue (1)
18-18
: Consider adding a comment to document the width requirement.Adding a brief comment explaining why 360px was chosen would help future maintainers understand the design decision.
.demo-date-picker-wrap { + // 360px width provides optimal space for date picker content and maintains consistency across components width: 360px;
examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (1)
Line range hint
1-22
: Consider enhancing component maintainabilityWhile the implementation is functional, consider these improvements:
- Add TypeScript type definitions for the
value
prop- Include JSDoc comments explaining the component's purpose and usage
- Consider adding prop validation for the
value
fieldHere's a suggested enhancement:
<script> +/** + * @component TinyDatePicker + * @description A date picker component with custom suffix icon support + * @example + * <tiny-date-picker v-model="date" :suffix-icon="IconMinus" /> + */ import { DatePicker } from '@opentiny/vue' import { iconMinus } from '@opentiny/vue-icon' export default { components: { TinyDatePicker: DatePicker }, data() { return { - value: '', + value: '', // @type {string} Selected date value IconMinus: iconMinus() } }, + props: { + modelValue: { + type: String, + default: '' + } + } }examples/sites/demos/pc/app/date-picker/label-inside.vue (1)
Line range hint
1-31
: Consider documenting responsive behavior expectations.While the fixed width of 360px provides a consistent layout, it would be helpful to document or consider how this component behaves on smaller viewports.
Consider adding a comment in the style section to explain the width choice and any responsive considerations:
<style scoped lang="less"> .demo-date-picker-wrap { width: 360px; + /* Fixed width for optimal date picker display. + * Note: Consider using max-width for better mobile support if needed */ & > * { margin-top: 12px; } } </style>examples/sites/demos/pc/app/date-picker/filter-mode.vue (1)
Line range hint
1-31
: Consider enhancing component documentation and accessibility.While the component implementation is functionally sound, consider these improvements:
- Add comments explaining the purpose of the filter shape variant
- Include ARIA labels for better accessibility
- Document the expected date format for users
Example enhancement:
<template> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" shape="filter" label="日期选择"></tiny-date-picker> + <!-- Filter shape provides a simplified date picker interface --> + <tiny-date-picker + v-model="value" + shape="filter" + label="日期选择" + aria-label="Select single date" + placeholder="YYYY-MM-DD"> + </tiny-date-picker> <tiny-date-picker v-model="rangeValue" type="daterange" shape="filter" label="日期范围选择"></tiny-date-picker> </div> </template>examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (1)
Line range hint
23-25
: Update width to match other components.The
.demo-date-picker-wrap
width should be updated to360px
to maintain consistency with other date picker components modified in this PR.Apply this change:
.demo-date-picker-wrap { - width: 280px; + width: 360px; }examples/sites/demos/pc/app/date-picker/unlink-panels.vue (1)
Line range hint
1-37
: Well-structured demo component with clear examples.The component effectively demonstrates both linked and unlinked panel behaviors with:
- Clear section headers in Chinese
- Consistent use of v-model
- Proper spacing between examples
- Scoped styling to prevent CSS leaks
Consider adding English translations for the Chinese text to improve international developer experience.
Here's a suggested enhancement for bilingual support:
<div> - <p>默认启用面板联动:</p> + <p>默认启用面板联动 (Default: Enabled Panel Linkage):</p> <br /> <div class="demo-date-picker-wrap"> <tiny-date-picker v-model="value" type="daterange"></tiny-date-picker> </div> <br /> - <p>关闭面板联动:</p> + <p>关闭面板联动 (Disabled Panel Linkage):</p> <br />examples/sites/demos/pc/app/time-picker/basic-usage.vue (1)
Line range hint
31-33
: Update width to match other date picker examples.The current width of 280px might be causing the style issues mentioned in the PR. Based on the AI summary, other date picker examples use 360px width.
Apply this change to maintain consistency:
.demo-date-picker-wrap { - width: 280px; + width: 360px; }examples/sites/demos/pc/app/date-picker/step.vue (1)
Line range hint
1-15
: Consider enhancing demo accessibility.While the functionality is well-demonstrated, consider adding ARIA labels and descriptions to improve accessibility:
<div> - <p>步长:</p> + <p id="step-label">步长:</p> <br /> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" type="datetime" :step="step"></tiny-date-picker> + <tiny-date-picker + v-model="value" + type="datetime" + :step="step" + aria-labelledby="step-label" + aria-description="Date picker with custom step intervals: 2 hours and 5 minutes" + ></tiny-date-picker> </div>examples/sites/demos/pc/app/date-picker/clear.vue (2)
Line range hint
1-22
: Enhance accessibility of the demo sectionsConsider improving the accessibility of the demo sections by:
- Using semantic heading tags (h2, h3) instead of p tags for demo descriptions
- Adding ARIA labels to the date pickers to describe their purpose
<template> <div> - <p>默认显示清除按钮:</p> + <h2>默认显示清除按钮:</h2> <br /> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value"></tiny-date-picker> + <tiny-date-picker v-model="value" aria-label="Date picker with default clear button"></tiny-date-picker> </div> <br /> - <p>隐藏清除按钮:</p> + <h2>隐藏清除按钮:</h2> <br /> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" :clearable="false"></tiny-date-picker> + <tiny-date-picker v-model="value" :clearable="false" aria-label="Date picker without clear button"></tiny-date-picker> </div> <br /> - <p>自定义清除图标:</p> + <h2>自定义清除图标:</h2> <br /> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" :clear-icon="IconClose"></tiny-date-picker> + <tiny-date-picker v-model="value" :clear-icon="IconClose" aria-label="Date picker with custom clear icon"></tiny-date-picker> </div> </div>
Line range hint
24-39
: Consider improving the demo implementationThe current implementation has all three date pickers sharing the same v-model value, which might make it difficult to demonstrate their individual behaviors. Consider using separate values for each picker.
data() { return { - value: new Date('2023-05-24'), + value1: new Date('2023-05-24'), + value2: new Date('2023-05-24'), + value3: new Date('2023-05-24'), IconClose: iconClose() } }Then update the template bindings accordingly:
- <tiny-date-picker v-model="value"></tiny-date-picker> + <tiny-date-picker v-model="value1"></tiny-date-picker> - <tiny-date-picker v-model="value" :clearable="false"></tiny-date-picker> + <tiny-date-picker v-model="value2" :clearable="false"></tiny-date-picker> - <tiny-date-picker v-model="value" :clear-icon="IconClose"></tiny-date-picker> + <tiny-date-picker v-model="value3" :clear-icon="IconClose"></tiny-date-picker>examples/sites/demos/pc/app/time-picker/basic-usage.spec.ts (3)
8-9
: LGTM! Time values updated correctly for scroll selection test.The time values have been consistently updated to include seconds precision (
:20
), aligning with the component's new behavior.Consider adding assertions to explicitly verify the seconds value is preserved after selection:
await expect(timePicker).toHaveValue('18:42:20')
15-16
: LGTM! Time values updated correctly for arrow control test.The time values have been consistently updated to include seconds precision (
:20
), matching the component's new behavior.Consider adding assertions to explicitly verify the seconds value is preserved after arrow navigation:
await expect(timePickerArrowControl).toHaveValue('19:42:20')
Line range hint
1-21
: Consider improving test structure and documentation.While the functionality is correct, there are several opportunities for improvement:
- Add English translations for test descriptions to improve international collaboration
- Consider using test fixtures to reduce code duplication
- Add more comprehensive assertions for error cases
Here's a suggested structure:
import { test, expect } from '@playwright/test' test.describe('Time Picker - Basic Usage (时间选择器基本用法)', () => { test.beforeEach(async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('time-picker#basic-usage') }) test('should update time when scrolling time list (滚动选择时间)', async ({ page }) => { const timePicker = page.getByRole('textbox', { name: '18:42:20' }).first() await page.getByRole('textbox', { name: '18:40:20' }).first().click() await page.getByRole('listitem').filter({ hasText: '42' }).first().click() await page.getByRole('button', { name: '确定' }).click() await expect(timePicker).toBeVisible() await expect(timePicker).toHaveValue('18:42:20') }) test('should update time when using arrow controls (箭头选择时间)', async ({ page }) => { const timePickerArrowControl = page.getByRole('textbox', { name: '19:42:20' }).nth(1) await page.getByRole('textbox', { name: '18:42:20' }).nth(1).click() await page.locator('.tiny-time-spinner__wrapper > i:nth-child(2)').first().click() await page.getByRole('button', { name: '确定' }).click() await expect(timePickerArrowControl).toBeVisible() await expect(timePickerArrowControl).toHaveValue('19:42:20') }) })examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (1)
Line range hint
1-40
: Documentation update needed for UI changes.Since this PR introduces UI changes to the date-picker component, please ensure that:
- The component's documentation reflects the new width
- Any design system specifications are updated
- Breaking changes (if any) are documented
examples/sites/demos/pc/app/date-picker/format.vue (1)
Line range hint
1-24
: Consider enhancing accessibility for date picker examples.While the formatting examples are well-structured, consider adding ARIA labels and descriptions to improve accessibility.
Consider these enhancements:
<div> - <p>日期输入框中显示的格式:</p> + <p id="date-format-label">日期输入框中显示的格式:</p> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="value" format="yyyy 年 MM 月 dd 日"></tiny-date-picker> + <tiny-date-picker + v-model="value" + format="yyyy 年 MM 月 dd 日" + aria-labelledby="date-format-label" + ></tiny-date-picker> </div>examples/sites/demos/pc/app/date-picker/now-composition-api.vue (3)
Line range hint
7-9
: Enhance accessibility and internationalization.Consider the following improvements:
- Use a
<button>
element instead of a<div>
for the "now" action- Add proper ARIA labels
- Extract the Chinese text to i18n translations
- <div class="nowclass" @click="now">此刻(服务器时间)</div> + <button + class="nowclass" + @click="now" + aria-label="Set to current server time" + > + {{ t('datePicker.setToServerTime') }} + </button>
Line range hint
24-36
: Improve date handling consistency and maintainability.The current implementation has several potential issues:
- Hardcoded dates in both methods make maintenance difficult
- Inconsistent date formats between methods
- Unnecessary setTimeout(0)
const nowClick = () => { - return new Promise((resolve) => { - setTimeout(() => { - resolve(new Date(2024, 9, 8, 18, 18, 18)) - }, 0) - }) + return Promise.resolve(new Date()) } const now = () => { - value1.value = '2018-12-11 18:18:18' + value1.value = new Date().toISOString().slice(0, 19).replace('T', ' ') }
Line range hint
41-47
: Enhance style maintainability and user experience.Consider the following improvements:
- Use design system variables for colors
- Add hover/focus states for better user experience
.demo-date-picker { width: 360px; } .nowclass { display: inline-flex; height: 28px; align-items: center; - color: #0067d1; + color: var(--ti-date-picker-primary-color); cursor: pointer; + &:hover { + color: var(--ti-date-picker-primary-hover-color); + } + &:focus { + outline: 2px solid var(--ti-date-picker-focus-ring-color); + outline-offset: 2px; + } }examples/sites/demos/pc/app/date-picker/timezone-composition-api.vue (1)
48-48
: LGTM! Consider using CSS custom property for better maintainability.The width increase to 360px provides better visual space for the date picker component, especially when displaying timezone information. This change is consistently applied across multiple date picker components, showing good attention to design consistency.
Consider defining the width as a CSS custom property for easier maintenance across all date picker components:
+:root { + --tiny-date-picker-width: 360px; +} + .demo-date-picker-wrap { - width: 360px; + width: var(--tiny-date-picker-width); }examples/sites/demos/pc/app/date-picker/now.vue (3)
Line range hint
3-17
: Consider implementing internationalization for text content.The template contains hardcoded Chinese text that should be internationalized for better maintainability and multi-language support.
Example implementation:
- <div class="title">插槽式:</div> + <div class="title">{{ t('datePicker.slotMode') }}</div> - <div class="title">函数式:</div> + <div class="title">{{ t('datePicker.functionMode') }}</div> - <div class="nowclass" @click="now">此刻(服务器时间)</div> + <div class="nowclass" @click="now">{{ t('datePicker.currentServerTime') }}</div>
Line range hint
32-34
: Replace static date with actual current time innow()
method.The current implementation sets a fixed date which defeats the purpose of a "now" button.
now() { - this.value1 = '2018-12-11 18:18:18' + const now = new Date() + this.value1 = now.toISOString().slice(0, 19).replace('T', ' ') }
Line range hint
35-41
: SimplifynowClick()
implementation and use actual current time.The current implementation has unnecessary setTimeout and uses a hardcoded future date.
nowClick() { - return new Promise((resolve) => { - setTimeout(() => { - resolve(new Date(2024, 9, 8, 18, 18, 18)) - }, 0) - }) + return Promise.resolve(new Date()) }examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (2)
Line range hint
1-40
: Consider improving accessibility with English translations and ARIA labels.The template structure is well-organized, but could benefit from accessibility improvements:
- Add English translations or i18n support for the Chinese text
- Include ARIA labels for screen readers
Example improvement:
- <p>默认隐藏周次序号:</p> + <p aria-label="Hide week number by default">默认隐藏周次序号:</p>
Line range hint
41-49
: Consider making the week formatting logic more pure and predictable.The
formatWeeks
function both mutates state and returns a value, which can make the code harder to test and maintain.Consider separating the state update from the formatting:
-function formatWeeks(customWeeks, weekFirstDays) { - eachWeekFirstDay.value = weekFirstDays - return customWeeks + 'w' +function formatWeeks(customWeeks, weekFirstDays) { + // Update state via watcher or effect + updateWeekFirstDays(weekFirstDays) + return customWeeks + 'w' +} + +function updateWeekFirstDays(weekFirstDays) { + eachWeekFirstDay.value = weekFirstDays }examples/sites/demos/pc/app/date-picker/timezone.vue (2)
Line range hint
6-13
: Consider documenting timezone behavior.While the implementation is correct, it would be helpful to add comments explaining:
- The purpose of
default-timezone="Etc/GMT+12"
- The format expected by
timezone-data
- The structure of the data emitted by
select-change
Example documentation:
<div class="demo-date-picker-wrap"> + <!-- Timezone picker with GMT+12 as default. + timezone-data provides the list of available timezones + select-change emits the selected timezone object { tz: { name: string, ... } } + --> <tiny-date-picker
Line range hint
39-41
: Consider enhancing error handling in selectChange.The current implementation shows a modal message but doesn't handle potential edge cases:
- When tz.tz is undefined
- When the timezone selection fails
selectChange(tz) { - Modal.message({ message: `当前值为 ${tz.tz.name}`, status: 'info' }) + if (tz?.tz?.name) { + Modal.message({ message: `当前值为 ${tz.tz.name}`, status: 'info' }) + } else { + Modal.message({ message: '时区选择无效', status: 'error' }) + } }Also applies to: 44-46
examples/sites/demos/pc/app/date-picker/custom-weeks.vue (2)
60-60
: LGTM! Consider adding responsive width handling.The width increase to 360px aligns with the standardized width across other date picker components. However, consider making the width responsive to accommodate different screen sizes.
Consider using this responsive approach:
-.demo-date-picker-wrap { - width: 360px; -} +.demo-date-picker-wrap { + width: 100%; + max-width: 360px; + min-width: 280px; +}
60-60
: Document the standardized width in design system.Since this 360px width is being standardized across multiple date picker components, it would be beneficial to document this in your design system documentation.
Consider adding a CSS custom property for reusability:
+:root { + --tiny-date-picker-width: 360px; +} + .demo-date-picker-wrap { - width: 360px; + width: var(--tiny-date-picker-width); }examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue (1)
Line range hint
1-22
: Consider enhancing accessibility with ARIA labels.While the template structure is clean and well-organized, consider adding ARIA labels to improve accessibility for screen readers.
- <p>整体禁用:</p> + <p id="disabled-label">整体禁用:</p> <div class="demo-date-picker-wrap"> - <tiny-date-picker v-model="disabledValue" disabled placeholder="请选择日期"></tiny-date-picker> + <tiny-date-picker v-model="disabledValue" disabled placeholder="请选择日期" aria-labelledby="disabled-label"></tiny-date-picker> </div>examples/sites/demos/pc/app/date-picker/disabled.vue (1)
Line range hint
1-58
: Consider enhancing accessibility and user feedback.While the component demonstrates various disabled states effectively, consider these improvements:
- Add
aria-label
attributes to provide better screen reader support- Include helper text to explain why dates are disabled when using
pickerOptions
- Consider adding validation messages for disabled dates
Example enhancement for the partially disabled date picker:
<tiny-date-picker v-model="value" :picker-options="pickerOptions" - placeholder="请选择日期" + placeholder="请选择日期" + aria-label="选择日期 - 仅可选择2023-08-01之后的日期" + :title="'只能选择2023-08-01之后的日期'" ></tiny-date-picker>examples/sites/demos/pc/app/date-picker/size-composition-api.vue (1)
Line range hint
3-9
: Consider improving accessibility and internationalization.While not directly related to the style changes, consider these improvements:
- Add an aria-label to the radio group for better screen reader support
- Use i18n for placeholder texts to support internationalization
Example improvement:
- <tiny-radio-group v-model="radioValue" size="mini" class="demo-date-picker__switch-size"> + <tiny-radio-group + v-model="radioValue" + size="mini" + class="demo-date-picker__switch-size" + aria-label="Select date picker size">Also applies to: 10-35
examples/sites/demos/pc/app/date-picker/validate-event.vue (1)
Line range hint
10-17
: Consider enabling immediate validation feedbackCurrently, both date and time pickers have
:validate-event="false"
, which defers validation until form submission. Consider enabling validate-event for immediate user feedback, especially since the validation rules are already well-defined.<tiny-date-picker - :validate-event="false" type="date" placeholder="选择日期" v-model="ruleForm.date1" ></tiny-date-picker> </tiny-form-item> <tiny-form-item prop="date2"> <tiny-time-picker - :validate-event="false" placeholder="选择时间" v-model="ruleForm.date2"> </tiny-time-picker>examples/sites/demos/pc/app/time-picker/format.spec.ts (2)
Line range hint
1-54
: Consider improving test maintainability.While the current implementation works, there are several opportunities for improvement:
- Inconsistent selector patterns: Some use
.first()
while others don't- Varying wait times: Mix of 100ms and 200ms waits
- Repeated selectors and text that could be constants
Consider applying these improvements:
import { test, expect } from '@playwright/test' + +const WAIT_TIME = 100 +const SELECTORS = { + confirmButton: 'button[name="确定"]', + hourList: '.tiny-scrollbar >> nth=0', + minuteList: '.tiny-scrollbar >> nth=1', + secondList: '.tiny-scrollbar >> nth=2' +} test('时间格式化', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('time-picker#format') // format: 时间输入框中显示的格式 await page.getByRole('textbox', { name: '06:40:00 pm' }).click() await page.getByText('07 pm').click() - await page.getByRole('button', { name: '确定' }).click() + await page.locator(SELECTORS.confirmButton).click() await expect(page.getByRole('textbox', { name: '07:40:00 pm' })).toBeVisible() await page.getByRole('textbox', { name: '19:40:00 pm' }).click() - await page.waitForTimeout(100) + await page.waitForTimeout(WAIT_TIME)
Line range hint
21-24
: Consider using test.step() for better test organization.The test has clear logical sections testing different formats. Using
test.step()
would improve readability and test reporting.Example refactor:
test('时间格式化', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('time-picker#format') await test.step('format: 时间输入框中显示的格式', async () => { await page.getByRole('textbox', { name: '06:40:00 pm' }).click() await page.getByText('07 pm').click() await page.getByRole('button', { name: '确定' }).click() await expect(page.getByRole('textbox', { name: '07:40:00 pm' })).toBeVisible() }) // Add similar steps for other format tests })Also applies to: 29-32, 36-39
packages/theme/src/time-panel/index.less (1)
Line range hint
89-89
: Consider addressing the TODO comment.While working on component optimizations, it would be good to clarify whether the borderless scenario is still needed. If not, we can remove this comment and potentially simplify the related styles.
Would you like me to help investigate the usage of borderless time panels in the codebase or create an issue to track this cleanup task?
packages/theme/src/picker/index.less (2)
118-127
: LGTM! Consider documenting z-index values.The close icon modifications improve consistency with:
- Uniform 16x16px dimensions
- Better vertical centering using transform
- Proper spacing from edges
Consider maintaining a z-index scale in your documentation to track the hierarchy of different UI layers. This helps prevent z-index wars and makes it easier to manage layering.
Line range hint
259-259
: Consider evaluating the scrollbar implementation.There's a TODO comment indicating that the scrollbar implementation needs evaluation. Consider creating a task to assess if this code is still needed.
Would you like me to create a GitHub issue to track the scrollbar evaluation task?
examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (1)
Line range hint
1-190
: Consider standardizing documentation format across all demos.While the changes improve clarity for some demos, there's an opportunity to standardize the documentation format across all demos. Several demos could benefit from the same clear parameter explanation style:
- Some demos use "通过" consistently while others don't
- Some demos mention default values while others don't
- Some demos have detailed explanations while others are very brief
Consider updating other demos to follow the same improved documentation pattern. For example:
- Add default values where missing
- Standardize the usage of "通过" across all Chinese descriptions
- Ensure consistent level of detail in parameter explanations
Would you like me to provide specific suggestions for standardizing the documentation format across all demos?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (49)
- examples/sites/demos/pc/app/date-picker/align-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/align.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/basic-usage.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/clear-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/clear.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-suffix-icon-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/custom-weeks.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/default-value.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/disabled.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/events-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/date-picker/events.vue (2 hunks)
- examples/sites/demos/pc/app/date-picker/filter-mode-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/filter-mode.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/format-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/format.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/label-inside-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/label-inside.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/multiple-dates.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/now-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/now.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/size-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/size.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/step-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/step.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/timezone-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/timezone.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/unlink-panels-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/unlink-panels.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/date-picker/validate-event.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/basic-usage.spec.ts (1 hunks)
- examples/sites/demos/pc/app/time-picker/basic-usage.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/format.spec.ts (1 hunks)
- examples/sites/demos/pc/app/time-picker/is-range-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/is-range.vue (1 hunks)
- examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (2 hunks)
- packages/theme/src/picker/index.less (2 hunks)
- packages/theme/src/time-panel/index.less (1 hunks)
- packages/theme/src/time-panel/vars.less (1 hunks)
- packages/theme/src/time-spinner/index.less (2 hunks)
- packages/vue/src/picker/src/pc.vue (1 hunks)
- packages/vue/src/time/src/pc.vue (0 hunks)
💤 Files with no reviewable changes (1)
- packages/vue/src/time/src/pc.vue
✅ Files skipped from review due to trivial changes (11)
- examples/sites/demos/pc/app/date-picker/align.vue
- examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue
- examples/sites/demos/pc/app/date-picker/basic-usage.vue
- examples/sites/demos/pc/app/date-picker/clear-composition-api.vue
- examples/sites/demos/pc/app/date-picker/custom-suffix-icon-composition-api.vue
- examples/sites/demos/pc/app/date-picker/format-composition-api.vue
- examples/sites/demos/pc/app/date-picker/multiple-dates.vue
- examples/sites/demos/pc/app/date-picker/step-composition-api.vue
- examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue
- examples/sites/demos/pc/app/time-picker/is-range-composition-api.vue
- examples/sites/demos/pc/app/time-picker/is-range.vue
🔇 Additional comments (40)
examples/sites/demos/pc/app/date-picker/label-inside-composition-api.vue (2)
Line range hint
16-23
: Verify the impact of removed deep selectors.The removal of deep selectors
:deep(.tiny-date-editor--daterange.tiny-input__inner)
and:deep(.tiny-range__close-icon)
suggests style simplification. Let's verify this doesn't affect the component's appearance.#!/bin/bash # Description: Check if the removed deep selectors are handled elsewhere # Expected: These styles should either be unnecessary or handled in a central location # Search for similar deep selector patterns across the codebase rg -g '*.vue' ':deep\(.*(tiny-date-editor--daterange|tiny-range__close-icon)'
18-18
: Verify consistent width across date picker components.The width increase to 360px improves the layout, but let's ensure this change is consistent across all date picker components.
✅ Verification successful
Width change to 360px is consistent across all date picker demos
Based on the search results, all date picker demo files consistently use the width of 360px for the
.demo-date-picker-wrap
class. This includes various types of date pickers (single date, date range, datetime, etc.) and different demo scenarios (basic usage, disabled state, alignment variations, etc.).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify width consistency across date picker demo files # Expected: All date picker demos should use the same width value # Search for width declarations in date-picker demo files rg -g '*/date-picker/*.vue' 'width:.*px' examples/sites/demos/pc/app/Length of output: 79
Script:
#!/bin/bash # Let's try a different approach to search for width declarations in date picker files # Using ripgrep with more specific patterns and showing context # Search for style blocks in date picker demo files rg -U --type vue -g '*date-picker/*.vue' '<style.*>[\s\S]*?</style>' examples/sites/demos/pc/app/ # Also search specifically for the demo-date-picker-wrap class rg -U --type vue -g '*date-picker/*.vue' '\.demo-date-picker-wrap\s*{[^}]*}' examples/sites/demos/pc/app/Length of output: 378
Script:
#!/bin/bash # Let's try a simpler approach with basic pattern matching # Search for width declarations in all .vue files under date-picker directory # First, find all .vue files in date-picker directory fd '.vue$' examples/sites/demos/pc/app/date-picker/ -x cat {} | grep -A 5 -B 5 'width:' # Also search for the specific class name fd '.vue$' examples/sites/demos/pc/app/date-picker/ -x cat {} | grep -A 5 -B 5 'demo-date-picker-wrap'Length of output: 28883
examples/sites/demos/pc/app/date-picker/filter-mode-composition-api.vue (1)
18-18
: LGTM! Width adjustment is consistent with other examples.The increase in width from 280px to 360px aligns with the PR's objective to optimize the time-picker component styling and maintains consistency across multiple example files.
examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue (1)
18-18
: LGTM! Width adjustment aligns with component-wide styling updates.The increased width to 360px is consistent with similar changes across other date picker components, providing a more uniform and improved visual presentation.
examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (1)
26-26
: LGTM: Style improvements enhance visual consistencyThe increased width and refined background color contribute to better visual presentation while maintaining consistency across the date picker components.
Also applies to: 30-30
examples/sites/demos/pc/app/date-picker/label-inside.vue (1)
26-26
: Verify width consistency across all date picker implementations.The width increase to 360px aligns with the broader style optimization effort across date picker components.
Let's verify this change is consistent across other date picker implementations:
✅ Verification successful
Width change is consistently implemented across all date picker demos
The verification confirms that:
- All date picker demos (
examples/sites/demos/pc/app/date-picker/*.vue
) consistently usewidth: 360px
- Time picker demos maintain their original
width: 280px
- The width change is uniformly applied across both regular and composition API variants
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent width implementation across date picker components # Expected: All date picker demos should use 360px width # Search for width declarations in date picker demo files rg -g '*.vue' -l 'demo-date-picker-wrap' | xargs rg 'width:'Length of output: 4902
examples/sites/demos/pc/app/date-picker/filter-mode.vue (1)
26-26
: LGTM: Width adjustment aligns with standardization effort.The width increase to 360px is consistent with the coordinated updates across other date picker components, improving visual consistency.
examples/sites/demos/pc/app/date-picker/unlink-panels-composition-api.vue (1)
27-27
: LGTM! Width adjustment improves component consistency.The increase in width to 360px aligns with the standardization effort across other date picker components, providing better visual presentation and more space for date range display.
examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (2)
Line range hint
1-12
: LGTM: Template structure is well-organized.The template demonstrates two time-picker variants (scroll and arrow control) effectively. The shared v-model appropriately demonstrates synchronized updates between the two instances.
19-19
: Verify time-picker's seconds display behavior.The initialization now includes seconds (
:20
). Please verify that the time-picker component correctly displays and handles seconds, as this might affect the user experience if seconds are not visible in the UI.✅ Verification successful
Let me check the content of these files to understand how seconds are handled in the time-picker component.
Based on the documentation, I can now provide a final response since I have found sufficient evidence about seconds handling in the time-picker component:
Time-picker fully supports seconds display and handling
The component has built-in support for seconds through:
- Format tokens
s
(seconds without leading zero) andss
(seconds with leading zero)- Step configuration via
step.second
prop (range 0-59)- Default step of 1 second when not configured
The initialization with seconds (
:20
) is perfectly valid and will work as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for time-picker props or documentation related to seconds display rg -l "seconds?" $(fd -t f "time-picker.(vue|md|js|ts)")Length of output: 264
Script:
#!/bin/bash # Check the content of time-picker documentation and API files echo "=== Time Picker API Documentation ===" cat examples/sites/demos/apis/time-picker.js echo -e "\n=== Time Picker Web Documentation ===" cat examples/sites/demos/pc/app/time-picker/webdoc/time-picker.jsLength of output: 18448
examples/sites/demos/pc/app/date-picker/unlink-panels.vue (1)
35-35
: Width adjustment aligns with PR objectives.The increase in width from 280px to 360px provides more space for the date picker component, which should improve the overall user experience and readability.
examples/sites/demos/pc/app/date-picker/step.vue (2)
39-39
: LGTM: Width standardization aligns with PR objectives.The width increase from 280px to 360px is consistent with the PR's goal of optimizing the time-picker component style. This change appears to be part of a broader standardization effort across multiple demo files.
Line range hint
1-41
: Verify the impact of width changes on responsive layouts.The width increase might affect the component's behavior in different viewport sizes.
examples/sites/demos/pc/app/date-picker/align-composition-api.vue (1)
34-34
: Consider responsive design implications of fixed width.While the width increase to 360px improves the date picker's visual presentation, using a fixed width might cause issues on smaller screens or when the component is used in narrower containers.
Consider these improvements:
- Use max-width instead of width to handle narrow viewports
- Add media queries for mobile devices
- Consider using relative units (e.g., rem) for better scaling
.demo-date-picker-wrap { - width: 360px; + max-width: 360px; + width: 100%; }Let's verify the parent container widths where this component is used:
examples/sites/demos/pc/app/date-picker/clear.vue (1)
44-44
: LGTM: Width adjustment aligns with style optimization goalsThe increase in width from 280px to 360px provides better layout for the date picker component. This change is consistently applied across multiple files in the PR, maintaining UI consistency.
examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (1)
34-34
: LGTM! Verify the width change impact.The width increase to 360px aligns with the PR's objective to optimize the time-picker component style. This change appears consistent across multiple files.
Let's verify the consistency of this width change across other date-picker components:
✅ Verification successful
Width change is consistent across all date-picker components
The width of 360px is consistently applied across all date-picker demo components in the
.demo-date-picker-wrap
class, confirming that this is an intentional, systematic change that maintains UI consistency throughout the component library.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify width consistency across date-picker components # Expected: All date-picker demos should use the same width of 360px # Search for width declarations in date-picker demo files rg -g '*/date-picker/*.vue' 'width:.*px' examples/sites/demos/pc/app/Length of output: 71
Script:
#!/bin/bash # Let's try a different approach to search for width styles in date-picker components # First, let's find all date-picker vue files echo "Date picker vue files:" fd -e vue . examples/sites/demos/pc/app/date-picker/ echo -e "\nChecking style sections for width declarations:" # Now let's look for style sections with width declarations rg -U "(?s)<style.*?>.*?width:.*?px.*?</style>" examples/sites/demos/pc/app/date-picker/Length of output: 27673
examples/sites/demos/pc/app/date-picker/default-value.vue (1)
42-42
: LGTM! Width adjustment improves date picker visibility.The increase to 360px provides better space for displaying date-time ranges and maintains consistency across all date picker demos.
examples/sites/demos/pc/app/date-picker/format.vue (1)
46-46
: Width increase improves readability of formatted dates.The increased width (360px) provides better accommodation for the formatted date strings, especially for the datetime picker showing full date and time in Chinese format.
Let's verify if this width is consistent with the design system:
✅ Verification successful
Width change is consistent across the design system
The width of 360px is consistently used across:
- The main theme package (
packages/theme/src/picker/index.less
)- All date picker demo components
- The SAAS theme package (
packages/theme-saas/src/date-picker/index.less
)This change aligns with the established design system patterns and maintains visual consistency throughout the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any design tokens or other width definitions for date pickers # Look for similar width definitions in style files and documentation # Search for width definitions in style files rg -g '*.{vue,less,css}' '(width:|width=).*360' # Search for design token definitions rg -g '*.{js,json,ts}' '".*width.*": ?"360'Length of output: 3509
examples/sites/demos/pc/app/date-picker/now-composition-api.vue (1)
41-41
: Verify layout impact of width change.The width increase from 280px to 360px might affect parent layouts. Please ensure this change doesn't cause unwanted layout shifts or overflow issues in various viewport sizes.
✅ Verification successful
The previous script failed because the
--type vue
flag wasn't supported. Let's try a broader search to find any references to this component or its usage.
The search results show this is a demo component in an examples directory. Let's examine the actual component and its context to better understand the width change's impact.
Width change is safe in this context
The width change from 280px to 360px is safe because:
- This is a demo component in an examples directory
- The CSS class
.demo-date-picker
is scoped to this component- The width is applied to a demo wrapper div, not the actual DatePicker component
- All similar demo files in the same directory follow the same pattern for consistent demo presentation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for parent components that might be affected by the width change rg -l "demo-date-picker" --type vueLength of output: 63
Script:
#!/bin/bash # Search for files containing "demo-date-picker" in any text file rg -l "demo-date-picker" # Also search for any file that might import or use this component rg -l "now-composition-api"Length of output: 4898
Script:
#!/bin/bash # Let's examine the content of the now-composition-api.vue file to understand its context cat "examples/sites/demos/pc/app/date-picker/now-composition-api.vue" # Also check its parent directory for any layout-related files ls -la "examples/sites/demos/pc/app/date-picker/"Length of output: 5743
examples/sites/demos/pc/app/date-picker/now.vue (1)
49-49
: LGTM! Width adjustment improves component layout.The increased width provides better spacing for the date picker component.
examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (1)
50-52
: Verify layout compatibility with the increased width.The width increase from 280px to 360px aligns with the standardization across other files, but ensure this doesn't cause layout issues in constrained containers.
Let's check for potential layout issues:
✅ Verification successful
Let me check the actual width values in these files to understand if there are any potential conflicts.
Let me check if there are any other date picker components that might be affected by this width change.
Width change aligns with existing standardization
The width increase from 280px to 360px in
custom-weeks-composition-api.vue
matches the standardized width already used across all other date picker components in the codebase. All date picker demos consistently use a width of 360px for their wrapper class.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for parent containers that might be affected by the width change rg -l "demo-date-picker-wrap" | xargs rg -l "width:\s*(2[8-9][0-9]|[1-2][0-9][0-9]|[1-9][0-9]|[1-9])px"Length of output: 1693
Script:
#!/bin/bash # Check the actual width values in the matched files rg -A 2 -B 2 "demo-date-picker-wrap" examples/sites/demos/pc/app/time-picker/*.vueLength of output: 30028
Script:
#!/bin/bash # Search for other date picker components using the same class rg -A 2 -B 2 "demo-date-picker-wrap" examples/sites/demos/pc/app/date-picker/Length of output: 49100
examples/sites/demos/pc/app/date-picker/timezone.vue (2)
58-58
: LGTM: Style width adjustment is consistent with other files.The width increase to 360px aligns with the broader standardization effort across date picker components, providing better visual presentation.
Line range hint
21-24
: Verify UTC+8 conversion behavior.The
:isutc8="true"
prop suggests automatic conversion to UTC+8, but this behavior should be verified.✅ Verification successful
Based on the comprehensive search results, I can now provide a final response about the UTC+8 conversion behavior:
The isutc8 prop correctly enforces UTC+8 timezone display
The implementation shows that:
- The
isutc8
prop is properly defined in the date picker component with a default value of false- When
isutc8
is true, the timezone conversion logic ingetTimezone()
function (packages/renderless/src/picker/index.ts) explicitly sets the target timezone to 8 (UTC+8)- The conversion is handled by the
getDateWithNewTimezone()
utility which properly adjusts the date/time values between timezones- The feature is documented in the API docs as: "displays time in GMT+8 when the default timezone is switched, applicable for showing GMT+8 time in regions outside China"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of isutc8 usage and its implementation rg -l "isutc8" --type vue ast-grep --pattern 'props: { $$$ isutc8: $_, $$$ }'Length of output: 113
Script:
#!/bin/bash # Search for isutc8 usage with ripgrep, trying different file extensions rg "isutc8" -A 5 -B 5 # Also search for timezone-related props and implementations rg "timezone" -A 5 -B 5Length of output: 61094
examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue (1)
53-53
: LGTM! Consistent width adjustment across components.The width increase to 360px aligns with the PR's objective to optimize the time-picker component's style and maintains consistency across multiple files.
examples/sites/demos/pc/app/date-picker/disabled.vue (1)
60-63
: LGTM! Style changes improve component layout.The increased width from 280px to 360px provides better spacing for the date picker components, especially beneficial for the daterange picker which needs more space to display two dates.
examples/sites/demos/pc/app/date-picker/size-composition-api.vue (1)
55-55
: LGTM! Width adjustment improves date picker visibility.The increased width provides better visual space for all date picker variants, especially beneficial for the daterange type. The simplified styling approach with a consistent width is a good improvement.
examples/sites/demos/pc/app/date-picker/events-composition-api.vue (2)
3-4
: LGTM: Layout optimization improves component spacingThe column width increase from span-4 to span-6 provides better spacing for the date picker components, making them more visually balanced in the layout.
Also applies to: 8-8, 12-12
59-61
: Consider responsive design and class namingWhile the 80% width constraint helps with layout, please verify:
- The layout remains usable on smaller screens
- Consider using a more semantic class name instead of "demo-row" if this styling is intended for production use
Let's verify the responsive behavior:
✅ Verification successful
Based on the search results, I can now generate the final response:
The 80% width constraint follows existing patterns in the codebase
The width constraint of 80% is consistently used across multiple demo components including other date-picker examples, card components, and dialog boxes. This appears to be an established pattern for demo layouts. The codebase also implements proper responsive design with multiple breakpoints (1279px, 1023px, 767px), suggesting the layout system is well-considered.
- The class name "demo-row" is appropriate since this is indeed within the demos directory
- The 80% width is consistent with other demo components in:
- date-picker/events.vue
- card/card-select.vue
- dialog-box examples
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any existing responsive design patterns or media queries rg -g '*.vue' -g '*.css' '@media' --type-add 'vue:*.vue' # Check if similar width constraints are used elsewhere rg -g '*.vue' -g '*.css' 'width:\s*80%' --type-add 'vue:*.vue'Length of output: 2675
examples/sites/demos/pc/app/date-picker/size.vue (2)
Line range hint
1-65
: Verify relevance to time-picker optimization.The PR title mentions time-picker optimization, but this file is modifying date-picker styles. Please clarify if this change is intentionally part of a broader picker component refactoring.
Let's check for related time-picker changes:
✅ Verification successful
Changes are part of a broader picker component refactoring
Based on the git diff output, this is clearly part of a larger refactoring effort that touches both date-picker and time-picker components. The changes include:
- Multiple date-picker demo files being updated
- Time-picker related changes (basic-usage, format, is-range files)
- Core picker component changes in
packages/theme/src/picker/
andpackages/vue/src/picker/
- Time panel and spinner component modifications
The size.vue changes are consistent with this broader refactoring effort that affects the entire picker component family.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for related time-picker files that might be affected fd -e vue -e less "time-picker" examples/sites/demos/pc/app/Length of output: 154
Script:
#!/bin/bash # Look for related time-picker files and changes rg -l "time-picker" --type vue --type less examples/sites/demos/pc/app/ # Check PR title and description for context gh pr view --json title,body # Look for any related changes in the component files rg -l "TimePicker|time-picker" src/Length of output: 265
Script:
#!/bin/bash # Look for time-picker related files in the entire repository fd "time-picker|timepicker" -I # Look for any changes in the current PR git diff HEAD~1 --name-onlyLength of output: 3932
61-61
: Width change looks good, but verify responsive behavior.The increased width provides better spacing for date picker components, especially for date range selections. However, we should ensure this change doesn't cause layout issues on smaller screens.
Let's check if other demo files have responsive handling:
✅ Verification successful
Width change is consistent and safe to use
The width change to 360px is consistently applied across all date-picker demo files and is part of a coordinated update. The change:
- Is applied uniformly in all 30+ date-picker demo files
- Uses scoped styles, preventing any global style leaks
- Doesn't require responsive handling as these are controlled demo environments
The relationship with time-picker appears to be part of a broader component style optimization, as both components are located under the same demo structure (
examples/sites/demos/pc/app/
).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for responsive-related styles or media queries in demo files rg -t less "(@media|responsive|mobile)" "examples/sites/demos/pc/app/date-picker/"Length of output: 154
Script:
#!/bin/bash # Let's try a different approach to check responsive styles # 1. Check all style sections in date-picker demo files rg -U "(?s)<style.*?>.*?</style>" "examples/sites/demos/pc/app/date-picker/" # 2. Also check if there are any shared/common style files for date-picker fd -e less -e css "date-picker" "examples" # 3. Check relationship between date-picker and time-picker components fd "time-picker|date-picker" "examples/sites/demos/pc/app/"Length of output: 25513
packages/theme/src/time-panel/vars.less (2)
Line range hint
1-53
: LGTM! Well-structured variables with clear documentation.The file maintains a clean organization with:
- Consistent variable naming convention
- Clear grouping of related properties
- Helpful comments for each variable
32-32
: Verify the visual impact of reduced header margin.The change reduces the bottom margin of the time panel header from 24px to 0px, which could affect the spacing between the header and content. While this aligns with the PR's goal to optimize component styles, we should ensure this doesn't create any visual crowding issues.
Consider the following:
- Verify that this change works well across all time-picker variants
- Ensure sufficient visual separation between header and content is maintained
- Test the appearance with different locales/text lengths
✅ Verification successful
Margin adjustment is safe and aligned with component design
The reduced bottom margin in the time panel header is a safe change based on the following findings:
The time panel has proper spacing through other CSS properties:
- The panel itself has padding (
--tv-TimePanel-padding
)- The content area has bottom padding (
--tv-TimePanel-content-padding-bottom
)- The footer has its own margin and padding variables
The header's new margin
var(--tv-space-xl, 16px) 2px 0 2px
maintains proper top spacing while removing redundant bottom spacing, which is consistent with the component's visual hierarchy.The component is used consistently across both PC and mobile implementations, and the styling follows the established design system with proper spacing variables.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all time-picker related test or demo files to verify visual impact fd -e vue -e jsx -e tsx . | rg -i "time.*picker|picker.*time"Length of output: 2673
Script:
#!/bin/bash # Let's examine the basic usage demo and test files to understand the visual structure and any potential issues cat packages/vue/src/time-picker/__tests__/time-picker.test.tsx examples/sites/demos/pc/app/time-picker/basic-usage.vue # Also check if there are any related style changes or issues reported rg -i "timepanel.*header|header.*margin" -g "*.{less,css,scss}"Length of output: 4492
Script:
#!/bin/bash # Let's check the time-panel component's structure and styling cat packages/theme/src/time-panel/index.less # Also examine if there are any related components that might be affected rg -g "*.{vue,tsx}" -B 2 -A 2 "TimePanel"Length of output: 4660
examples/sites/demos/pc/app/date-picker/events.vue (1)
62-64
: Verify responsive behavior with the new width constraint.The new
.demo-row
class sets a fixed 80% width. While this helps control the layout, please verify that:
- The date pickers remain usable at different viewport sizes
- There's no conflict with TinyLayout's built-in responsive behavior
Let's check if this width constraint is consistently applied across other date-picker demos:
✅ Verification successful
Let me check the content of these files to understand how the
demo-row
class is used across them.
Let me check how this component integrates with TinyLayout by examining the grid system implementation.
The width constraint is consistently implemented and works with the grid system
The
.demo-row
class with 80% width is consistently applied in both the standard and composition API versions of the date-picker events demo. The class is scoped and works in conjunction with Tiny's grid system (tiny-layout
,tiny-row
,tiny-col
) where columns are properly defined with:span="6"
. This implementation:
- Maintains consistency across demo variants
- Works with the grid system's responsive behavior as it:
- Uses relative width (percentage)
- Preserves the column spans within the row
- Is wrapped in
tiny-layout
which handles responsive margins/padding🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar width constraints in other date-picker demos rg -l "demo-row" "examples/sites/demos/pc/app/date-picker/"Length of output: 175
Script:
#!/bin/bash # Check the content and context of demo-row class in both files rg -C 5 "demo-row" "examples/sites/demos/pc/app/date-picker/events.vue" "examples/sites/demos/pc/app/date-picker/events-composition-api.vue"Length of output: 3205
Script:
#!/bin/bash # Check the TinyLayout and grid system implementation rg -l "tiny-row|tiny-col" "examples/sites/demos/pc/app/date-picker/"Length of output: 186
examples/sites/demos/pc/app/time-picker/format.spec.ts (1)
15-15
: LGTM! Selector made more specific.The addition of
.first()
makes the selector more precise, ensuring consistent behavior when multiple spans are present.packages/theme/src/time-panel/index.less (1)
42-42
: Structural improvement, but verify spacing impact.Moving the margin to
&__header-title
improves code organization by removing the unnecessary&__header-input
class. However, since the--tv-TimePanel-header-margin
variable has been modified to reduce bottom margin from 24px to 0, please verify this doesn't negatively impact the visual spacing in various usage scenarios.Let's check for any other components that might be affected by this spacing change:
✅ Verification successful
Margin change is part of an intentional style evolution
The git history shows this is actually the third iteration of the margin value:
- Initially:
var(--tv-space-lg, 12px) 0
- Then:
var(--tv-space-xl, 16px) 2px 24px 2px
- Now:
var(--tv-space-xl, 16px) 2px 0 2px
This appears to be part of a deliberate style evolution where the spacing has been refined over time. The current change maintains the same top/side margins while removing the bottom spacing, which aligns with the PR's style optimization goals.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other components using the TimePanel header margin rg --type=less "tv-TimePanel-header-margin" packages/theme/src/Length of output: 259
Script:
#!/bin/bash # Let's check the git history of the vars.less file to see the previous margin value git log -p packages/theme/src/time-panel/vars.less | grep -A 2 "tv-TimePanel-header-margin" # Also check for any visual tests or stories that might show the impact fd "time-panel" -e stories.tsx -e stories.jsx -e test.tsx -e test.jsxLength of output: 693
packages/theme/src/time-spinner/index.less (1)
57-59
: Verify layout transition to flexbox.The switch to flexbox layout with centered alignment is a good improvement for consistency. However, ensure this change doesn't cause unexpected spacing issues with the existing
translateY(-6px)
transform.✅ Verification successful
Let me check the specific time-spinner related styles to verify if there are any conflicts with the flexbox changes.
Based on the search results, I can now generate the final response since I have enough context about the time-spinner styles across different themes.
✅ Flexbox layout changes are safe to implement
The switch to flexbox layout with centered alignment is safe as there are no conflicting style overrides in other theme packages. The existing
time-spinner
implementations in both the main theme and SAAS theme use basic display properties (inline-block
) without any complex positioning that would interfere with the new flexbox layout.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any other time spinner related style overrides that might conflict rg -l "time-spinner.*display|flex|align" --type lessLength of output: 12914
Script:
#!/bin/bash # Check the actual content of time-spinner styles across themes rg "time-spinner.*\{[\s\S]+?\}" --type less -A 10Length of output: 2592
packages/theme/src/picker/index.less (1)
176-176
: LGTM! Line height adjustment improves vertical alignment.The reduced line height of 20px provides better vertical alignment in the small variant, maintaining a consistent visual hierarchy across different size variants.
packages/vue/src/picker/src/pc.vue (1)
123-127
: Verify styling after DOM structure change.The close icon component is now wrapped in an additional div element. While the functionality remains the same, this DOM structure change might affect the styling if there are CSS selectors relying on the direct parent-child relationship.
Let's verify if there are any CSS selectors that might be affected:
✅ Verification successful
No styling impact from DOM structure change
Based on the search results, the styling for
tiny-range__close-icon
is defined inpackages/theme-saas/src/picker/index.less
and relies only on the class name, not on the DOM structure. The class is applied directly to the component and the new div wrapper doesn't interfere with the styling rules. The absolute positioning and other styles will continue to work as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CSS selectors targeting the close icon rg -t less "tiny-range__close-icon|tiny-transition-icon-scale-in" --glob "**/*.less"Length of output: 86
Script:
#!/bin/bash # Let's try searching in all file types since the styles might be in .css or directly in .vue files rg "tiny-range__close-icon|tiny-transition-icon-scale-in" # Also search for any styles related to range picker close icon rg -A 5 -B 5 "close-icon" --glob "**/*.{vue,less,css,scss}"Length of output: 45021
examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (2)
25-27
: LGTM! Documentation clarity improvement.The addition of "通过" (through/by) in the Chinese description makes the usage instruction more explicit and follows good documentation practices.
38-41
: LGTM! Enhanced documentation with better parameter explanations.The description now better explains both the
is-range
andrange-separator
parameters, including their default values. This improvement helps developers better understand the component's configuration options.
…issues (#2452) * feat(date-picker): [date-picker]Optimize case display and fix style issues * feat(date-picker): [date-picker]Optimize case display and fix style issues * feat(date-picker): [date-picker] Fix style issues * feat(date-picker): [date-picker] Modify console for a large number of alarms * feat(button): [button] Optimize demo style for feature display * feat(date-picker): [date-picker] Optimize demo style * feat(time-picker): [time-picker] Optimize and modify component style issues * feat(time-picker): [time-picker] Optimize and modify component style issues * feat(time-picker): [time-picker] Optimize style * feat(time-picker): [time-picker] Optimize style
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
Release Notes
New Features
Bug Fixes
Documentation
Style