fix(calendar-view): Multiple tasks display abnormally in the same time period #3697
Conversation
WalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Vue as CalendarView (Vue)
participant Renderer as Slot Renderer
participant Styles as LESS (.is-scroll-list)
User->>Vue: Open calendar / navigate day
Vue->>Renderer: Render each time slot (i)
Renderer->>Renderer: events_curr = getEventByTime(date, time_i)
Renderer->>Renderer: events_next = getEventByTime(date, time_{i+1})
alt events_next.length > 1
Renderer->>Renderer: add class "is-scroll-list"
Renderer->>Styles: apply overflow-y:auto, overflow-x:hidden, hide scrollbar
else
Renderer->>Renderer: do not add scroll modifier
end
Renderer->>Vue: Render event items with width:92% and normal flow (no left offset), add mb-0.5
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
packages/theme-saas/src/calendar-view/index.less (2)
498-503: Consistent scroll behavior across browsersLooks good for WebKit. Consider also hiding scrollbars on Firefox/legacy Edge if
scrollbar-size-0doesn’t already do it in your setup.
511-517: Remove no-op positioning utilities on non-positioned items
top-0/left-0have no effect without positioned context. Trim them to reduce noise.@apply ~'px-1.5'; - @apply top-0; - @apply left-0; @apply z-10; @apply leading-normal; @apply rounded-sm;packages/theme/src/calendar-view/index.less (2)
497-505: Fix nested selector; hide scrollbars in all enginesThe nested
&.is-scroll-list::-webkit-scrollbarrepeats the class. Simplify and add Firefox/IE rules.- &.is-scroll-list{ - overflow-y: auto; - overflow-x: hidden; - - &.is-scroll-list::-webkit-scrollbar { - width: 0; - height: 0; - } - } + &.is-scroll-list { + overflow-y: auto; + overflow-x: hidden; + } + &.is-scroll-list::-webkit-scrollbar { + width: 0; + height: 0; + } + &.is-scroll-list { + scrollbar-width: none; /* Firefox */ + -ms-overflow-style: none; /* IE/Edge legacy */ + }
509-521: Droptop/leftsince items aren’t absolutely positionedThese properties don’t apply now that absolute positioning is gone.
padding: 0 6px; - top: 0; - left: 0; z-index: 10; line-height: 1.5; border-radius: 2px;packages/vue/src/calendar-view/src/pc.vue (2)
165-170: Avoid recomputinggetEventByTimetwice per slotYou call it for class computation and again for rendering. Consider extracting a helper (e.g.,
eventsInSlot(date, i)) to compute once.
183-184: Remove inline width; rely on themeWidth is already defined in theme (
.events-item { width: 92% }). Drop the inline style to prevent overrides.- 'width': `92%`packages/vue/src/calendar-view/src/mobile-first.vue (1)
246-252: Unify with desktop: useis-scroll-listinstead of raw overflow utilitiesKeeps behavior/theming consistent with PC and LESS.
- :class="{ - 'border-dashed': i % 2 === 0, - 'border-solid': i % 2 !== 0, - 'overflow-hidden overflow-y-auto scrollbar-size-0': - getEventByTime(date.value, item.time, state.dayTimes[i + 1] && state.dayTimes[i + 1].time) - .length > 1 - }" + :class="{ + 'border-dashed': i % 2 === 0, + 'border-solid': i % 2 !== 0, + 'is-scroll-list': + getEventByTime(date.value, item.time, state.dayTimes[i + 1] && state.dayTimes[i + 1].time) + .length > 1 + }"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/theme-saas/src/calendar-view/index.less(2 hunks)packages/theme/src/calendar-view/index.less(2 hunks)packages/vue/src/calendar-view/src/mobile-first.vue(2 hunks)packages/vue/src/calendar-view/src/pc.vue(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
| 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', | ||
| 'left': event.left + 'px', | ||
| 'width': `calc(92% - ${event.left}px)` | ||
|
|
||
| 'width': `92%` | ||
| }" |
There was a problem hiding this comment.
Fix default theme bug and trim redundant classes/styles
- Missing quotes around
'blue'cause a runtime error. top-0/left-0are no-ops; remove.- Inline
width: 92%duplicatesw-11/12/theme width; remove one (prefer theme/class).
- 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}`)]"
+ class="w-11/12 flex mb-0.5 items-center px-1.5 z-10 leading-normal rounded-sm"
+ :class="[gcls(`theme-${event.theme || 'blue'}`)]"
:style="{
'height': event.height + 'px',
-
- 'width': `92%`
+ /* width via class/theme */
}"📝 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.
| 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', | |
| 'left': event.left + 'px', | |
| 'width': `calc(92% - ${event.left}px)` | |
| 'width': `92%` | |
| }" | |
| class="w-11/12 flex mb-0.5 items-center px-1.5 z-10 leading-normal rounded-sm" | |
| :class="[gcls(`theme-${event.theme || 'blue'}`)]" | |
| :style="{ | |
| 'height': event.height + 'px', | |
| /* width via class/theme */ | |
| }" |
🤖 Prompt for AI Agents
In packages/vue/src/calendar-view/src/mobile-first.vue around lines 261 to 267,
the template has three issues: the default theme identifier is missing quotes
causing a runtime error, there are no-op positioning classes top-0 and left-0
that should be removed, and an inline width style duplicates the w-11/12 class;
fix by quoting the default theme as 'blue' in the gcls call, remove top-0 and
left-0 from the class attribute, and delete the inline 'width' style so the
component relies on the existing w-11/12 class for width (keep height inline if
needed).
discreted66
force-pushed
the
lk-0911
branch
from
6a2a5eb to
1bedbcf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/vue/src/calendar-view/src/pc.vue (3)
2-2: Safer height coercion (preserve decimals, avoid NaN).Use parseFloat (or Number) and guard invalid input.
- <div class="tiny-calendar-view" :style="{ 'height': height ? `${parseInt(height)}px` : 'auto' }"> + <div class="tiny-calendar-view" :style="{ height: (height !== undefined && height !== null && height !== '') ? `${parseFloat(height)}px` : 'auto' }">
165-170: Guard endTime for last slot and avoid double compute of getEventByTime.
- Add a fallback end time to handle the last slot reliably.
- You’re calling getEventByTime twice per slot (once for class, once for v-for). Consider memoizing/caching per (date,start,end) to cut work roughly in half.
- :class="{ - 'is-even-num': i % 2 === 0, - 'is-scroll-list': - getEventByTime(date.value, item.time, state.dayTimes[i + 1] && state.dayTimes[i + 1].time) - .length > 1 - }" + :class="{ + 'is-even-num': i % 2 === 0, + 'is-scroll-list': getEventByTime( + date.value, + item.time, + (state.dayTimes[i + 1] && state.dayTimes[i + 1].time) || _constants.DAY_END_TIME + ).length > 1 + }"If you’d like, I can propose a small memoizer (Map keyed by
${date}|${start}|${end}) exposed from setup() and used in both places.
172-185: Keys and width: prefer stable keys; make width themable; avoid backticks for static strings.
- Use a stable key if available (e.g., event.id) to reduce patching glitches.
- Replace hard-coded 92% with a CSS var for theme overrides; keep a fallback.
- <div - v-for="(event, idx) of getEventByTime( - date.value, - item.time, - state.dayTimes[i + 1] && state.dayTimes[i + 1].time - )" - :key="idx" + <div + v-for="(event, idx) of getEventByTime( + date.value, + item.time, + (state.dayTimes[i + 1] && state.dayTimes[i + 1].time) || _constants.DAY_END_TIME + )" + :key="event.id ?? idx" class="events-item" :class="[`theme-${event.theme || 'blue'}`]" :style="{ 'height': event.height + 'px', - 'width': `92%` + 'width': 'var(--tv-CalendarView-event-width, 92%)' }" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/theme-saas/src/calendar-view/index.less(3 hunks)packages/theme/src/calendar-view/index.less(3 hunks)packages/vue/src/calendar-view/src/mobile-first.vue(2 hunks)packages/vue/src/calendar-view/src/pc.vue(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/vue/src/calendar-view/src/mobile-first.vue
- packages/theme/src/calendar-view/index.less
- packages/theme-saas/src/calendar-view/index.less
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
| :class="{ | ||
| 'is-even-num': i % 2 === 0, | ||
| 'is-scroll-list': | ||
| getEventByTime(date.value, item.time, state.dayTimes[i + 1] && state.dayTimes[i + 1].time) | ||
| .length > 1 | ||
| }" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm supporting styles exist for is-scroll-list in both theme packages.
Ensure the LESS for .events-list > li.is-scroll-list (overflow-y, scrollbar hiding) shipped in both packages/theme and packages/theme-saas to avoid visual regressions.
Also applies to: 172-185
🏁 Script executed:
#!/bin/bash
# Verify is-scroll-list styles exist in both themes.
rg -nC2 -g "packages/theme*/**/calendar-view/**.less" -e 'is-scroll-list'
# Optional: inspect getEventByTime signature/behavior for optional endTime.
rg -nC3 -e 'getEventByTime\s*\(' packages | sed -n '1,200p'Length of output: 6199
Add missing scrollbar-hiding rules to theme-saas calendar-view styles.
packages/theme/src/calendar-view/index.less already hides scrollbars (.is-scroll-list::-webkit-scrollbar { width: 0; height: 0; }) while packages/theme-saas/src/calendar-view/index.less only applies overflow-y/x via @apply — add equivalent scrollbar-hiding (or matching utility) to packages/theme-saas/src/calendar-view/index.less to avoid visual regressions.
packages/theme/src/calendar-view/index.less
packages/theme-saas/src/calendar-view/index.less
🤖 Prompt for AI Agents
In packages/vue/src/calendar-view/src/pc.vue around lines 165-170, the component
toggles the is-scroll-list class but
packages/theme-saas/src/calendar-view/index.less is missing the scrollbar-hiding
rules present in packages/theme/src/calendar-view/index.less; update
packages/theme-saas/src/calendar-view/index.less to include equivalent rules for
.is-scroll-list to hide scrollbars (add the same ::-webkit-scrollbar
width/height: 0 rule and matching cross-browser rules such as scrollbar-width:
none and -ms-overflow-style: none), or reference/apply the existing
scrollbar-hiding utility used by packages/theme so the visual behavior matches
and prevents regressions.
PR
fix:同时间段任务过多显示异常
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
New Features
Style