fix: updates DRAW to work better - #20
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings remain in src/apps/Draw.tsx.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR improves the Draw app’s line/polyline workflows, snapping, node editing, text editing, and UI.
Changes:
- Adds line drawing, snapping, and enhanced polyline behavior.
- Improves node operations, text editing, and toolbox styling.
- Expands tests and documents Draw behavior.
File summaries
| File | Description |
|---|---|
tests/drawapp.test.tsx |
Adds Draw interaction tests. |
tests/draw.test.ts |
Tests snapping and node geometry. |
src/lib/draw/ops.ts |
Implements improved node smoothing. |
src/lib/draw/geom.ts |
Adds anchor snapping geometry. |
src/apps/Draw.tsx |
Implements new Draw interactions and controls. |
src/apps/draw.css |
Styles glyphs, snapping, nodes, and text editing. |
CLAUDE.md |
Documents new Draw behavior. |
Review details
Suppressed comments (6)
src/apps/Draw.tsx:229
- This only clears
selectedNodewhen the selected shape or tool changes, so undo/redo can leave an index attached to a different document snapshot. For example, insert a node, select it, then undo: the index now refers to the old following node (or is out of range), and Delete/Smooth can act on the wrong node or record a no-op edit. Clear or reconcile the node selection whenever undo/redo restores a path with a different node list.
useEffect(() => setSelectedNode(null), [selected, tool])
src/apps/Draw.tsx:1579
- The side-panel field can still commit
text: '', leaving a zero-width, invisible shape that can no longer be selected, whereas the inline editor removes empty text and the new documentation promises the same behavior. Handle the empty value by removing the shape and clearing the selection here too.
onChange={(e) =>
commit(
(d) => replaceShape(d, { ...editable, text: e.target.value }),
`text:${editable.id}`,
)
src/apps/Draw.tsx:1296
- With the Node tool active on a selected path but no node chosen, Delete/Backspace falls through to
remove(), so it deletes the entire curve even though the Object → Delete node action is disabled. Treat the Node tool as handled whenevernodePathexists, and only remove the whole shape for non-node selections.
if (tool === 'node' && nodePath && selectedNode !== null) {
editNode(deleteNode, null)
return
}
remove()
src/apps/Draw.tsx:111
- This only changes the family for newly created text. Drawings created before this PR contain
font-family="var(--font-plain)";parseSVGpreserves that value, so opening and saving one still emits a CSS variable that does not resolve outside BeanWeb despite the new portability guarantee. Normalize the legacy family when adopting parsed documents (and cover that migration).
const FONTS: { label: string; value: string }[] = [
{
label: 'Sans',
value: '"Bitstream Vera Sans", "DejaVu Sans", "Noto Sans", "Segoe UI", system-ui, sans-serif',
},
{ label: 'Serif', value: 'Georgia, "Times New Roman", Times, serif' },
{
label: 'Mono',
value: '"Bitstream Vera Sans Mono", "DejaVu Sans Mono", Consolas, "Courier New", monospace',
src/apps/Draw.tsx:697
- The node gesture mutates the rendered path directly in
flush, but this early return does not restore thatdattribute. If the pointer moves and comes back to its start position (especially when the node was pressed a few pixels off-center), the model remains unchanged while the DOM is left visibly nudged until another render. Restore the model's path data before returning on a zero-net drag.
if (samePoint(mode.cur, mode.start)) return
src/apps/Draw.tsx:1457
- The selected text is rendered inside a rotated
<g>viarestTransform(shape), but this editor is a siblingforeignObjectwith only the unrotated bounds. Editing a rotated text object will place the input over the wrong location and angle instead of where the text is drawn; apply the same transform to theforeignObject(and cover the rotated case).
{editingText && (
<foreignObject
x={editBox.x}
y={editBox.y}
width={editBox.w}
height={editBox.h}
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+819
to
+821
| if (!nodePath || selectedNode === null) return | ||
| commit((d) => replaceShape(d, fn(nodePath, selectedNode))) | ||
| if (after !== undefined) setSelectedNode(after) |
Comment on lines
864
to
875
| const move = (ev: PointerEvent) => { | ||
| const mm = gesture.current.mode | ||
| if (mm.kind !== 'poly') { | ||
| window.removeEventListener('pointermove', move) | ||
| return | ||
| } | ||
| mm.cur = toDocPoint(ev) | ||
| const s = snapPoint(toDocPoint(ev), mm.points) | ||
| mm.cur = s.p | ||
| mm.snapped = s.hit | ||
| schedule() | ||
| } | ||
| window.addEventListener('pointermove', move) |
hali-coding
force-pushed
the
fix/draw
branch
from
September 12, 2026 07:45
7ae0fd6 to
8cad12c
Compare
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.
Changes