Skip to content
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 #2450

Closed
wants to merge 8 commits into from

Conversation

Youyou-smiles
Copy link
Collaborator

@Youyou-smiles Youyou-smiles commented Oct 28, 2024

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • New Features

    • Enhanced layout of date picker components with increased width for better visibility.
    • Various date picker components now support customization options, including week number display and first day of the week settings.
  • Bug Fixes

    • Adjusted validation rules for date fields to trigger on change events.
  • Style

    • Updated CSS styles for date picker components, including width adjustments and removal of unnecessary styles to improve layout consistency.
  • Documentation

    • Clarified descriptions in the time picker documentation to enhance user understanding of property usage.

Copy link

coderabbitai bot commented Oct 28, 2024

Walkthrough

This pull request primarily focuses on modifying the CSS styling of various Vue components related to a date picker. The key change involves increasing the width of the .demo-date-picker-wrap class from 280px to 360px across multiple files. This adjustment affects the layout of the date picker components but does not alter their functionality or logic.

Changes

File Path Change Summary
examples/sites/demos/pc/app/date-picker/*.vue Increased the width of the .demo-date-picker-wrap class from 280px to 360px.
examples/sites/demos/pc/app/date-picker/clear-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; no functional changes.
examples/sites/demos/pc/app/date-picker/clear.vue Increased width of .demo-date-picker-wrap from 280px to 360px; added three instances of tiny-date-picker.
examples/sites/demos/pc/app/date-picker/custom-suffix-icon-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; changed background color of .my-date-picker.
examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue Increased width of .demo-date-picker-wrap from 280px to 360px; updated background color of .my-date-picker.
examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; added props for week number display customization.
examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; removed nested style rule.
examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; removed deep selector for .tiny-date-editor.
examples/sites/demos/pc/app/date-picker/events-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; added demo-row class for layout adjustments.
examples/sites/demos/pc/app/date-picker/format-composition-api.vue Increased width of .demo-date-picker-wrap from 280px to 360px; no functional changes.
examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue Changed width of .demo-date-picker-wrap from 400px to 360px; updated validation rules for date2.
packages/theme/src/picker/index.less Modified styles for .range-prefix-cls and related classes; adjusted hover states and z-index properties.
packages/vue/src/picker/src/pc.vue Adjusted rendering logic for tiny-input component; refined class structure for close icon.
packages/vue/src/time/src/pc.vue Removed tiny-input component and its import; adjusted template structure accordingly.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • zzcr

🐰 In the land of code, where changes abound,
A wider date picker has come to be found.
With styles that expand, oh what a delight,
Our components now fit, looking perfectly right!
So hop along, friends, let’s code and create,
For a brighter UI, let’s celebrate! 🎉


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the enhancement New feature or request (功能增强) label Oct 28, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Outside diff range and nitpick comments (28)
examples/sites/demos/pc/app/date-picker/label-inside-composition-api.vue (1)

Line range hint 1-5: Consider enhancing accessibility attributes.

While the implementation is clean, consider adding ARIA labels and roles to improve accessibility for screen readers.

-    <tiny-date-picker v-model="dateValue" label="选择日期"></tiny-date-picker>
-    <tiny-date-picker v-model="dateRangeValue" label="选择日期范围" type="daterange"></tiny-date-picker>
+    <tiny-date-picker v-model="dateValue" label="选择日期" aria-label="Select Date"></tiny-date-picker>
+    <tiny-date-picker v-model="dateRangeValue" label="选择日期范围" type="daterange" aria-label="Select Date Range"></tiny-date-picker>
examples/sites/demos/pc/app/date-picker/filter-mode-composition-api.vue (2)

18-18: LGTM! Consider documenting the width change.

The width increase from 280px to 360px is consistent with other files in this PR. Consider adding a comment explaining the rationale behind this specific width choice for better maintainability.

 .demo-date-picker-wrap {
+  // Increased width to 360px to accommodate longer date formats and maintain consistency across all date pickers
   width: 360px;

Line range hint 3-4: Consider internationalizing the labels.

The hardcoded Chinese labels "日期选择" and "日期范围选择" should be internationalized for better global accessibility.

-    <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.singleLabel')"></tiny-date-picker>
+    <tiny-date-picker v-model="rangeValue" type="daterange" shape="filter" :label="t('datePicker.rangeLabel')"></tiny-date-picker>
examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (1)

Line range hint 13-21: Consider adding TypeScript type safety.

While the implementation is correct, consider enhancing type safety by:

  1. Adding TypeScript
  2. Defining proper types for the value prop
 export default {
+  // @vue/component
   components: {
     TinyDatePicker: DatePicker
   },
   data() {
-    return {
+    return {
+      // Consider using a more specific type than empty string
+      // e.g., null for no selection
       value: '',
       IconMinus: iconMinus()
     }
   }
 }
examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (1)

Line range hint 24-26: Update the demo-date-picker-wrap width to 360px.

The current width (280px) appears inconsistent with other files in this PR where the width has been increased to 360px for better component display.

Apply this change:

.demo-date-picker-wrap {
-  width: 280px;
+  width: 360px;
}
examples/sites/demos/pc/app/time-picker/basic-usage.vue (1)

Line range hint 31-33: Update width to match other files.

The width should be updated from 280px to 360px to maintain consistency with other date picker implementations mentioned in the AI summary.

Apply this change:

.demo-date-picker-wrap {
-  width: 280px;
+  width: 360px;
}
examples/sites/demos/pc/app/date-picker/step.vue (1)

39-39: Width change looks good but verify responsive behavior.

The width increase from 280px to 360px aligns with the standardization effort across date picker components. However, we should ensure this fixed width works well across different screen sizes.

Consider adding responsive width handling:

.demo-date-picker-wrap {
-  width: 360px;
+  width: 100%;
+  max-width: 360px;
}

This change would maintain the desired width while ensuring better compatibility with smaller screens.

examples/sites/demos/pc/app/date-picker/align.vue (1)

41-43: Consider documenting the minimum width requirement.

Since this width change is applied consistently across multiple files, it might indicate a minimum width requirement for optimal display of the date picker.

Add a comment explaining the width requirement:

 .demo-date-picker-wrap {
+  /* Minimum width required for optimal date picker display */
   width: 360px;
 }
examples/sites/demos/pc/app/date-picker/clear-composition-api.vue (1)

Line range hint 1-35: Consider responsive design implications.

While the width increase improves the component's appearance, consider adding responsive width adjustments for different screen sizes to maintain optimal layout across devices.

Consider adding media queries for better responsiveness:

@media screen and (max-width: 480px) {
  .demo-date-picker-wrap {
    width: 100%;
    max-width: 360px;
  }
}
examples/sites/demos/pc/app/date-picker/clear.vue (2)

Line range hint 1-21: Enhance template structure for better accessibility and semantics.

Consider replacing <p> and <br /> tags with more semantic HTML elements for better accessibility and maintainability.

 <template>
   <div>
-    <p>默认显示清除按钮:</p>
-    <br />
+    <h3>默认显示清除按钮:</h3>
     <div class="demo-date-picker-wrap">
       <tiny-date-picker v-model="value"></tiny-date-picker>
     </div>
-    <br />
 
-    <p>隐藏清除按钮:</p>
-    <br />
+    <h3>隐藏清除按钮:</h3>
     <div class="demo-date-picker-wrap">
       <tiny-date-picker v-model="value" :clearable="false"></tiny-date-picker>
     </div>
-    <br />
 
-    <p>自定义清除图标:</p>
-    <br />
+    <h3>自定义清除图标:</h3>
     <div class="demo-date-picker-wrap">
       <tiny-date-picker v-model="value" :clear-icon="IconClose"></tiny-date-picker>
     </div>
   </div>
 </template>

Line range hint 23-36: Consider enhancing type safety and documentation.

The implementation is correct, but could benefit from TypeScript adoption and JSDoc documentation for better maintainability.

 <script>
+// @ts-check
 import { DatePicker } from '@opentiny/vue'
 import { iconClose } from '@opentiny/vue-icon'

+/**
+ * Demonstrates different clear button configurations for DatePicker
+ * @component
+ */
 export default {
   components: {
     TinyDatePicker: DatePicker
   },
   data() {
     return {
+      /** @type {Date} Current selected date */
       value: new Date('2023-05-24'),
+      /** @type {Function} Custom clear icon component */
       IconClose: iconClose()
     }
   }
 }
 </script>
examples/sites/demos/pc/app/date-picker/timezone-composition-api.vue (1)

Line range hint 1-46: Consider enhancing component documentation and type safety.

While the component functions correctly, there are opportunities for improvement:

  1. Add JSDoc comments for the component's props and events
  2. Consider using TypeScript for better type safety, especially for the timezone data structure

Example enhancement:

/**
 * @prop {string} modelValue - The selected date/time value
 * @prop {boolean} showTimezone - Whether to show timezone selection
 * @prop {Object[]} timezoneData - Array of available timezones
 * @prop {string} defaultTimezone - Default timezone to use
 * @event {(timezone: { tz: { name: string } }) => void} selectChange - Emitted when timezone is changed
 */
examples/sites/demos/pc/app/date-picker/now.vue (2)

Line range hint 33-35: Misleading method name and implementation.

The now() method sets a fixed past date ('2018-12-11 18:18:18') which contradicts the button text suggesting "current server time".

Consider updating the implementation to actually use the current time:

-    now() {
-      this.value1 = '2018-12-11 18:18:18'
-    },
+    now() {
+      this.value1 = new Date().toISOString().slice(0, 19).replace('T', ' ')
+    },

Line range hint 36-42: Unnecessary Promise delay and inconsistent date handling.

The nowClick method uses an unnecessary setTimeout(0) and returns a hardcoded future date.

Consider simplifying and making it consistent with the first method:

-    nowClick() {
-      return new Promise((resolve) => {
-        setTimeout(() => {
-          resolve(new Date(2024, 9, 8, 18, 18, 18))
-        }, 0)
-      })
-    }
+    nowClick() {
+      return Promise.resolve(new Date())
+    }
examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (2)

Line range hint 1-24: Consider improving HTML semantics and accessibility

The template structure could be enhanced by:

  1. Using semantic HTML elements (e.g., section, h2) instead of p tags with <br /> for spacing
  2. Adding ARIA labels or descriptions for better screen reader support

Example refactor:

 <template>
   <div>
-    <p>默认隐藏周次序号:</p>
-    <br />
+    <section>
+      <h2>默认隐藏周次序号:</h2>
     <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 />
+    </section>

Line range hint 39-45: Add TypeScript types and JSDoc documentation

The script implementation would benefit from type annotations and documentation for better maintainability.

Example enhancement:

-function formatWeeks(customWeeks, weekFirstDays) {
+/**
+ * Formats the week number display and updates the week first days reference
+ * @param {number} customWeeks - The week number to format
+ * @param {Date[]} weekFirstDays - Array of dates representing first days of weeks
+ * @returns {string} Formatted week number with 'w' suffix
+ */
+function formatWeeks(customWeeks: number, weekFirstDays: Date[]): string {
   eachWeekFirstDay.value = weekFirstDays
   return customWeeks + 'w'
 }
examples/sites/demos/pc/app/date-picker/custom-weeks.vue (2)

60-60: Consider using CSS variables for consistent widths.

The width increase to 360px improves the component's visual presentation. However, since this width is being updated across multiple files, consider using a CSS variable for better maintainability.

Consider refactoring to use CSS variables:

+:root {
+  --tiny-date-picker-demo-width: 360px;
+}
+
 .demo-date-picker-wrap {
-  width: 360px;
+  width: var(--tiny-date-picker-demo-width);
 }

Line range hint 1-42: Enhance accessibility and documentation.

While the component structure is well-organized, consider the following improvements:

  1. Add ARIA labels to date pickers for better accessibility
  2. Enhance section descriptions with more detailed explanations
  3. Add TypeScript type information for props and events

Example improvements:

 <div class="demo-date-picker-wrap">
-  <tiny-date-picker v-model="value"></tiny-date-picker>
+  <tiny-date-picker
+    v-model="value"
+    aria-label="Date selection"
+    :aria-description="'Select a date using arrow keys. ' + 
+      (showWeekNumber ? 'Week numbers are shown.' : 'Week numbers are hidden.')"
+  ></tiny-date-picker>
 </div>

Consider adding JSDoc comments for the component and its methods:

interface DatePickerProps {
  /** The selected date value */
  modelValue?: string | Date;
  /** Whether to display week numbers */
  showWeekNumber?: boolean;
  /** Custom week number formatting function */
  formatWeeks?: (week: number, firstDay: Date) => string;
}
examples/sites/demos/pc/app/date-picker/disabled.vue (1)

60-60: LGTM! Consider documenting the width adjustment rationale.

The increase in width from 280px to 360px is a reasonable change that better accommodates date range pickers and various date formats. However, it would be helpful to document the reasoning behind this specific width choice in a code comment.

 .demo-date-picker-wrap {
+  // Increased width to accommodate date ranges and longer date formats
   width: 360px;
examples/sites/demos/pc/app/date-picker/size-composition-api.vue (1)

Line range hint 1-58: Consider enhancing accessibility attributes.

The component demonstrates good functionality but could benefit from improved accessibility. Consider adding ARIA labels and roles where appropriate.

For example:

-<div class="demo-date-picker-wrap">
+<div class="demo-date-picker-wrap" role="group" aria-label="Date picker size examples">
examples/sites/demos/pc/app/date-picker/size.vue (1)

61-64: Consider documenting the width requirement.

Since this width change is being applied consistently across multiple files, it would be helpful to document this as a design requirement in the component's documentation.

Add a comment explaining the minimum width requirement:

 .demo-date-picker-wrap {
+  // Minimum width required to properly display all date picker variants,
+  // especially the daterange picker which shows two dates
   width: 360px;
   & > * {
     margin-top: 12px;
   }
 }
examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue (1)

Line range hint 8-20: Consider streamlining validation structure.

The form structure has redundant required validation:

  1. Parent form-item has required attribute
  2. Child form-items have their own validation rules with required: true

Consider removing the required attribute from the parent form-item to avoid potential confusion and maintain a single source of truth for validation.

-      <tiny-form-item label="活动时间" required>
+      <tiny-form-item label="活动时间">
examples/sites/demos/pc/app/date-picker/validate-event.vue (2)

Line range hint 39-43: Fix inconsistent validation triggers and missing initial state

  1. The validation triggers are inconsistent between date1 (blur) and date2 (change). Consider using 'change' for both for consistency.
  2. The date2 field is missing from the initial ruleForm state.

Apply this diff to fix the issues:

 data() {
   return {
     ruleForm: {
       name: '',
-      date1: ''
+      date1: '',
+      date2: ''
     },
     rules: {
       name: [
         { required: true, trigger: 'blur' },
         { min: 3, max: 5, trigger: 'blur' }
       ],
       date1: [
         {
           type: 'date',
           required: true,
           message: '请选择日期',
-          trigger: 'blur'
+          trigger: 'change'
         }
       ],

Also applies to: 53-61


Line range hint 91-95: Consider extracting common styles

The width and margin styles are specific to this demo but might be useful across other date-picker demos. Consider extracting these to a shared style file for consistency and reusability.

Create a shared style file (e.g., date-picker-demo.less):

.demo-date-picker-wrap {
  width: 360px;

  .tiny-form-item .tiny-form-item {
    margin-bottom: 8px;
  }
}

Then import and use it across all date-picker demos.

packages/theme/src/time-panel/index.less (1)

Line range hint 108-108: Consider addressing or removing 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 verify if this borderless scenario is still needed and create an issue to track its removal if unnecessary?

packages/theme/src/time-spinner/index.less (1)

Line range hint 57-75: Overall implementation looks good with room for enhancement.

The changes successfully improve the time spinner's layout and structure. The flex layout provides better control over item alignment, and the dedicated heights for arrow states ensure consistent spacing. While functional, consider the suggested improvements for better maintainability and responsiveness.

Consider implementing a comprehensive responsive design strategy for the entire time-picker component to ensure consistent behavior across different device sizes and orientations.

packages/vue/src/picker/src/pc.vue (1)

Line range hint 2-2: Track TODO comment in issue system.

There's a TODO comment indicating that the tiny-date-container class should be removed in the future. This should be tracked properly to ensure it's not forgotten.

Would you like me to create a GitHub issue to track this technical debt item? The issue would include:

  • The TODO's context and location
  • The rationale for removing the class
  • Any potential impact on existing styles
examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (1)

English translations need improvement in several places

After reviewing the content, there are several inconsistencies and issues with English translations:

  • The suffix-icon demo's English description is incorrect - it shows the description for the format feature instead of describing the suffix icon functionality
  • The editable demo's English description incorrectly describes the disabled property instead of the editable property
  • The event demo is missing its English description entirely
  • Some translations are too literal and need polishing, for example:
    • "picker-options Setting a fixed time range" should be "Use picker-options to set a fixed time range"
    • "whether the setting is range selection" should be "Use is-range to enable range selection"

These translations should be reviewed and updated to maintain consistency and clarity in the documentation.

🔗 Analysis chain

Line range hint 25-41: Verify English translations match Chinese descriptions.

While the Chinese descriptions have been improved, please ensure that the English translations accurately reflect these changes and maintain consistency in explaining how to use the properties.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential inconsistencies between Chinese and English descriptions
# Look for patterns where Chinese mentions "通过" but English doesn't use corresponding phrases like "through" or "using"

echo "Checking translation consistency..."
rg -U "zh-CN':\s*'.*通过.*'.*\n.*'en-US':\s*'[^']*(?<!through|using|by)[^']*'" 

Length of output: 500


Script:

#!/bin/bash
# Let's try a simpler approach to check translations
echo "Checking time-picker translations..."
rg --json -U "'zh-CN':.+?'en-US':" examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js | jq -r '.data.lines.text'

Length of output: 223


Script:

#!/bin/bash
# Let's try to get the content directly to analyze translations
echo "Content of time-picker.js:"
cat examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js

Length of output: 8337

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 9acc34e and 1afb3ce.

📒 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 (13)
  • examples/sites/demos/pc/app/date-picker/align-composition-api.vue
  • examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue
  • examples/sites/demos/pc/app/date-picker/filter-mode.vue
  • examples/sites/demos/pc/app/date-picker/format-composition-api.vue
  • examples/sites/demos/pc/app/date-picker/label-inside.vue
  • examples/sites/demos/pc/app/date-picker/multiple-dates-composition-api.vue
  • examples/sites/demos/pc/app/date-picker/multiple-dates.vue
  • examples/sites/demos/pc/app/date-picker/now-composition-api.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/unlink-panels.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 (47)
examples/sites/demos/pc/app/date-picker/custom-suffix-icon-composition-api.vue (2)

22-22: LGTM! Background color change improves contrast.

The change from #ddd to #f0f0f0 provides better visual distinction while maintaining a subtle background.


18-18: Verify width consistency across date picker components.

The width increase from 280px to 360px aligns with the PR's objective to optimize the time-picker component styling.

Let's verify this change is consistent across all date picker demos:

✅ Verification successful

Let me analyze the search results for width changes across date picker components:

  1. The file under review changes width from 280px to 360px
  2. Looking at other date picker related files:
  • examples/sites/demos/pc/app/date-picker/basic-usage.vue uses width: 360px
  • examples/sites/demos/pc/app/date-picker/custom-weeks.vue uses width: 360px
  • examples/sites/demos/pc/app/date-picker/filter-mode.vue uses width: 360px
  • examples/sites/demos/pc/app/date-picker/format.vue uses width: 360px
  • And many other date picker demo files consistently use width: 360px

The width change from 280px to 360px in custom-suffix-icon-composition-api.vue aligns with the standardized width of 360px used across other date picker components in the codebase.

Width change is consistent with other date picker components

The change from 280px to 360px matches the standard width used across the date picker component demos, making the UI more consistent.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all date picker demos use the same width
# Expected: All .demo-date-picker-wrap classes should have width: 360px

rg -l "demo-date-picker-wrap" --type vue | xargs rg "width:.*px" -A 1 -B 1

Length of output: 530852

examples/sites/demos/pc/app/date-picker/label-inside-composition-api.vue (2)

18-18: Width adjustment aligns with design improvements.

The increased width from 280px to 360px provides more space for the date picker component, which can improve readability and user interaction, especially for the date range picker that needs to display two dates.


Line range hint 15-22: Verify responsive behavior with new width.

The fixed width of 360px might affect the component's responsive behavior on smaller screens.

examples/sites/demos/pc/app/date-picker/custom-suffix-icon.vue (2)

30-30: LGTM: Improved background contrast.

The lighter background color (#f0f0f0) provides better visual contrast compared to the previous #ddd.


26-26: Verify layout compatibility with increased width.

The width increase from 280px to 360px is significant. While this change is consistent across multiple files, please ensure that:

  1. Parent containers can accommodate the wider component
  2. The layout remains responsive on smaller screens
✅ Verification successful

Based on the search results, I can see that the width change from 280px to 360px is actually part of a consistent pattern across all date-picker demo files. The demo-date-picker-wrap class is consistently set to 360px width in date-picker examples, while it's 280px in time-picker examples. This indicates an intentional design decision to standardize the date picker width at 360px.

Width change is part of component standardization

The width increase from 280px to 360px in custom-suffix-icon.vue aligns with the standard width used across all date picker examples in the codebase. This is a consistent pattern that maintains visual uniformity across the date picker component demos.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for potential fixed-width parent containers that might conflict
rg -g '*.vue' -g '*.css' -g '*.scss' 'demo-date-picker-wrap' -B 2 -A 2

Length of output: 77242

examples/sites/demos/pc/app/time-picker/basic-usage-composition-api.vue (1)

19-19: LGTM: Date initialization change adds seconds precision.

The change improves the time picker's precision by including seconds in the initial value, which better demonstrates the component's full capabilities.

examples/sites/demos/pc/app/time-picker/basic-usage.vue (1)

24-24: LGTM! Verify test coverage for seconds display.

The addition of seconds (20) to the initial time is consistent with related changes in the composition API version and test files.

Let's verify the test coverage:

✅ Verification successful

Test coverage for seconds display is comprehensive

The test files show thorough coverage of the seconds display functionality:

  • basic-usage.spec.ts explicitly tests the '18:40:20' time format with seconds
  • Multiple test files verify different time formats including seconds ('18:40:00', '18:42:20')
  • Tests cover various interactions and edge cases with the time picker while maintaining seconds precision
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the test files properly cover the seconds display functionality

# Test: Look for time-related assertions in test files
rg -A 2 "'18:4[0-9]:[0-9]{2}'" examples/sites/demos/pc/app/time-picker/

Length of output: 5250

examples/sites/demos/pc/app/date-picker/step.vue (1)

Line range hint 1-41: Verify visual consistency across all date picker variants.

Since this is part of a broader style standardization effort, let's verify that all date picker demos maintain consistent styling.

✅ Verification successful

Visual consistency verified across date picker demos

All date picker demos consistently use a width of 360px for the .demo-date-picker-wrap class, showing proper standardization. Time picker demos use 280px, which is expected as they are a different component with different styling requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent width styling across all date picker demos
# Expected: All date picker demos should use the same width value

# Search for date picker wrap class width definitions
echo "Checking date picker wrap widths across all files:"
rg -l "demo-date-picker-wrap" | xargs rg "width:.*px"

Length of output: 5008

examples/sites/demos/pc/app/date-picker/align.vue (1)

42-42: LGTM! Verify width consistency across all date picker demos.

The increased width provides better spacing for the date picker component.

Let's verify that this width change is consistent across all date picker demos:

✅ Verification successful

Width change is consistently applied across all date picker demos

The verification confirms that all date picker demos (examples/sites/demos/pc/app/date-picker/*.vue) consistently use the new width of 360px. Time picker demos maintain their original 280px width, which is appropriate as they are separate components with different space requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all date picker demos use the same width
# Expected: All files should show the new width of 360px

# Search for width declarations in date picker demo files
rg -l "demo-date-picker-wrap" | xargs rg "width:"

Length of output: 4891

examples/sites/demos/pc/app/date-picker/clear-composition-api.vue (1)

34-34: Width adjustment aligns with PR objectives.

The width increase from 280px to 360px provides more space for the date picker component, which should improve the overall user experience and readability. This change is consistent with similar adjustments across other date picker demo files.

Let's verify the consistency of this width change across other date picker demos:

✅ Verification successful

Width change is consistent across date picker demos

The verification confirms that all date picker demos consistently use the width value of 360px for their wrapper components (either .demo-date-picker-wrap or .demo-date-picker). This change aligns with the PR's optimization objectives and maintains visual consistency across all date picker examples.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent width values across date picker demo files
# Expect: All demo files should use the same 360px width

# 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 find width-related styles in date picker demos
# First, let's find all date picker demo files
fd -e vue . examples/sites/demos/pc/app/date-picker/ -x cat {} | grep -B2 -A2 "width:"

Length of output: 3377

examples/sites/demos/pc/app/date-picker/clear.vue (1)

43-45: Verify the increased width doesn't cause layout issues.

The width increase from 280px to 360px looks reasonable for better UX, but let's ensure it doesn't cause any layout issues in different contexts.

examples/sites/demos/pc/app/time-picker/basic-usage.spec.ts (2)

8-9: LGTM! Test expectations updated for scroll selection scenario.

The time format change from 'HH:mm:00' to 'HH:mm:20' aligns with the component's updated behavior while maintaining the test's integrity.


15-16: LGTM! Test expectations updated for arrow control scenario.

The time format change maintains consistency with the scroll selection scenario and properly validates the arrow control functionality.

examples/sites/demos/pc/app/date-picker/default-value-composition-api.vue (2)

Line range hint 1-38: LGTM! Well-structured component following Vue best practices.

The component demonstrates good practices:

  • Uses Composition API with setup sugar syntax
  • Properly scoped styles
  • Clear separation of concerns between template, script, and styles

34-34: Verify responsive behavior with increased width.

The width increase to 360px provides better spacing for date range pickers, but we should ensure this doesn't cause issues on mobile devices.

Let's check if there are any responsive design considerations in the codebase:

✅ Verification successful

Based on the search results, I can see that:

  1. The codebase has extensive mobile-first and responsive design patterns, with media queries and breakpoints defined in theme files.

  2. The date picker component is part of a design system that supports both mobile and desktop views, with specific mobile-first variants (mobile-first/app/date-picker/).

  3. The width change from 280px to 360px appears in a PC demo example, not in the core component styles.

  4. Let me verify if this width is used consistently across other date picker demos:


Width change is consistent with design system standards

The width change from 280px to 360px is part of a consistent design pattern across the codebase:

  1. The 360px width is standardized across all PC date picker demos (40+ files)
  2. This width is also defined in the core theme files (packages/theme/src/picker/index.less and packages/theme-saas/src/date-picker/index.less)
  3. The width aligns with the mobile viewport width (360px) used in the mobile demo containers, suggesting it's part of a responsive design system that maintains consistency between desktop and mobile views

The change is safe and follows the established design patterns in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for responsive design patterns and breakpoints
# Look for media queries or responsive utilities that might be affected

# Search for responsive design patterns in style files
rg -g '*.{vue,css,less,scss}' '(media|@screen|responsive|mobile|tablet|desktop|breakpoint)'

# Search for documentation about responsive design guidelines
fd -g 'README*' -X grep -l "responsive"

Length of output: 287953


Script:

#!/bin/bash
# Check for width declarations in date picker related files
rg -g '*.{vue,css,less}' 'width:.*360px' --context 2

Length of output: 17267

examples/sites/demos/pc/app/date-picker/basic-usage.vue (2)

34-34: LGTM: Width adjustment aligns with component optimization goals.

The increase in width from 280px to 360px provides more space for the date picker components, which should improve readability and usability.


Line range hint 1-39: Verify accessibility of date picker components.

While the style changes look good, ensure that the date picker components remain accessible with proper ARIA labels and keyboard navigation.

examples/sites/demos/pc/app/date-picker/default-value.vue (2)

Line range hint 1-46: Verify the removal of datetime range picker specific styles.

The AI summary indicates that the nested style for .tiny-date-editor--datetimerange.tiny-input__inner was removed. Please confirm if this was intentional and if the datetime range picker still maintains proper styling.

#!/bin/bash
# Check for any global styles that might be handling the datetime range picker width
rg -g '*.vue' -g '*.less' -g '*.css' 'tiny-date-editor--datetimerange.*width'

42-42: Verify the width change impact across different screen sizes.

The width increase to 360px is consistent with other files in this PR, which should improve the date picker's usability. However, ensure this fixed width doesn't cause issues on smaller screens.

Consider adding responsive breakpoints or using relative units (e.g., rem, %) for better adaptability across different screen sizes.

✅ Verification successful

The width change is consistent with the design system

The width of 360px is consistently used across all date picker components in the codebase, including:

  • Base theme styles (packages/theme-saas/src/date-picker/index.less)
  • All date picker demo files
  • Related picker components

This change aligns the component with the established design system, making it a valid modification.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any media queries or responsive utilities being used with date pickers
rg -g '*.vue' -g '*.less' -g '*.css' 'demo-date-picker-wrap.*@media' || rg -g '*.vue' -g '*.less' -g '*.css' 'tiny-date-picker.*@media'

Length of output: 145


Script:

#!/bin/bash
# Let's check for width-related styles in date picker components and their wrappers
rg -g '*.vue' -g '*.less' -g '*.css' 'demo-date-picker-wrap' -A 2 -B 2

# Also check for any responsive utilities or classes that might be used
rg -g '*.vue' -g '*.less' -g '*.css' '(tiny-date|date-picker).*responsive|responsive.*(tiny-date|date-picker)'

# Look for similar width declarations in other date picker files
rg -g '*.vue' -g '*.less' -g '*.css' 'width:.*360px' -A 2 -B 2

Length of output: 94633

examples/sites/demos/pc/app/date-picker/format.vue (1)

46-46: Width adjustment aligns with global date picker styling updates.

The width increase to 360px is consistent with the broader changes being made across all date picker demos, which should provide better visibility and usability for date/time formats.

Let's verify the width consistency across all date picker demos:

✅ Verification successful

Width change is consistently applied across all date picker demos

The verification confirms that all 36 date picker demo files in the codebase have been updated to use the new width of 360px, maintaining a uniform appearance across the component examples.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all date picker demos use the same width
# Expected: All files should show the new width of 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/timezone-composition-api.vue (2)

48-48: Width adjustment aligns with design system update.

The increased width provides more space for the date-picker component, which is particularly beneficial for the timezone variant that includes additional UI elements.

Let's verify this is a consistent change across other date-picker demos:

✅ Verification successful

Width change is consistent across date-picker demos

The search results confirm that the 360px width is consistently applied across all date-picker demo components (except for events.vue and events-composition-api.vue which use 80% width). This standardization aligns with the design system update and provides a uniform experience across all date-picker variants.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify width consistency across date-picker demos
# Expected: All date-picker demos should use the same width value

rg -l "demo-date-picker-wrap" | xargs rg "width: \d+px"

Length of output: 99


Script:

#!/bin/bash
# Let's try a different approach to find width-related styles in date-picker components
rg "width:" examples/sites/demos/pc/app/date-picker/

# Also search for class names that might be related to date-picker width
rg "\.(?:date-picker|tiny-date-picker|demo-date-picker)" examples/sites/demos/pc/app/date-picker/ -A 2

Length of output: 12690


Line range hint 22-24: Verify UTC+8 conversion behavior.

The isutc8 prop usage needs verification to ensure correct timezone conversion, especially with the recent component updates.

Let's check for any related timezone conversion logic:

✅ Verification successful

Based on the search results, I can now verify the UTC+8 conversion behavior. The codebase shows:

  1. The isutc8 prop is properly defined in the date-picker component with timezone conversion logic
  2. The implementation in packages/renderless/src/picker/index.ts shows the timezone conversion is handled correctly:
to: isutc8 ? 8 : clientTimezone,
  1. The conversion is done using getDateWithNewTimezone utility which properly handles timezone offsets

The UTC+8 conversion behavior is implemented correctly and working as expected. The component will convert the time to UTC+8 (East eighth district time) when isutc8 is true, regardless of the system timezone.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Look for UTC+8 conversion implementation
# Expected: Find the implementation handling isutc8 prop

ast-grep --pattern 'isutc8: {
  type: Boolean,
  $$$
}'

# Also check for any timezone conversion utilities
rg -A 5 "convertToUTC8|toUTC8|fromUTC8"

Length of output: 99


Script:

#!/bin/bash
# Let's try a broader search for timezone-related code and props
# Search for timezone props and implementations
ast-grep --pattern 'props: {
  $$$
}'

# Search for any timezone or UTC related code
rg -i "timezone|utc.*8|gmt.*8" -A 5

# Look for date-related imports and utilities
rg "import.*date" -A 2

Length of output: 92881

examples/sites/demos/pc/app/date-picker/now.vue (2)

Line range hint 1-19: LGTM! Well-structured template with clear demonstration of both slot and function approaches.

The template effectively demonstrates two different implementation patterns for the "now" functionality, making it a good example for documentation purposes.


49-49: Verify consistent width changes across related components.

The width change from 280px to 360px should be consistent across all date picker implementations.

✅ Verification successful

Width property is consistently set to 360px across all date picker components

The verification shows that all date picker demo components in the directory have their width property consistently set to 360px, confirming that the width change is uniform across the entire date picker implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the width property is consistently set to 360px across all date picker demos
# Expected: All .demo-date-picker classes should have width: 360px

# Search for any inconsistent width values in date picker related files
rg -g '*.vue' 'demo-date-picker.*\{[^}]*width:\s*(?!360px)\d+px' examples/sites/demos/pc/app/date-picker/

Length of output: 393


Script:

#!/bin/bash
# Search for width property in date picker related files
rg -g '*.vue' 'width:.*px' examples/sites/demos/pc/app/date-picker/

Length of output: 3051

examples/sites/demos/pc/app/date-picker/custom-weeks-composition-api.vue (1)

51-51: Verify layout responsiveness with increased width

The width increase from 280px to 360px aligns with changes across other date-picker components. However, we should ensure this doesn't cause layout issues on smaller viewports.

Consider adding responsive breakpoints or using relative units:

 .demo-date-picker-wrap {
-  width: 360px;
+  width: min(360px, 100%);
 }
examples/sites/demos/pc/app/date-picker/timezone.vue (2)

Line range hint 1-24: LGTM! Well-structured template with clear examples.

The template effectively demonstrates three different use cases of the date-picker component, with clear separation and appropriate prop usage.


Line range hint 26-47: LGTM! Clean and efficient implementation.

The script section follows Vue best practices with proper component registration and clean event handling.

examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue (2)

Line range hint 1-58: Implementation looks good!

The component demonstrates good practices:

  • Clear separation of concerns between template, script, and styles
  • Consistent use of Vue 3 composition API
  • Well-organized demonstration of various date picker states
  • Proper scoping of styles

53-53: Verify consistent width across all date picker components.

The width increase from 280px to 360px aligns with the PR objectives to optimize the component style. However, let's verify this change is consistent across all date picker implementations.

✅ 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 codebase, including both composition API and options API versions. This confirms that the width modification is part of a uniform style update.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the width consistency across all date picker demo files
# Expected: All date picker demos should use the same width of 360px

# Search for width declarations in date picker demo files
rg -g '*.vue' --type-add 'vue:*.vue' -A 1 'demo-date-picker-wrap\s*{' examples/sites/demos/pc/app/date-picker/

Length of output: 226


Script:

#!/bin/bash
# Search for width declarations in date picker demo files
rg -g '*.vue' 'width:.*px' examples/sites/demos/pc/app/date-picker/

Length of output: 3051

examples/sites/demos/pc/app/date-picker/size-composition-api.vue (1)

55-55: Width increase aligns with cross-file standardization.

The width increase to 360px is part of a systematic change across multiple date picker demo files, which should improve the component's visual presentation and accommodate longer date formats.

examples/sites/demos/pc/app/date-picker/events-composition-api.vue (2)

3-4: LGTM! Grid layout changes improve component spacing.

The modification of column spans from 4 to 6 creates a well-balanced 4-column layout that properly utilizes the 24-column grid system.

Also applies to: 8-8, 12-12


3-18: Verify consistent styling across other date picker components.

The layout changes look good, but let's ensure consistency with other date picker components in the codebase.

✅ Verification successful

Layout pattern is appropriate and consistent

The grid layout with span="6" is only used in event-related demos where multiple date pickers need to be shown side by side. Other demos appropriately use a simpler stacked layout with consistent width (360px). This variation in layout is intentional and correct based on the different demo purposes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent layout patterns across date picker components

# Search for other date picker demos with different grid layouts
echo "Checking for inconsistent grid layouts in date picker components..."
rg -l "tiny-date-picker.*:span=\"[^6]\"" "examples/sites/demos/pc/app/date-picker/"

# Check for consistent usage of demo-row class
echo "Checking for demo-row class usage..."
rg -l "class=\".*demo-row.*\"" "examples/sites/demos/pc/app/date-picker/"

Length of output: 499


Script:

#!/bin/bash
# Let's examine the layout patterns in both files to verify consistency

echo "Checking layout patterns in events.vue..."
rg "tiny-col.*:span=" examples/sites/demos/pc/app/date-picker/events.vue -A 2

echo -e "\nChecking layout patterns in events-composition-api.vue..."
rg "tiny-col.*:span=" examples/sites/demos/pc/app/date-picker/events-composition-api.vue -A 2

# Also check for any other date picker demos to ensure we haven't missed any
echo -e "\nListing all date picker demo files..."
fd . examples/sites/demos/pc/app/date-picker/ -e vue

Length of output: 4523


Script:

#!/bin/bash
# Let's check a few more demo files to ensure layout consistency
# Sampling different types of date picker demos

echo "Checking layout in basic-usage-composition-api.vue..."
rg "tiny-col.*:span=" examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue

echo -e "\nChecking layout in date-range-composition-api.vue..."
rg "tiny-col.*:span=" examples/sites/demos/pc/app/date-picker/date-range-composition-api.vue

echo -e "\nChecking layout in size-composition-api.vue..."
rg "tiny-col.*:span=" examples/sites/demos/pc/app/date-picker/size-composition-api.vue

# Also check for any other grid classes that might be used
echo -e "\nChecking for other grid patterns in date picker demos..."
rg "tiny-col.*class=" examples/sites/demos/pc/app/date-picker/

Length of output: 820


Script:

#!/bin/bash
# Let's try a different approach to check the layout patterns
# Using cat to view the complete structure of a few sample files

echo "Checking complete structure of basic-usage-composition-api.vue..."
cat examples/sites/demos/pc/app/date-picker/basic-usage-composition-api.vue

echo -e "\nChecking complete structure of size-composition-api.vue..."
cat examples/sites/demos/pc/app/date-picker/size-composition-api.vue

# Also check for any row/column structure with a different pattern
echo -e "\nChecking for any row/column patterns in demos..."
rg "<tiny-(row|col)" examples/sites/demos/pc/app/date-picker/ -A 1

Length of output: 5274

examples/sites/demos/pc/app/date-picker/size.vue (1)

61-61: Verify layout compatibility with the increased width.

The width increase from 280px to 360px provides more space for date picker content, which is particularly beneficial for the daterange picker. However, we should ensure this change doesn't cause layout issues in parent containers.

Let's check for potential layout issues:

✅ Verification successful

Width change is part of a consistent design pattern

The width increase from 280px to 360px aligns with a standardized width being used across all date picker demo components in the codebase. The search results show that all date picker demos have already adopted the 360px width, while time picker demos use 280px width consistently. This indicates an intentional design decision to provide more space for date picker content.

  • The change maintains consistency with other date picker demos in the codebase
  • Time pickers remain at 280px as they require less horizontal space
  • No parent container conflicts found as these are isolated demo components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for parent containers that might be affected by the width change

# Look for parent containers with fixed widths that might conflict
rg -g '*.vue' -g '*.less' -g '*.css' --type-add 'vue:*.vue' -A 2 'demo-date-picker-wrap'

Length of output: 53070

packages/theme/src/time-panel/vars.less (1)

32-32: LGTM! Consider verifying the visual alignment.

The reduction of bottom margin from 24px to 0px aligns with the PR's goal of optimizing the time-picker component styles. This change will create a more compact layout by reducing the space between the header and content.

Let's verify if this change maintains proper visual alignment with other components:

✅ Verification successful

Let me analyze the related files to verify if this margin change is consistent with other panel components.


Change is consistent with other time-related components

The margin adjustment aligns well with other time-related components in the codebase. The TimeRange component uses --tv-TimeRange-header-margin-bottom: 0px, which matches this change. This creates a consistent visual pattern across related components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any related margin/padding variables in other panel components
# that might need similar adjustments for consistency

# Search for similar header margin variables in other panel components
rg -l "header-margin" "packages/theme/src/"

# Search for related time panel styling
rg "TimePanel" "packages/theme/src/"

Length of output: 4249


Script:

#!/bin/bash
# Check header margin values in other panel components
rg "header-margin.*" packages/theme/src/transfer/vars.less packages/theme/src/time-range/vars.less packages/theme/src/quarter-panel/vars.less

# Check the actual usage of TimePanel header margin
rg -A 2 "header.*margin" packages/theme/src/time-panel/index.less

Length of output: 941

examples/sites/demos/pc/app/date-picker/events.vue (1)

3-4: LGTM! Layout changes improve component spacing.

The column span adjustments from 4 to 6 create a better layout with the date picker components taking up more width. This aligns well with the overall objective to optimize the component's style.

Let's verify that the responsive layout works correctly with these changes:

Also applies to: 8-8, 12-12

✅ Verification successful

Layout changes are properly implemented and consistent

The layout changes are well-implemented across both the Options API (events.vue) and Composition API (events-composition-api.vue) versions of the component:

  • Both files use the same grid structure with four columns of :span="6" (totaling 24 columns)
  • The .demo-row class consistently applies 80% width in both files
  • The layout maintains proper spacing with demo-date-picker-label margins
  • The changes create a balanced, responsive layout that accommodates the wider date picker components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any other responsive layout configurations or breakpoint definitions
rg -g '*.vue' -g '*.scss' -g '*.css' 'demo-row|tiny-row.*span' examples/sites/demos/pc/app/date-picker/

Length of output: 429


Script:

#!/bin/bash
# Let's check the content of both files to understand the full context of the layout changes
cat examples/sites/demos/pc/app/date-picker/events.vue examples/sites/demos/pc/app/date-picker/events-composition-api.vue

Length of output: 3778

examples/sites/demos/pc/app/date-picker/validate-event-composition-api.vue (2)

Line range hint 52-60: Good improvement on validation trigger.

Changing the validation trigger from 'blur' to 'change' for the time picker is a good improvement as it:

  1. Provides immediate feedback to users
  2. Better aligns with how users interact with time pickers
  3. Matches the component's natural interaction model

Line range hint 87-91: Verify layout implications of increased width.

The width increase to 360px aligns with similar changes across other files, but we should verify that:

  1. Parent containers can accommodate the wider component
  2. The layout remains responsive
  3. The change is consistent with design specifications
✅ Verification successful

Width increase to 360px appears consistent with design patterns

The width increase from 280px to 360px in the date picker component is safe and follows existing patterns in the codebase:

  1. Several mobile components already use widths around 356px
  2. Multiple components across the codebase use widths ranging from 280px to 360px
  3. The parent container uses flexbox layout with form items, which can accommodate the wider width
  4. The change aligns with other date/time picker components that have similar width adjustments
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any fixed-width containers that might conflict with the new width
rg -g '*.vue' -g '*.less' -g '*.css' 'width:\s*(2[8-9][0-9]|3[0-5][0-9])px' --type-add 'vue:*.vue'

Length of output: 20554

examples/sites/demos/pc/app/date-picker/validate-event.vue (2)

Line range hint 10-14: LGTM: Validation event handling optimization

Good optimization to disable validation events on the pickers themselves, letting the form-item handle validation. This prevents potential double validation and aligns with best practices.

Also applies to: 16-17


Line range hint 1-95: Verify consistent styling across other date-picker demos

The width and validation changes should be consistent across all date-picker demos for a uniform user experience.

✅ Verification successful

Based on the shell script results, I can generate the final response:

Width styling is consistent across date-picker demos

The width of 360px is consistently applied across all date-picker demos in the .demo-date-picker-wrap class, while time-picker demos consistently use 280px. Let's try another script to verify validation triggers.


Based on the shell script results, I can provide the final response:

Styling and validation are consistent across date-picker demos

The width of 360px is consistently applied across all date-picker demos in the .demo-date-picker-wrap class. Let's verify the validation triggers.


No inconsistencies found in date-picker demos

The validation and styling patterns are consistent:

  • All date-picker demos use the standard 360px width
  • The validate-event demo and its composition API version have identical validation triggers and validate-event prop usage
  • The validate-event prop is only used in the validation-specific demos as expected
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for inconsistent widths in other date-picker demos
echo "Checking for inconsistent widths in date-picker demos..."
rg -l "demo-date-picker-wrap" | xargs rg "width:" -A 1

# Check for inconsistent validation triggers
echo "Checking for inconsistent validation triggers in date-picker demos..."
rg "trigger: ['\"](blur|change)['\"]" --type vue

Length of output: 9454


Script:

#!/bin/bash
# Search for validation triggers in Vue files, fixing the file type issue
rg "trigger: ['\"](?:blur|change)['\"]" -t vue

# Also check for validate-event prop usage
rg ":validate-event=" -t vue

Length of output: 137


Script:

#!/bin/bash
# Search for validation triggers without file type flag
rg "trigger: ['\"](?:blur|change)['\"]"

# Check for validate-event prop usage
rg ":validate-event="

# List all files with .vue extension for reference
fd -e vue

Length of output: 299956


Script:

#!/bin/bash
# Search for validation triggers in the specific date-picker demo files
rg "trigger: ['\"](?:blur|change)['\"]" examples/sites/demos/pc/app/date-picker/validate-event*.vue

# Check for validate-event prop usage in date-picker demos
rg ":validate-event=" examples/sites/demos/pc/app/date-picker/

Length of output: 1628

examples/sites/demos/pc/app/time-picker/format.spec.ts (1)

15-15: LGTM! More deterministic selector.

The change to use .first() makes the test more precise by explicitly selecting the first matching element, reducing potential flakiness in cases where multiple elements might match the filter criteria.

Let's verify the consistency of this pattern across the test file:

✅ Verification successful

Pattern is consistently used across time-picker tests

The verification shows that .first() is consistently used with filtered list items across multiple time-picker test files, including:

  • basic-usage.spec.ts
  • default-value.spec.ts
  • disabled.spec.ts
  • format.spec.ts
  • picker-options.spec.ts

The modified line follows the established pattern in the codebase, making it a good change for test reliability.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of .first() with filtered list items
# Expect: All similar selectors should follow the same pattern

rg --type ts "getByRole\('listitem'\).+filter.+first\(\)" -A 1

Length of output: 3169

packages/theme/src/time-panel/index.less (1)

42-42: LGTM! Margin consolidation looks good.

The consolidation of margin styling into the &__header-title class helps maintain a cleaner structure while preserving the layout spacing.

packages/theme/src/picker/index.less (2)

176-179: LGTM!

The line-height adjustment for the small variant improves vertical alignment while maintaining consistency with other size variants.


Line range hint 141-143: Verify layout compatibility with increased width.

The width increase from 280px to 360px (28.5% larger) might affect responsive layouts or parent containers. Please ensure this change doesn't cause unwanted wrapping or overflow in various viewport sizes.

✅ Verification successful

Based on the search results, I can generate the final response since I have enough information to assess the layout impact:

Width increase appears compatible with existing layouts

The codebase scan shows that most components using fixed widths are already using 300px or larger widths, including:

  • Select components (300px)
  • Menu bars (300-350px)
  • Grid tables (320px)
  • Error pages (350px)
  • Notification components (340px)

The increase from 280px to 360px aligns with these existing widths and shouldn't cause layout issues since:

  1. Parent containers are already handling similar or larger widths
  2. The component uses inline-flex display which adapts to content
  3. The width is applied specifically to the picker component and doesn't cascade
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for potential fixed-width containers that might be affected
rg -g '*.{vue,less,css}' --type-add 'vue:*.vue' -A 2 'width:\s*(2[8-9][0-9]|3[0-5][0-9])px'

Length of output: 56655

packages/vue/src/picker/src/pc.vue (1)

123-127: Verify styling after class name simplification.

The close icon's class has been simplified to just tiny-range__close-icon. While this improves code cleanliness, please ensure that:

  1. The icon's appearance and positioning remain correct
  2. The hover/focus states still work as expected
  3. The transition effects are preserved
examples/sites/demos/pc/app/time-picker/webdoc/time-picker.js (2)

25-27: LGTM! Clear and consistent description improvement.

The description for the picker-options demo has been improved by adding "通过" to make the explanation clearer and more consistent with other descriptions in the file.


38-41: LGTM! Enhanced clarity in range selection description.

The description for the is-range demo has been improved by:

  1. Adding "通过" to maintain consistency with other descriptions
  2. Clarifying the relationship between is-range and range-separator properties

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request (功能增强)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant