-
Notifications
You must be signed in to change notification settings - Fork 17.3k
UI: Fix Gantt tooltip showing wrong end date on queued/scheduled bars #68570
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,8 @@ import { renderDuration } from "src/utils/datetimeUtils"; | |||||||||||||||||||||||||
| import { buildTaskInstanceUrl } from "src/utils/links"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export type GanttDataItem = { | ||||||||||||||||||||||||||
| /** Effective task end (end_date, or "now" while running) — consistent across all segments of the same try. */ | ||||||||||||||||||||||||||
| end_when?: string | null; | ||||||||||||||||||||||||||
| isGroup?: boolean | null; | ||||||||||||||||||||||||||
| isMapped?: boolean | null; | ||||||||||||||||||||||||||
| /** Source try times for tooltips (matches TaskInstance `*_when` fields). */ | ||||||||||||||||||||||||||
|
|
@@ -135,13 +137,6 @@ export const transformGanttData = ({ | |||||||||||||||||||||||||
| const queuedMs = queuedDttm === null ? undefined : dayjs(queuedDttm).valueOf(); | ||||||||||||||||||||||||||
| const scheduledMs = scheduledDttm === null ? undefined : dayjs(scheduledDttm).valueOf(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Include scheduled/queued/start times in tooltip data whenever the timestamps exist. | ||||||||||||||||||||||||||
| const tryWhenForTooltip = { | ||||||||||||||||||||||||||
| ...(scheduledMs === undefined ? {} : { scheduled_when: scheduledDttm }), | ||||||||||||||||||||||||||
| ...(queuedMs === undefined ? {} : { queued_when: queuedDttm }), | ||||||||||||||||||||||||||
| ...(startDate === null ? {} : { start_when: startDate }), | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| let endMs: number; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (hasTaskRunning) { | ||||||||||||||||||||||||||
|
|
@@ -152,6 +147,17 @@ export const transformGanttData = ({ | |||||||||||||||||||||||||
| endMs = dayjs(endDate).valueOf(); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Include scheduled/queued/start/end times in tooltip data whenever the timestamps exist. | ||||||||||||||||||||||||||
| // start_when/end_when are carried on every segment of a try so the tooltip reports the | ||||||||||||||||||||||||||
| // task's actual start and end on the scheduled and queued bars too, not just the | ||||||||||||||||||||||||||
| // execution bar's own bounds. | ||||||||||||||||||||||||||
| const tryWhenForTooltip = { | ||||||||||||||||||||||||||
| ...(scheduledMs === undefined ? {} : { scheduled_when: scheduledDttm }), | ||||||||||||||||||||||||||
| ...(queuedMs === undefined ? {} : { queued_when: queuedDttm }), | ||||||||||||||||||||||||||
| ...(startDate === null ? {} : { start_when: startDate }), | ||||||||||||||||||||||||||
| ...(startDate === null && !hasTaskRunning ? {} : { end_when: dayjs(endMs).toISOString() }), | ||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two lines aren't symmetric with each other:
Suggest collapsing both into a single symmetric form — derive an
Suggested change
Then the block doesn't need to live below |
||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (scheduledMs !== undefined) { | ||||||||||||||||||||||||||
| const scheduledEndMs = | ||||||||||||||||||||||||||
| queuedMs ?? startMs ?? (hasTaskRunning || tryRow.state === "scheduled" ? Date.now() : endMs); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
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.
Could you tighten this to pin the actual value of
end_when, not just that it's consistent across segments? Withvi.useFakeTimers()+vi.setSystemTime(...)you can also assertend_whenmatches that fixed "now" — otherwise a regression returning a stale or zero timestamp would slip past.