Skip to content

Fix uneven column widths for overlapping schedules in compare mode - #451

Open
JaimeJr1 wants to merge 1 commit into
gt-scheduler:mainfrom
JaimeJr1:fix/compare-overlap-packing
Open

JaimeJr1 wants to merge 1 commit into
gt-scheduler:mainfrom
JaimeJr1:fix/compare-overlap-packing

Conversation

@JaimeJr1

Copy link
Copy Markdown

Summary

In compare mode, a set of calendar blocks that overlap each other now always
divides its day column evenly, with no empty column left between or beside
them. Two people busy at the same time each get half the column, however many
other (non-concurrent) blocks that day happens to contain. The regular
(non-compare) Scheduler calendar is untouched — its packing code runs exactly
as before.

Compare mode sizes a day's columns by walking the connected component of
overlapping blocks and stamping every block in that component with the same
column count. A connected component is not the same thing as "how many people
are busy at once": blocks chain together (A overlaps B, B overlaps C, A and C
never overlap), and the whole chain is then sized to the length of the chain
rather than to the largest number of blocks that are ever concurrent.

Before. Pin your own 9:30–10:45 section and a friend's 10:00–11:15 block:
they correctly split the Monday column in half. Now toggle on a second friend
whose block runs 11:00–12:15 — it overlaps the first friend but not you. All
three blocks immediately shrink to a third of the column, and a visibly empty
third opens up next to each of them. Nothing about your 9:30 conflict changed,
but it now renders narrower and further from the block it actually conflicts
with. The more schedules you overlay, the longer the chains get and the worse
it looks: the grid reads as far more crowded than it is, and it becomes harder
to see which blocks genuinely conflict.

After. Each of those blocks keeps half the column, because only two people
are ever busy at the same instant. Toggling a third, non-concurrent schedule on
or off no longer resizes anything it does not actually overlap.

src/components/Calendar/index.tsx now has two packing paths, selected by the
existing compare prop:

if (compare) {
  packMeetingsCompare(meetings, meetingSizeInfo);
} else {
  packMeetingsLegacy(meetings, meetingSizeInfo);
}
  • packMeetingsLegacy is the existing algorithm, moved verbatim out of the
    component body into a module-level function (including its
    updateJoinedRowSizes helper). No behavior change.
  • packMeetingsCompare collects one block per (id, day, period) and hands
    each day's blocks to packDayBlocks.
  • packDayBlocks sorts a day's blocks by start time (ties broken by end
    time, then by block key, so the result never depends on the order schedules
    happened to be merged in) and assigns each block the leftmost column that is
    free at the moment it starts. Blocks are grouped into maximal runs connected
    by overlap; when a group closes, every block in it is given the same
    rowSize — the number of columns that group actually needed.

For time intervals, first-fit over start-sorted intervals uses exactly the
minimum number of columns, which is the maximum number of blocks concurrent at
any instant (this is optimal coloring of an interval graph). So a group of
mutually overlapping blocks always tiles its day column with no gaps, and the
answer is independent of insertion order.

packMeetingsCompare also de-duplicates blocks: a single schedule version that
is reachable through two different people is overlaid once per person, and the
two identical blocks are drawn on top of each other. It now claims one column
instead of two, so the duplicate no longer halves everyone's width.

Compare-only by construction. The non-compare path is guaranteed unchanged,
not merely believed to be:

  • The old packing code is untouched, only relocated. Its logic is identical
    apart from a blockKey(block) helper that replaces the inline
    'crn' in x ? x.crn : x.id expression, which is the same expression.
  • The only new call site is behind if (compare), and compare already gated
    compare-mode behavior throughout this component.
  • Two tests lock the non-compare layout down with the exact widths and offsets
    main produces today (thirds of a column for a three-block chain, a full
    column for a lone block). They fail if the new packing ever leaks into the
    Scheduler tab.

This fix is deliberately scoped to compare mode only, to limit risk to the one
place this was reported and keep the regular Scheduler's packing behavior
completely untouched. The same connected-component-vs-concurrent-overlap issue
plausibly affects the regular Scheduler too (see #130) — if maintainers want
it, the packDayBlocks approach here could likely be extended to the legacy
path as a follow-up, but that's out of scope for this PR.

Notes for reviewers: no new dependencies, no changes to props, context,
styles, or any other component; packDayBlocks mutates the rowIndex /
rowSize of the block objects it's given, matching how the surrounding code
already builds meetingSizeInfo.

Resolves #450

Checklist

  • Tests added (src/components/Calendar/index.test.tsx, new file — covers both compare-mode packing and non-compare lock-down)
  • Existing test suite passes
  • Lint/typecheck/format clean (tsc --noEmit, eslint, prettier --check on touched files)
  • No behavior change outside compare mode (regular Scheduler packing code untouched, pinned by dedicated lock-down tests)

How to Test

  1. Run the suite: CI=true yarn test --watchAll=false.
  2. Open compare mode with 3+ overlapping schedules — e.g. your own section
    (9:30–10:45), a friend's block (10:00–11:15), and a second friend's block
    (11:00–12:15, which overlaps only the first friend, not you).
  3. Confirm each block splits evenly with only the block(s) it actually
    overlaps — here, all three should stay at half-column width with no dead
    column, since no more than two are ever concurrent. On main today, all
    three incorrectly shrink to a third of the column with a visible gap.

Detail on what's covered in src/components/Calendar/index.test.tsx: it
renders Calendar inside stub ScheduleContext / FriendContext providers
with a one-section stub Oscar, then reads the left / width inline styles
off the rendered .meeting elements — i.e. it asserts on what a user actually
sees, not on internals.

Compare mode:

  1. Own section + one overlapping friend block → both 10% wide (half of the 20%
    day column), sitting at 0% and 10% with no gap.
  2. Own section + three friends chained across the morning, never more than two
    concurrent → all four blocks stay 10% wide at the expected offsets. This is
    the reported bug; on main these blocks come out 6.67% wide.
  3. One schedule version shown under two different people → the doubled block
    takes a single column, so widths stay at 10%.

Regular scheduler (lock-down):

  1. A three-block overlap chain still renders at 20/3% wide with offsets 0,
    20/3, 40/3 — the current main behavior.
  2. A single non-overlapping block still takes the full 20% column.

Verified by reverting only Calendar/index.tsx to main and re-running the
file: tests 2 and 3 fail there (blocks come out 6.67% wide) while tests 4 and 5
pass, so the regression tests really do pin the reported behavior and the
lock-down tests really do describe today's Scheduler layout. Test 1 passes
either way — with only two blocks the old algorithm already got the right
answer, which is exactly why the bug went unnoticed until several schedules
were compared at once.

CI=true yarn test --watchAll=false on main before the change:

Test Suites: 8 passed, 8 total
Tests:       31 passed, 31 total

and with the change:

Test Suites: 9 passed, 9 total
Tests:       36 passed, 36 total

— the whole existing suite still passes, plus the 5 new tests. Also clean:
tsc --noEmit, eslint, and prettier --check on both touched files.

This branch has not been deployed

No deployments
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.

Compare mode calendar gives overlapping blocks uneven widths when 3+ schedules chain together

1 participant