fix(calendar-view): [calendar-view] modify the configuration of calendar view hours, minutes, and seconds, and adding an attribute #2932
Conversation
WalkthroughThe changes update the calendar view’s date handling and introduce a new property for controlling the display of a button. A new function Changes
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/vue/src/calendar-view/src/mobile-first.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
WalkthroughThis pull request addresses a bug in the calendar view by modifying the configuration of hours, minutes, and seconds. It introduces functions to handle date-time parsing and conversion, ensuring proper handling of time zones and default values when time parts are missing. Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| // 时间转GMT8 | ||
| const timesToGMT8 = (date) => { | ||
| const originalDate = new Date(date) | ||
| const gmt8 = new Date(originalDate.getTime() + 15 * 60 * 60 * 1000) |
There was a problem hiding this comment.
The conversion to GMT+8 seems incorrect. The offset should be 8 hours, not 15. Please verify the logic for time zone conversion.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/renderless/src/calendar-view/index.ts (1)
98-132: Simplify and optimize theparseDatefunction.The function's logic can be simplified and optimized:
- Complex branching with multiple objects
- Missing type definitions
- Unnecessary object creation and merging
Consider this simplified implementation:
-export const parseDate = (time) => { +export const parseDate = (time: string): DateTimeParts => { /* istanbul ignore next */ - let date = {} + let date = new Date() if (new Date(time) !== 'Invalid Date') { date = new Date(time) } else { date = new Date() } - // 识别无时分秒的日期时间 const timeParts = hasNoTime(time) - let timesPartsOne = {} - let timesPartsTwo = {} - if (!timeParts.hours) { - timesPartsOne = { - year: timeParts.year, - month: timeParts.month, - day: timeParts.day, - hours: 0, - minutes: 0, - seconds: 0 - } - } else { - timesPartsTwo = { - year: date.getFullYear(), - month: date.getMonth() + 1, - day: date.getDate(), - hours: date.getHours(), - minutes: date.getMinutes(), - seconds: date.getSeconds() - } - } - const timePartsList = Object.assign(timesPartsOne, timesPartsTwo) - return timePartsList + return timeParts.hours ? { + year: date.getFullYear(), + month: date.getMonth() + 1, + day: date.getDate(), + hours: date.getHours(), + minutes: date.getMinutes(), + seconds: date.getSeconds() + } : timeParts }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/renderless/src/calendar-view/index.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/renderless/src/calendar-view/index.ts
[error] 185-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 187-188: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
packages/renderless/src/calendar-view/index.ts (1)
246-258:❓ Verification inconclusive
Verify timezone handling for events.
The event time conversion logic needs careful verification:
- Ensure correct handling of events across different timezones
- Verify that GMT+8 conversion doesn't cause issues with daylight saving time
Please verify the timezone handling by testing with events from different timezones:
🏁 Script executed:
#!/bin/bash # Description: Find all event time handling code to verify timezone logic # Search for timezone-related code echo "Searching for timezone-related code..." rg -n "getTime|GMT|timezone|UTC" --type ts # Search for date manipulation code echo "Searching for date manipulation code..." ast-grep --pattern 'new Date($$$)'Length of output: 65829
Action Required: Verify and Test Timezone Handling for Calendar Events
The updated calendar view conversion logic now uses
hasNoTime,getTime, andtimesToGMT8to set event times. Please manually verify the following:
- Event Time Data: Confirm that events with explicit time values are correctly processed by
getTime, while those without time information use thetimesToGMT8conversion.- Fixed Offset Logic: Validate that adding a fixed 15-hour offset in
timesToGMT8reliably converts dates to GMT+8 without introducing issues—especially around daylight saving time transitions.- Cross-Timezone Scenarios: Test with events originating from multiple timezones to ensure the conversion behaves as expected across diverse settings.
Given the automated search results didn't yield conclusive evidence and potential DST edge cases might exist, please add or update unit tests to cover these scenarios.
| const timesToGMT8 = (date) => { | ||
| const originalDate = new Date(date) | ||
| const gmt8 = new Date(originalDate.getTime() + 15 * 60 * 60 * 1000) | ||
| return gmt8 | ||
| } |
There was a problem hiding this comment.
Improve timezone handling in timesToGMT8 function.
The function has several issues:
- Hardcoded magic number for hour offset
- No type definitions
- Potential timezone conversion issues
Consider this improved implementation:
-const timesToGMT8 = (date) => {
+const timesToGMT8 = (date: string | number | Date): Date => {
+ const GMT_8_OFFSET = 8; // Hours
+ const HOURS_TO_MS = 60 * 60 * 1000;
+
const originalDate = new Date(date)
- const gmt8 = new Date(originalDate.getTime() + 15 * 60 * 60 * 1000)
+ const utcOffset = originalDate.getTimezoneOffset() / 60;
+ const offsetHours = GMT_8_OFFSET + utcOffset;
+ const gmt8 = new Date(originalDate.getTime() + offsetHours * HOURS_TO_MS)
return gmt8
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const timesToGMT8 = (date) => { | |
| const originalDate = new Date(date) | |
| const gmt8 = new Date(originalDate.getTime() + 15 * 60 * 60 * 1000) | |
| return gmt8 | |
| } | |
| const timesToGMT8 = (date: string | number | Date): Date => { | |
| const GMT_8_OFFSET = 8; // Hours | |
| const HOURS_TO_MS = 60 * 60 * 1000; | |
| const originalDate = new Date(date) | |
| const utcOffset = originalDate.getTimezoneOffset() / 60; | |
| const offsetHours = GMT_8_OFFSET + utcOffset; | |
| const gmt8 = new Date(originalDate.getTime() + offsetHours * HOURS_TO_MS) | |
| return gmt8 | |
| } |
| const hasNoTime = (date) => { | ||
| const datetimeStr = date | ||
| let hoursTime = 0 | ||
| let minutesTime = 0 | ||
| let secondsTime = 0 | ||
| const [datePart, timePart] = datetimeStr.split(' ') | ||
| const [year, month, day] = datePart && datePart.split('-') | ||
| if (timePart) { | ||
| const [hours, minutes, seconds] = timePart && timePart.split(':') | ||
| hoursTime = hours | ||
| minutesTime = minutes | ||
| secondsTime = seconds | ||
| } | ||
| // 提取时间 | ||
| return { | ||
| year: Number(year), | ||
| month: Number(month), | ||
| day: Number(day), | ||
| hours: hoursTime, | ||
| minutes: minutesTime, | ||
| seconds: secondsTime | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and type safety to hasNoTime function.
The function needs better error handling and type safety:
- Missing input validation
- Potential null/undefined issues with split operations
- No TypeScript type definitions
Consider this safer implementation:
-const hasNoTime = (date) => {
+const hasNoTime = (date: string): DateTimeParts => {
+ if (!date || typeof date !== 'string') {
+ throw new Error('Invalid date string provided');
+ }
+
const datetimeStr = date
let hoursTime = 0
let minutesTime = 0
let secondsTime = 0
- const [datePart, timePart] = datetimeStr.split(' ')
+ const [datePart = '', timePart] = datetimeStr.split(' ')
- const [year, month, day] = datePart && datePart.split('-')
+ const [year = '0', month = '0', day = '0'] = datePart?.split('-') ?? []
if (timePart) {
- const [hours, minutes, seconds] = timePart && timePart.split(':')
+ const [hours = '0', minutes = '0', seconds = '0'] = timePart?.split(':') ?? []
hoursTime = hours
minutesTime = minutes
secondsTime = seconds
}
return {
year: Number(year),
month: Number(month),
day: Number(day),
hours: hoursTime,
minutes: minutesTime,
seconds: secondsTime
}
}
+interface DateTimeParts {
+ year: number;
+ month: number;
+ day: number;
+ hours: number;
+ minutes: number;
+ seconds: number;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasNoTime = (date) => { | |
| const datetimeStr = date | |
| let hoursTime = 0 | |
| let minutesTime = 0 | |
| let secondsTime = 0 | |
| const [datePart, timePart] = datetimeStr.split(' ') | |
| const [year, month, day] = datePart && datePart.split('-') | |
| if (timePart) { | |
| const [hours, minutes, seconds] = timePart && timePart.split(':') | |
| hoursTime = hours | |
| minutesTime = minutes | |
| secondsTime = seconds | |
| } | |
| // 提取时间 | |
| return { | |
| year: Number(year), | |
| month: Number(month), | |
| day: Number(day), | |
| hours: hoursTime, | |
| minutes: minutesTime, | |
| seconds: secondsTime | |
| } | |
| } | |
| const hasNoTime = (date: string): DateTimeParts => { | |
| if (!date || typeof date !== 'string') { | |
| throw new Error('Invalid date string provided'); | |
| } | |
| const datetimeStr = date | |
| let hoursTime = 0 | |
| let minutesTime = 0 | |
| let secondsTime = 0 | |
| const [datePart = '', timePart] = datetimeStr.split(' ') | |
| const [year = '0', month = '0', day = '0'] = datePart?.split('-') ?? [] | |
| if (timePart) { | |
| const [hours = '0', minutes = '0', seconds = '0'] = timePart?.split(':') ?? [] | |
| hoursTime = hours | |
| minutesTime = minutes | |
| secondsTime = seconds | |
| } | |
| // 提取时间 | |
| return { | |
| year: Number(year), | |
| month: Number(month), | |
| day: Number(day), | |
| hours: hoursTime, | |
| minutes: minutesTime, | |
| seconds: secondsTime | |
| } | |
| } | |
| interface DateTimeParts { | |
| year: number; | |
| month: number; | |
| day: number; | |
| hours: number; | |
| minutes: number; | |
| seconds: number; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 185-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 187-188: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/sites/demos/mobile-first/app/calendar-view/webdoc/calendar-view.js (1)
27-27: Fix spacing in English description.Add spaces around the code block for better readability.
- '<p>The <code>mode</code> attribute specifies that each month of the current year is displayed. The options are <code>month</code> / <code>timeline</code> / <code>schedule</code>. Control the display of the left button through the<code>show back today</code>attribute.</p>\n' + '<p>The <code>mode</code> attribute specifies that each month of the current year is displayed. The options are <code>month</code> / <code>timeline</code> / <code>schedule</code>. Control the display of the left button through the <code>show back today</code> attribute.</p>\n'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/sites/demos/apis/calendar-view.js(1 hunks)examples/sites/demos/mobile-first/app/calendar-view/calendar-mode.vue(1 hunks)examples/sites/demos/mobile-first/app/calendar-view/webdoc/calendar-view.js(1 hunks)packages/vue/src/calendar-view/src/index.ts(1 hunks)packages/vue/src/calendar-view/src/mobile-first.vue(2 hunks)
🔇 Additional comments (5)
packages/vue/src/calendar-view/src/index.ts (1)
80-83: LGTM! Well-structured prop definition.The new
showBackTodayproperty is correctly defined with appropriate type and default value.examples/sites/demos/apis/calendar-view.js (1)
175-188: LGTM! Comprehensive API documentation.The new property is well-documented with:
- Clear bilingual descriptions
- Correct type and default value
- Version tracking via meta information
- Appropriate demo reference
examples/sites/demos/mobile-first/app/calendar-view/calendar-mode.vue (1)
2-8: LGTM! Clear demonstration of the new property.The demo effectively shows how to disable the "back to today" button using the new prop.
packages/vue/src/calendar-view/src/mobile-first.vue (2)
23-23: Verify layout consistency whenshowBackTodayis false.The removal of margin when
showBackTodayis false could cause layout issues:
- The date picker might appear misaligned without the left margin
- The spacing between elements might become inconsistent
Please test the component's layout in both states (with and without the "back to today" button) to ensure visual consistency.
Also applies to: 26-26
23-26: Changes seem unrelated to the PR's objective.The PR aims to fix date display issues (specifically for May 25th), but these changes only affect the UI layout and button visibility. The date parsing and display logic modifications mentioned in the PR objectives are not reflected in these changes.
Could you clarify how these UI changes relate to fixing the date display issue? The core date handling logic might need to be addressed separately.
| 'mark-color', | ||
| 'multi-select' | ||
| 'multi-select', | ||
| 'showBackToday' |
There was a problem hiding this comment.
Define prop type and default value for showBackToday.
The prop is added without type checking or a default value, which could lead to runtime issues.
Add type definition and default value:
- 'showBackToday'
+ {
+ name: 'showBackToday',
+ type: Boolean,
+ default: true
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'showBackToday' | |
| { | |
| name: 'showBackToday', | |
| type: Boolean, | |
| default: true | |
| } |
James-9696
changed the title
James-9696
changed the title
PR
第一个修改点：
修改前：日历显示有问题，25号，日历视图上显示24号
修改后：日期显示正常及悬浮的tooltips上的提示信息事件，显示 2023-5-25 ~ 2023-5-28
第二个修改点：
如图：mobile-first新增showBackToday属性，控制左上角按钮是否显示，设置为false，不显示“回今天”的按钮。
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
show-back-todayto control the visibility of the "back to today" button in the calendar view.Improvements
showBackTodayproperty.