Skip to content

Fix calendar axis column width assignment during view transitions#1837

Open
loukandr wants to merge 3 commits intocallumalpass:mainfrom
loukandr:fix/1742-calendar-axis-column-rendering
Open

Fix calendar axis column width assignment during view transitions#1837
loukandr wants to merge 3 commits intocallumalpass:mainfrom
loukandr:fix/1742-calendar-axis-column-rendering

Conversation

@loukandr
Copy link
Copy Markdown

Fixes #1742.

In timeGrid views, time-axis labels intermittently render in the middle of the day grid after switching views. Width assignment in applyTodayColumnWidth and resetTodayColumnWidths used positional slice math on <colgroup> cols, which can target the axis col when the col count and dated-cell count drift mid-render.

Cross-reference each dated cell to its corresponding <col> via cellIndex instead. The axis cell has no [data-date] and is never iterated, so its col is never touched.

Tests added in tests/unit/issues/issue-1742-calendar-axis-column-rendering.test.ts cover the helper, defensive col mapping, and three regression scenarios including the colgroup-drift case.

Copilot AI review requested due to automatic review settings April 28, 2026 08:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an intermittent FullCalendar timeGrid layout regression where time-axis labels shift into the middle of the grid after view switches by changing how <col> widths are mapped and applied.

Changes:

  • Add findColForCell to map a dated table cell to its corresponding <col> via cellIndex (avoiding slice-based <colgroup> assumptions).
  • Update applyTodayColumnWidth / resetTodayColumnWidths to use the new mapping and avoid touching the axis column during transitional DOM states.
  • Add unit/regression tests covering the width helper logic and the axis-column non-interference scenarios (including colgroup drift).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/bases/CalendarView.ts Reworks today-column width application/reset to map from dated cells to <col> elements via cellIndex, preventing accidental width assignment to the time-axis column during transitions.
tests/unit/issues/issue-1742-calendar-axis-column-rendering.test.ts Adds unit tests for shouldWidenTodayColumn, getTodayColumnWidths, findColForCell, and regression scenarios ensuring the axis <col> is never modified.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

});

test("extra colgroup cols (transitional state) do not leak widths to axis", () => {
const dateKeys = ["2026-04-28"];
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dateKeys is declared but never used in this test case. Removing it will reduce noise and avoid confusion about whether the value is meant to influence the assertions.

Suggested change
const dateKeys = ["2026-04-28"];

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 520f15c.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/bases/CalendarView.ts Outdated
export function findColForCell(cell: HTMLTableCellElement): HTMLTableColElement | null {
const table = cell.closest("table");
if (!table) return null;
const colgroup = table.querySelector("colgroup");
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findColForCell uses table.querySelector("colgroup"), which will also match a nested <colgroup> inside descendant tables (e.g., if user content renders a table inside a calendar cell). That contradicts the docstring claim of returning null when the table has no colgroup, and could apply widths to the wrong table. Consider restricting the lookup to a direct child colgroup (e.g., :scope > colgroup or scanning table.children) so only the table’s own colgroup is used.

Suggested change
const colgroup = table.querySelector("colgroup");
const colgroup = Array.from(table.children).find(
(child) => child.tagName === "COLGROUP"
);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restricted to direct-child colgroup in 242f901. Added a regression test for the nested-table case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Calendar view timeline is shifted to the center

2 participants