Repository navigation
Align tables to prose and polish unavailable images, formula toggles, and icon titles - #287
Conversation
|
Review: Looks good (one polish fix suggested, nothing blocking) Checked on a fresh build of What works
Findings
No unrelated changes. Tabs, |
|
Follow-ups from review, in fe4b800:
After, desktop: the block frame, then the frame inside a callout: CI: e2e and container pass. Validation fails only on the reviewed-owner hashes for the three icon source files, listed in the PR body. |
# Conflicts: # apps/web/src/assets/icons/planner-stop.svg # apps/web/src/icon-assets.test.ts
Fixes that only exist because several open PRs now share one tree: - Drop #290's duplicate ImageIcon and its LineIcon titles, which #287 removed. - Remove the import #302 left unused after #276's inline code. - Renew design-contract pins for files more than one PR edited, and record #290's new icons and #295's face colours as reviewed exceptions. - Find icons by path rather than the titles #287 removed, and faces by the label #295 gives them. - A tool that returns after its turn was stopped ends as failed, so #314's early completion keeps #272's stopped-question outcome. Assistant-model: Claude Opus 5.5 Assistant-workflow: inline Assistant-verification: typecheck passed: bun run types Assistant-verification: dprint/oxlint/design checks passed: bun run ci Assistant-verification: bun test passed: 3860 pass, 0 fail Co-authored-by: Alex Lavaee <lavaman131@github.com>
|
PR babysitter could not rebase this branch cleanly onto main. The branch is unchanged. Please resolve the rebase conflict locally; automation will try again after the branch changes. |
This comment has been minimized.
This comment has been minimized.
|
PR babysitter could not rebase this branch cleanly onto main. The branch is unchanged. Please resolve the rebase conflict locally; automation will try again after the branch changes. |
This comment has been minimized.
This comment has been minimized.
|
PR babysitter could not rebase this branch cleanly onto main. The branch is unchanged. Please resolve the rebase conflict locally; automation will try again after the branch changes. |
|
PR babysitter could not rebase this branch cleanly onto main. The branch is unchanged. Please resolve the rebase conflict locally; automation will try again after the branch changes. |
|
CI babysitter rechecked the current failed I did not queue a commit from this runner because
|
2c993ec to
5a49fa5
Compare
… and icon titles - Tables start at the prose edge instead of centring; wide ones scroll within the prose measure while the rails stay in the gutter. - An image that fails to load becomes a quiet framed placeholder when it is alone in its paragraph and an inline chip inside a sentence, both labelled by the alt text. Adds a Nucleo-style ImageIcon. - A block formula's show-source toggle sits in its top-end corner and is revealed on hover or focus (always shown on touch). - Icons no longer render <title>, which showed the raw asset name as a native tooltip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… reveal The paragraph's managed line break added ~26px below the frame. Hide it while the frame is shown, and check in e2e that the formula toggle appears on focus-within. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5a49fa5 to
19e3979
Compare
19e3979 to
3f82c90
Compare


Tables align to prose; unavailable images adapt to block or inline placement; formula source controls sit in the block corner. Decorative icons no longer show raw asset names as native tooltips.
Integration preserves main’s newer stop/play shapes and ink color, removing only their titles. Two newer icons also needed title removal; the regression test caught both. Reviewed the icon forwarding boundaries, added ImageIcon’s matching case, and renewed exact pins including the finite audit catalogue.
Local checks and types pass. The initial unit run had 503 passes and one title regression; all eight icon tests pass after fixing it. All 46 responsive-content/table/code browser tests pass. Published head
5a49fa59; final GitHub CI is pending.Current integration: rebased cleanly onto merged #286. Local checks/types and all 37 table/rich-content/interface browser cases pass on the current base; CI caught three remaining navigation assertions that depended on deliberately removed SVG titles. They now check the actual rendered icons, including exactly one header project icon. All 485 web tests pass; revised head
3f82c90fawaits required CI.Original proposal and screenshots
Why
Four small authored-content issues from the design audit:
<>toggle floated on its own line below the formula, disconnected from the block.<title>(search,box-archive,xmark…), so hovering showed the raw asset name as a native tooltip.What changed
radius-md, hairline border, a muted image icon, and one line of tertiary xs text. Inside a sentence, it is an inline-code-style chip with the icon and alt text. Its accessible name is the alt text, or "Image unavailable" when there is none. Adds a Nucleo-styleImageIconto@chopin/iconsand the icon catalogue.btn btn-icon btn-ghostcontrol and tooltip as code blocks. With a fine pointer it appears on hover or focus-within; it is always shown on touch, and it stays visible while the source is open. The preview's scrollport stops short of the toggle, so a long formula scrolls horizontally without passing under it.LineIcon,LoaderIconand the two planner SVG assets no longer contain<title>. Every icon is decorative and controls get their names fromIconTooltip.icon-assets.test.tsnow asserts there are no titles.responsive-content.e2e.ts: the table and the image placeholder share the prose start edge, the inline fallback is a chip, and the formula toggle sits in the block corner.Screenshots
Table, before (top) and after (bottom), desktop:
Unavailable images, before (left) and after (right), desktop. Shown at block level, in a sentence, and in a callout:
Unavailable images, phone, before and after:
Formula, before (top: toggle below the formula) and after (bottom: corner toggle with tooltip on hover):
Formula, phone (toggle always shown), before and after:
Needs a design-contract review
bun run cifails only on the reviewed-owner SHA-256 for the icon sources. The edits removed thetitleprop and<title>elements, and line.tsx gainedImageIcon. I added theImageIcon > <LineIcon>case to the existing line.tsx exception. It is the same props-forwarding pattern as every other icon. I did not change the hashes:packages/icons/src/icon.tsxscripts/design-contract/exceptions/dynamic-packages.jsonefda1a4921302c8c92c0eccb5cabc56b6a8967893246e2b5bc995f1f42bf755bpackages/icons/src/line.tsxscripts/design-contract/exceptions/dynamic-packages.json6b76f84548559d644b3fc3c75b196265c4e9e7013ff79fe8e0a77ad8eedd5178packages/icons/src/system.tsxscripts/design-contract/exceptions/dynamic-packages.json81931c163dae45bdf46196e0a5c8080f81d50712b99445e3ab2cc7f88b4cba6bThese are safe because the forwarded-props data flow is unchanged. The only differences are that nothing renders a
<title>child any more, and one new icon follows the existing pattern.Testing
bun test packages/editor apps/web/src/icon-assets.test.ts apps/web/src/hosted.test.ts apps/web/src/chat/transcript.test.tsx: 494 pass, 0 failbun run types: passbun scripts/check-design-record.ts: passbun run ci: dprint, oxlint and tokens pass. It fails only on the design-contract hashes above.responsive-content,tableandcode.🤖 Generated with Claude Code