Repository navigation
fix(gantt): don't render task bars ending on an excluded day - #8047
himanshu-2l wants to merge 2 commits into
Conversation
fixTaskDates() tracked renderEndTime using the invalid flag from the previous loop iteration instead of the current one. If a run of excluded (e.g. weekend) days extended all the way through the end of a task's naive duration window, the loop would exit before renderEndTime ever caught up to the fully-adjusted endTime - leaving it stuck on an excluded day and causing the rendered task bar to visually end mid-weekend. renderEndTime is now always set to the final, fully-adjusted endTime (or null if the task has zero/negative span, preserving existing behavior for that case). Also corrects three existing test assertions in ganttDb.spec.ts that had encoded this buggy behavior as expected, and adds a regression test using the exact scenario from the issue. Fixes mermaid-js#6421
🦋 Changeset detectedLatest commit: eb7d020 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reachedNext included review available in 57 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Gantt date adjustment now computes ChangesGantt end-time correction
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mermaid/src/diagrams/gantt/ganttDb.spec.ts (1)
270-285: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the zero-span branch.
fixTaskDates()explicitly preservesnullwhen the adjustedstartTimeis afterendTime. Add a zero-duration task case with exclusions and assert thatrenderEndTimeremainsnull.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mermaid/src/diagrams/gantt/ganttDb.spec.ts` around lines 270 - 285, Extend the Gantt date tests near the existing excluded-weekend case to cover a zero-duration task whose adjusted startTime exceeds endTime. Configure date format and exclusions, add the zero-span task, then assert its renderEndTime remains null, exercising the null-preservation branch in fixTaskDates().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/mermaid/src/diagrams/gantt/ganttDb.spec.ts`:
- Around line 270-285: Extend the Gantt date tests near the existing
excluded-weekend case to cover a zero-duration task whose adjusted startTime
exceeds endTime. Configure date format and exclusions, add the zero-span task,
then assert its renderEndTime remains null, exercising the null-preservation
branch in fixTaskDates().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2163c830-1502-4989-b6cf-3ae81e147609
📒 Files selected for processing (3)
.changeset/gantt-render-end-time-weekend-offset.mdpackages/mermaid/src/diagrams/gantt/ganttDb.jspackages/mermaid/src/diagrams/gantt/ganttDb.spec.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8047 +/- ##
===========================================
- Coverage 79.52% 76.94% -2.58%
===========================================
Files 607 607
Lines 83393 83397 +4
Branches 14491 16838 +2347
===========================================
- Hits 66319 64173 -2146
- Misses 16040 18335 +2295
+ Partials 1034 889 -145
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Summary
Fixes a bug where a gantt task's rendered bar could visually end on an excluded day (e.g. a Saturday) instead of the next valid day, when
excludes: weekends(or similar) pushes the end date out to skip excluded days.Root cause
In
fixTaskDates()(ganttDb.js),renderEndTimewas only refreshed to match the paddedendTimeif the loop happened to run at least one more iteration after the last excluded day was resolved. When a run of excluded days extended through to the very end of the naive duration window, the loop exited before that catch-up iteration occurred, leavingrenderEndTimestuck on a stale, excluded date - whichganttRenderer.jsthen uses directly for the bar's rendered width/endpoint (timeScale(d.renderEndTime || d.endTime) - timeScale(d.startTime)).Example from the issue:
Act2: 2025-07-09, 3dwithexcludes weekendscorrectly computedendTime = 2025-07-14(Monday) for scheduling purposes, butrenderEndTimewas2025-07-12- a Saturday - so the bar was drawn two days short.Fix
renderEndTimeis now always set to the fully-adjustedendTimeonce the loop completes (or staysnullfor a zero/negative-span task, matching existing behavior for that edge case). This removes the one-iteration lag entirely.Three existing test assertions in
ganttDb.spec.tsasserted the old, buggyrenderEndTimevalues (e.g. expecting arenderEndTimethat lands on an excluded day) - these are corrected to the now-consistent values, which also matchendTimein every case. A new regression test reproduces the exact scenario from the issue.Test plan
Ran locally:
npx eslint --quiet packages/mermaid/src/diagrams/gantt/ganttDb.js packages/mermaid/src/diagrams/gantt/ganttDb.spec.ts→ cleannpx prettier --check packages/mermaid/src/diagrams/gantt/ganttDb.js packages/mermaid/src/diagrams/gantt/ganttDb.spec.ts→All matched files use Prettier code style!pnpm lint:jison→ cleannpx tsx scripts/tsc-check.ts→tsc-check OKnpx vitest run packages/mermaid/src/diagrams/gantt→Test Files 2 passed (2),Tests 77 passed | 1 skipped (78)npx vitest run packages/mermaid(full package suite) →Test Files 48 passed (48),Tests 1032 passed | 1 skipped (1033)pnpm build→ succeeds, all dist artifacts producedVerified the new test actually catches the bug: reverted the
ganttDb.jschange only and reran the spec file - both the new regression test and one pre-existing test failed with the exact expected wrong dates (2025-07-12/Saturday and2019-02-02/Saturday respectively), then restored the fix and confirmed all 77 tests pass.Includes a changeset (
patch, since this is a bug fix with no API changes).Fixes #6421
Summary
excludes: weekendsis enabled.fixTaskDates()to assignrenderEndTimeafter all date adjustments.nullfor zero- or negative-span tasks.