fix: rebuild the connector menu as one kind of control - #528
Merged
Conversation
Four reports on one menu, and they overlap enough that building them apart would have meant three passes over the same file. The Arrow endpoint drew a stroked Lucide arrow while the renderer produces a solid triangle, so the icon promised an open arrowhead nothing makes. It takes a drawn glyph mirroring ConnectorMarker's own path. Open keeps its chevron — that one is stroked on the canvas too (#490). Dash and Corners were segmented controls carrying words, directly below three rows of icon cells. Both are now icon cells like their neighbours: the dash row draws the pattern at the connector's own width, using the rule ConnectorView draws with, and Corners takes a drawn pair — the same L-bend rounded and square. Corners was not in the report; converting only the dash row would have left the identical inconsistency one line below it (#493). Line colour was the last control in the app still asking for a hex. It takes the Espresso grid that fill, border and text colour already use, with None disabled: the grid emits 'none' in fill mode and a connector with no stroke colour would vanish (#494). The label field is gone. Double-clicking a connector already opened a label, on both hit targets, focused and selected — the field was the redundant half (#492). Every native title went with the TabButtons. The menu carries its own tooltip provider: it opens as a Popover, whose content is teleported out of the toolbar's provider, so without one the tooltips would match in looks but each wait out its own delay. Width had no tooltip at all. The menu's toolbar trigger stops using the plain minus (#491). It previews the selected line's own colour, weight and dash, the way Fill and Border preview theirs — copying the insert glyph across would have brought back the very complaint that the two line controls looked alike. Closes #490 Closes #491 Closes #492 Closes #493 Closes #494 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Five reports on the connector's edit menu. Four edit the same file and two touch the same import line, so they land together — as the issues themselves flag.
What changed
#493 — Dash and Corners were segmented controls carrying words, sitting directly below three rows of icon cells. Both are icon cells now.
stroke-dasharrayruleConnectorViewpaints, at this connector's own width. So it previews this line rather than illustrating "dashed" generically.#490 — the Arrow endpoint icon drew
lucide-arrow-right, a stroked shaft with two open diagonals, whileConnectorMarkerproducesM0,0 L10,5 L0,10 z— a solid triangle. It takes a drawn glyph with the marker's own 1:1 proportions, fill-only (a stroke would round its point off against the caps the glyph sets). Open keeps its chevron: that endpoint really is stroked on the canvas.#494 — line colour was the last control in the app still asking for a hex. It takes the Espresso grid that fill, border and text colour already use.
allow-noneis false: the grid emits'none'in fill mode and a connector with no stroke colour would vanish. The default#7C7C7Cis still in the grey family after #481 moved it, so the row shows as selected — verified, since the issue asked.#492 — the label field is gone. Double-click already opened a label on both hit targets, focused and text-selected.
setLabelwent with it; the inline editor writes throughupdateConnectordirectly, so nothing else called it. The known trade stands: nothing in the UI now announces that a connector can carry a label.#491 — the menu's trigger stops using
lucide-minus, the bar #457 had already concluded does not read as a line tool. It now previews the selected line — its own colour, weight and dash — the way Fill and Border preview theirs. The issue warned specifically against copying the insert glyph across, since the original complaint was that the two line controls looked alike; a preview contrasts with the insert glyph because it is not a generic mark at all, and it says which line is selected.Tooltips
Every native
:titlein this file is gone — three of the four sites #497 lists.Widthhad no tooltip at all and now has one.The menu carries its own
TooltipProvider. This is the trap #497 flags: the menu opens in a Popover whose content is teleported out of the toolbar's provider, so without one the tooltips would match the toolbar's in looks but each wait out its own delay. Verified thatTooltiprenders throughTooltipTrigger as-childand portals its bubble, so theflex-1cells keep their layout — no wrapper element lands in the row.That leaves #497 with only the pen/highlighter
TabButtons.Checked by eye
Drawn glyphs and hand-built cells are not something a source assertion can judge, so I rendered the rows at real size and screenshotted them. Two things only showed up there, both fixed:
Tests
connectorSection.test.js(12 new): noTabButtonsand noTextInputsurvive; the dash row draws a pattern instead of naming it; Corners has drawn glyphs; no:titleon any control; the provider is present; the label field and its setter are gone; the Espresso grid is used withallow-nonefalse and the default stays a swatch; Arrow carries a glyph and Open keeps its chevron.insertCatalog.test.js(3 new, onShapeGlyph): the arrowhead is filled and closed with no stroke; the corner pair is the same bend twice, one curved; neither new family overrides the house 24-box, stroke-width or caps.These strip comments before asserting — the comments in the file name the things being asserted gone, which is the trap the earlier QA round hit twice.
connector-menu.spec.js(3 new E2E): this menu had no end-to-end coverage at all, which is how it could be rebuilt with nothing to catch a menu that renders empty. Each draws a real connector and reads the persisted document: a dash cell writesstyle.dashthrough; no label field remains while the other rows still render; no text input remains and a swatch writesstyle.colorthrough.Full unit suite green: 1521 tests.
Closes #490
Closes #491
Closes #492
Closes #493
Closes #494
🤖 Generated with Claude Code