Conversation
Fix card placement when the tracked cursor row becomes stale after cards are moved or inserted. This change adds a canvas-floor calculation and overlap checks so a new row always starts below the lowest existing card, and in-row inserts are skipped when a card already occupies that slot. This prevents cards from being laid on top of other cards when the row height no longer reflects the current canvas state.
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves card placement logic on the canvas by ensuring new cards do not overlap with existing ones and that new rows start below the lowest card on the canvas. The feedback suggests optimizing performance on larger canvases by refactoring canvasFloorY to accept pre-fetched shapes and lazily fetching and reusing the page shapes list to avoid redundant traversals.
Avoid repeated calls to getCurrentPageShapes while calculating card slots. The placement logic now uses a cached snapshot for canvas floor and collision checks, keeping new rows positioned consistently below existing cards and preventing stale shape data during drag placement.
Add a forceNewRow flag to nextSlot and track placedThisTask so the first card placed during a task always starts a fresh row (prevents batches from splitting into leftover gaps). placedThisTask is reset using queueMicrotask to scope the behavior to the current microtask. Update placement logic and comments to skip row-fit when forceNewRow is true.
pablonoel
marked this pull request as ready for review
September 14, 2026 15:28
pablonoel
requested review from
beets,
miss-o-soup and
nick-nlb
as code owners
September 14, 2026 15:28
Add canvasContentXRange to compute the horizontal extent of existing cards. When a batch starts a new row, compute rowStartX so the batch's full-width row is centered under the canvas content and store it in the cursor. Introduce lastPlacementStartedNewRow to track when a placement started a new row and pass it into keepInView as frameWholeCanvas so the viewport frames the entire canvas (instead of only the new grid) after such placements. Update keepInView signature and framing logic accordingly.
A new row took its y from the lowest card the store had published, but a multi-shape `store.put` publishes nothing until it commits, so every wrapping card in a paste landed on its siblings. Take the y from the max of the canvas floor and the cursor's tracked row.
Replace the `placedThisTask` microtask flag with an explicit `RowStart` argument from the store subscriber, which knows the real batch boundary; an SSE chunk is not a query result. Center a new row on the batch's real widths rather than assuming a full row of the first card's width, which sent successive pastes walking left. Narrow collision exclusion to the current row, merge the two canvas scans into one, and add `register_card_placement.test.ts`.
Describe batches, clones, and the store visibility rule the placement code depends on, and record the `afterCreate` ordering caveat behind the camera behavior.
Clones are positioned in a `beforeCreate` handler, which tldraw hands one record at a time, so the first clone of a paste could not see its siblings and its row fell back to the grid's x while query rows were centered. Read the batch from `editor.createShapes`, the one method every creation passes through, and center the row on the width it will really occupy.
juliawu
approved these changes
Sep 18, 2026
There was a problem hiding this comment.
This file is now very, very long. I'd suggest in a follow-up to split this into separate files, perhaps split by responsibility: camera controls, grid/geometry calculations, lifecycle place/cleanup functions.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Cards could be placed on top of each other as they were created, and a set of
cards arriving together was positioned no differently from a single card. This
change makes placement aware both of what is already on the canvas and of which
cards belong to the same set.
Placement now checks whether the next slot in the current row is free before
using it, and wraps to a new row when something is already sitting there. A row
that wraps drops below both the row being tracked and the lowest card anywhere
on the canvas, so a card the user has dragged underneath the grid, or a sibling
created moments earlier in the same paste, is cleared rather than covered.
Cards that arrive together now behave as a set. A query result, a paste and a
duplicate each open a row of their own rather than filling whatever slots were
left over from earlier work, and that row is centered beneath the existing
content according to the combined width of the cards it will hold. When a card
opens a new row the camera frames the whole canvas, so an arriving set is seen
in the context of what was already there.
PLACEMENT.mddescribes the fullmodel.
Changes Made
another card already occupies that space.
canvas, whichever sits lower.
RowStartargument marksthe first card of a set: the store subscriber supplies it for query results,
and a wrapper around
editor.createShapessupplies it for pastes, drops andduplicates. The wrapper restores the editor's own method on cleanup.
the set that opens it. A set whose widths cannot be determined keeps the
grid's current horizontal position rather than guessing, because a guess
biases the row and the bias accumulates across successive rows.
framing only the grid currently being built.
PLACEMENT.mdnow documents how a row picks its horizontal position, thestore visibility rule that placement depends on during a paste, and the
afterCreateordering caveat that affects camera framing.register_card_placement.test.tscovers the wrap floor, per-set rows,centering, drift across repeated pastes, clone placement and blocked slots.
No new UI surface is introduced, but where cards land on creation does change.
Before and after recordings for desktop and mobile viewports are attached below.
Testing Done
The following were run from
dataweaver/:Lint passes across
tsc, Biome and Stylelint. The suite passes with 126 testsacross 18 files. The production build completes.
The new tests were also run against the placement code on
main, to confirmthat they describe the change rather than merely restating it. Five of the nine
fail there: the new row's clearance of the lowest card, both centering cases,
the horizontal drift across repeated sets, and the blocked-slot check.
The behavior was then checked by hand in the browser:
card, centered under the existing content, overlapping nothing.
than creeping left.
and confirm the row is still centered.
stack into one column with no overlap.
covering canvas placement, and the behavior is exercised by unit tests
against a stubbed editor.
required; no goldens or fixtures changed.
Risk & Rollback
The wrapper around
editor.createShapesis the only notable risk. It isinstalled when the placement system registers and removed when that system is
cleaned up, so it cannot outlive the canvas that installed it, and a signature
change in a future tldraw release would fail type-checking at build time rather
than misbehaving at runtime.
Nothing here touches configuration, stored data or an API boundary, so rolling
back means reverting the commits.
Checklist
AGENTS.mdand followedCODING_GUIDELINES.md, plusFRONTEND.mdfor UI changes.that application's guide.
Note: Only Maintainers can approve and merge PRs. Expected initial review
time: 3 business days.