Implement Improved Card UX#385
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic auto-height adjustments and custom drag handles for card shapes on the canvas, improving the user experience when interacting with and moving cards. The feedback highlights several critical improvements: handling the lostpointercapture event and marking history stopping points in the drag handler to prevent stuck states, utilizing requestAnimationFrame in the auto-height hook to avoid layout thrashing and ResizeObserver loop errors, and scoping the text selection clearing logic to the specific card container to prevent global selection issues.
bc64872 to
1735a8e
Compare
nick-nlb
left a comment
There was a problem hiding this comment.
Thank you Paulo!
The primary comments I put in here actually relate more to the previous PR. You could either do them here or roll them out to the tracker and do them separately.
I've also added a couple of other items to the tracker that relate to these changes that you could do either here or separately.
|
|
||
| export const QueryProvider = ({ children }: QueryProviderProps) => { | ||
| const { editor } = useAtlas(); | ||
| const selectedShapeIds = editor ? editor.getSelectedShapeIds() : []; |
There was a problem hiding this comment.
This isn't part of this PR, but I noticed this as I was testing:
If you select a card (putting the chip into the prompt box) and then submit a new query, the submitted query is one card selection behind.
I.e. Run query -> Select card A -> Run query [as if no card is selected] -> Select card A & B -> Run query [As if only card A is selected] etc.
The reason may be that this call to get the selectedShapeIds isn't "reactive" - i.e. selectedShapeIds() doesn't trigger here when a new item is selected, meaning that the dependency below isn't triggered either. If that's the case, we could fix this by removing this selectedShapesId here, and move it to inside runPrompt().
|
|
||
| // tldraw forces 'user-select: none' on all nested nodes. This ensures we | ||
| // allow selecting text when in single selection mode | ||
| :is([data-selection="single"]) & * { |
There was a problem hiding this comment.
This is a heavy hammer, as it cascades down into every node (including buttons). Once we have follow up cards, etc appearing in the card, we may have to make this more selective.
There was a problem hiding this comment.
I agree unfortunately tldraw adds:
.tl-container * {
-webkit-touch-callout: none;
-webkit-tap-highlight-color: transparent;
scrollbar-highlight-color: transparent;
-webkit-user-select: none;
user-select: none;
box-sizing: border-box;
outline: none;
}
So we're pretty much fighting this to ensure we can select text :/
Overview
Card auto-height
useCardAutoHeighthook — syncs a card shape'shto its rendered content height (viaResizeObserver+scrollHeight), capped at the variant max, with epsilon guards to avoid feedback loops and ignoring culled / off-screen measurements. Writes are history-ignored so they don't pollute the undo stack.CARD_VARIANT_SIZE→CARD_VARIANT_MAX;hnow means maximum height rather than fixed. Cards start at the max footprint on placement, then shrink to real content height.Card drag handle
useCardDragHandlehook — makes the actions bar a drag handle, since it sits outside the shape's geometry and tldraw can't drag it natively. Translates the shape by pointer delta (screen→page via zoom) using pointer capture.cursor: grab/grabbingand is wired up via the newonActionsPointerDownprop.Selection & text behavior
children-container→content-container; content is now a selectable text surface (cursor: auto,user-select: textin single-selection mode) instead of a drag handle.pointerdownstops propagation so the canvas doesn't start a gesture; unselected/multi-selected events pass through to tldraw.Sizing / spacing / styling
DISTANCE_FROM_OTHER_CARDS40 → 56. This was done to account for initial placement of cards relative to grid and fact that actions bar now moves above card. The bar is 48 pixels tall so this ensures it doesn't get cropped before user moves canvas.--color-query-surface→--color-query-surface-raised.Refactor
use_streaming_query.tsout of thehooks/subfolder into theatlas/scope root. As part of this change droppedhooksfolder.