-
Notifications
You must be signed in to change notification settings - Fork 338
feat(CalendarView): Added custom color support for CalendarView and u… #4012
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
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughAdds support for color-value day backgrounds (hex, rgb, rgba or preset names) by introducing getDayBgStyle and getDayBgClass in the renderless layer, exposing them via the Vue API, and switching component templates to use these helpers; demos updated to demonstrate multiple color formats. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as "Calendar Component\n(mobile/pc)"
participant VueAPI as "Vue API Layer\n(getDayBgStyle/getDayBgClass)"
participant Renderless as "Renderless Module\n(getDayBgRawValue/isColorValue/getDayBgColor)"
participant Demo as "Demo Setter\n(setDayBgColor)"
rect rgba(200,230,255,0.5)
Component->>VueAPI: request day background for <day>
VueAPI->>Renderless: call getDayBgRawValue(day) / getDayBgColor(day)
Renderless->>Demo: invoke setter (if provided) to obtain raw value
Demo-->>Renderless: return color (hex/rgb/rgba/preset/class-name/object)
Renderless->>Renderless: isColorValue(raw) ? produce style : produce class
Renderless-->>VueAPI: return { style, class }
VueAPI-->>Component: supply style and class for rendering
Component->>Component: render day cell with style + class
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/renderless/src/calendar-view/index.ts`:
- Around line 142-153: getDayBgStyle currently only handles string color results
from getDayBgRawValue and throws away style-objects returned by setDayBgColor;
update getDayBgStyle to detect when result is an object (style object) and
return it directly, otherwise if isColorValue(result) return { backgroundColor:
result }, and finally return {} for anything else; use the existing helpers
getDayBgRawValue and isColorValue to locate where to add the object check so
templates receive object-style returns.
In `@packages/vue/src/calendar-view/src/mobile-first.vue`:
- Around line 166-176: The template uses an undefined identifier blue as the
fallback for event.theme which yields classes like theme-undefined; update the
fallback to a string literal so the class is correct. In the mobile-first.vue
template where the v-for renders events (the getEventByTime loop) and gcls is
called, change the interpolation to use a string fallback (e.g.,
gcls(`theme-${event.theme || 'blue'}`)) so event.theme falls back to the literal
"blue" rather than the undefined variable.
🧹 Nitpick comments (1)
packages/renderless/src/calendar-view/index.ts (1)
128-139: Avoid double invocation ofsetDayBgColorper day.
getDayBgClassandgetDayBgStyleeach callsetDayBgColorviagetDayBgRawValue. If the user callback is expensive or non‑pure, this can cause performance and consistency issues. Consider caching per day or exposing a single helper that returns both class and style from one call.
| <div v-for="(event, idx) of getEventByTime( | ||
| date.value, | ||
| item.time, | ||
| state.dayTimes[i + 1] && state.dayTimes[i + 1].time | ||
| )" :key="idx" | ||
| class="w-11/12 flex mb-0.5 items-center px-1.5 top-0 left-0 z-10 leading-normal rounded-sm" | ||
| :class="[gcls(`theme-${event.theme || blue}`)]" | ||
| :style="{ | ||
| :class="[gcls(`theme-${event.theme || blue}`)]" :style="{ | ||
| 'height': event.height + 'px', | ||
|
|
||
| 'width': `92%` | ||
| }" | ||
| > | ||
| }"> |
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.
Fix default theme fallback to avoid theme-undefined.
Line 172 references blue which isn’t defined in component scope; when event.theme is falsy the class becomes theme-undefined. Use a string literal fallback.
Proposed fix
- :class="[gcls(`theme-${event.theme || blue}`)]" :style="{
+ :class="[gcls(`theme-${event.theme || 'blue'}`)]" :style="{📝 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.
| <div v-for="(event, idx) of getEventByTime( | |
| date.value, | |
| item.time, | |
| state.dayTimes[i + 1] && state.dayTimes[i + 1].time | |
| )" :key="idx" | |
| class="w-11/12 flex mb-0.5 items-center px-1.5 top-0 left-0 z-10 leading-normal rounded-sm" | |
| :class="[gcls(`theme-${event.theme || blue}`)]" | |
| :style="{ | |
| :class="[gcls(`theme-${event.theme || blue}`)]" :style="{ | |
| 'height': event.height + 'px', | |
| 'width': `92%` | |
| }" | |
| > | |
| }"> | |
| <div v-for="(event, idx) of getEventByTime( | |
| date.value, | |
| item.time, | |
| state.dayTimes[i + 1] && state.dayTimes[i + 1].time | |
| )" :key="idx" | |
| class="w-11/12 flex mb-0.5 items-center px-1.5 top-0 left-0 z-10 leading-normal rounded-sm" | |
| :class="[gcls(`theme-${event.theme || 'blue'}`)]" :style="{ | |
| 'height': event.height + 'px', | |
| 'width': `92%` | |
| }"> |
🤖 Prompt for AI Agents
In `@packages/vue/src/calendar-view/src/mobile-first.vue` around lines 166 - 176,
The template uses an undefined identifier blue as the fallback for event.theme
which yields classes like theme-undefined; update the fallback to a string
literal so the class is correct. In the mobile-first.vue template where the
v-for renders events (the getEventByTime loop) and gcls is called, change the
interpolation to use a string fallback (e.g., gcls(`theme-${event.theme ||
'blue'}`)) so event.theme falls back to the literal "blue" rather than the
undefined variable.
…pdated the documentation,修复了格式问题
f502f0a to
eac0767
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/sites/demos/apis/calendar-view.js (1)
128-139: Consider adding English translation and verify type definition.Two observations:
Missing English translation: Since this is a key feature of this PR (custom color support), consider adding an English description to improve documentation accessibility.
Type definition: The
type: '() => void'suggests the function returns nothing, but the description indicates it should return a color value (hex, rgb, rgba, or preset name). Consider updating to something like'(date: Date) => string'or'() => string'to accurately reflect the expected return type.Suggested improvement
{ name: 'set-day-bg-color', - type: '() => void', + type: '(date: Date) => string', defaultValue: '', desc: { 'zh-CN': '设置日期背景色,使用函数返回颜色值时,返回十六进制、rgb、rgba 是自定义颜色,使用颜色名则是预设颜色', - 'en-US': '' + 'en-US': 'Set the date background color. When using a function to return color values, hex, rgb, or rgba values are custom colors, while color names are preset colors.' },examples/sites/demos/apis/cascader-view.js (1)
202-208: Update en-US description to match new color-format behavior.The zh-CN text now documents hex/rgb/rgba vs preset color names, but the en-US string is still generic. This leaves English docs incomplete.
✅ Suggested doc alignment
- 'en-US': 'Set Date Background Color' + 'en-US': 'Set date background color. When the function returns a color value, hex/rgb/rgba are treated as custom colors; color names use presets.'
♻️ Duplicate comments (1)
packages/renderless/src/calendar-view/index.ts (1)
142-154:getDayBgStylestill does not handle object-style returns.The past review flagged that object returns from
setDayBgColorare dropped bygetDayBgStyle. The current implementation still only handles color strings viaisColorValue(), returning{}for everything else—including style objects.🐛 Proposed fix to preserve object-style returns
export const getDayBgStyle = ({ props }) => (day) => { const result = getDayBgRawValue({ props, day }) + // 如果返回的是样式对象,直接返回 + if (result && typeof result === 'object') { + return result + } + // 如果是颜色值,返回样式对象 if (isColorValue(result)) { return { backgroundColor: result } } return {} // 不覆盖默认背景色 }
🧹 Nitpick comments (3)
examples/sites/demos/apis/cascader-view.js (1)
46-46: Consider isolating formatting-only doc tweaks.These look like spacing/punctuation-only adjustments. If they were caused by auto-formatting, consider separating them from feature changes or updating formatter config to avoid incidental diffs.
Based on learnings, ...
Also applies to: 283-283, 564-564, 586-586
packages/renderless/src/calendar-view/index.ts (1)
86-107:getDayBgColorduplicates logic now handled by helpers.The color detection logic (checking
startsWith('#')andstartsWith('rgb')) is duplicated betweengetDayBgColorand the newisColorValuehelper. Consider refactoringgetDayBgColorto useisColorValueandgetDayBgRawValuefor consistency.♻️ Suggested refactor to reduce duplication
export const getDayBgColor = ({ props }) => (day) => { - const date = day.year + '-' + day.month + '-' + day.value - const isFunction = props.setDayBgColor instanceof Function - - if (isFunction) { - const result = props.setDayBgColor(date) - // 如果返回的是样式对象,直接返回 - if (typeof result === 'object') { - return result - } - // 如果是字符串且包含颜色格式(#开头的十六进制或 rgb),返回样式对象 - if (typeof result === 'string' && (result.startsWith('#') || result.startsWith('rgb'))) { - return { backgroundColor: result } - } - // 否则保持原有的 class 名称逻辑 - return result + const result = getDayBgRawValue({ props, day }) + + // 如果返回的是样式对象,直接返回 + if (result && typeof result === 'object') { + return result } - - return 'white' + // 如果是颜色值,返回样式对象 + if (isColorValue(result)) { + return { backgroundColor: result } + } + // 否则保持原有的 class 名称逻辑 + return result || 'white' }examples/sites/demos/pc/app/calendar-view/custom-day-bg-color.vue (1)
19-34: Inconsistent formatting and fragile date comparison.
Formatting: Inconsistent spacing around
else if(e.g.,}else ifvs} else if).Date matching: Comparing with string
'5'and'8'may fail if dates are zero-padded (e.g.,'05','08'). Consider usingparseInt()for robust comparison.♻️ Suggested cleanup
setDayBgColor(date) { - if (date.split('-')[2] === '10') { + const day = parseInt(date.split('-')[2], 10) + if (day === 10) { return 'red' - }else if (date.split('-')[2] === '15') { + } else if (day === 15) { return '#00FF00' - } else if (date.split('-')[2] === '20') { + } else if (day === 20) { return 'rgb(0,0,255)' - }else if (date.split('-')[2] === '25') { + } else if (day === 25) { return 'rgba(255,0,0,0.5)' - }else if( date.split('-')[2] === '5'){ + } else if (day === 5) { return 'yellow' - }else if ( date.split('-')[2] === '8'){ + } else if (day === 8) { return 'green' } return '' }
…pdated the documentation
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: 3802
What is the new behavior?
Does this PR introduce a breaking change?
Other information
https://eid5t7dqqmy.feishu.cn/docx/QPfNdwy4ToQOLrxZyAmc7TVRnWg?from=from_copylink
相关的操作文档
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.