githubnext/chopinPublic

Let's compose rich plans together

Align tables to prose and polish unavailable images, formula toggles, and icon titles #287

MergedMaggieAppletonmerged 3 commits intomainfromrefs/pull/287/head
  1. MA

    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 `3f82c90f` awaits required CI.

    <details><summary>Original proposal and screenshots</summary>

    ## Why

    Four small authored-content issues from the design audit:

    1. Compact tables were centred in the prose column, so a table started 40px right of every paragraph, callout and code block. 2. A broken image became a small grey "Image unavailable" pill, whether it was a block, inside a sentence, or inside a callout. 3. A block formula's show-source `<>` toggle floated on its own line below the formula, disconnected from the block. 4. Icons rendered a `<title>` (`search`, `box-archive`, `xmark`…), so hovering showed the raw asset name as a native tooltip.

    ## What changed

    - **Tables** start at the prose edge and shrink-wrap their columns. Wide tables scroll inside the prose measure. The row and column rails still sit in the gutter outside the column. - **Unavailable images** use one element styled by placement. Alone in a paragraph (top level or in a callout), it is a quiet frame at prose width: 6rem tall, inset background, `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-style `ImageIcon` to `@chopin/icons` and the icon catalogue. - **Block formulas**: the toggle sits in the block's top-end corner. It uses the same `btn btn-icon btn-ghost` control 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. - **Icons**: `LineIcon`, `LoaderIcon` and the two planner SVG assets no longer contain `<title>`. Every icon is decorative and controls get their names from `IconTooltip`. `icon-assets.test.ts` now asserts there are no titles. - New e2e assertion in `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:

    ![Table before and after](https://github.com/user-attachments/assets/8107788a-4816-4046-b77e-081011203957)

    Unavailable images, before (left) and after (right), desktop. Shown at block level, in a sentence, and in a callout:

    ![Images before and after desktop](https://github.com/user-attachments/assets/a8d804fd-53a8-4a5a-bdd7-09fef3c0d0e9)

    Unavailable images, phone, before and after:

    ![Images before and after phone](https://github.com/user-attachments/assets/c485de6a-87a9-470d-a219-eeaff04ca1be)

    Formula, before (top: toggle below the formula) and after (bottom: corner toggle with tooltip on hover):

    ![Formula before and after](https://github.com/user-attachments/assets/7c938dce-54c3-48a7-a3a1-bda09c56d779)

    Formula, phone (toggle always shown), before and after:

    ![Formula phone before and after](https://github.com/user-attachments/assets/7c84be5d-04fe-4893-85b7-b1a6b8f85223)

    ## Needs a design-contract review

    `bun run ci` fails only on the reviewed-owner SHA-256 for the icon sources. The edits removed the `title` prop and `<title>` elements, and line.tsx gained `ImageIcon`. I added the `ImageIcon > <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:

    | File | Exception file | New SHA-256 | | --- | --- | --- | | `packages/icons/src/icon.tsx` | `scripts/design-contract/exceptions/dynamic-packages.json` | `efda1a4921302c8c92c0eccb5cabc56b6a8967893246e2b5bc995f1f42bf755b` | | `packages/icons/src/line.tsx` | `scripts/design-contract/exceptions/dynamic-packages.json` | `6b76f84548559d644b3fc3c75b196265c4e9e7013ff79fe8e0a77ad8eedd5178` | | `packages/icons/src/system.tsx` | `scripts/design-contract/exceptions/dynamic-packages.json` | `81931c163dae45bdf46196e0a5c8080f81d50712b99445e3ab2cc7f88b4cba6b` |

    These 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 fail - `bun run types`: pass - `bun scripts/check-design-record.ts`: pass - `bun run ci`: dprint, oxlint and tokens pass. It fails only on the design-contract hashes above. - Checked in the browser on a fake-GitHub server (desktop 1440×900, phone 390×844). Table rails still appear in the gutter. - Local e2e is suspended by the orchestrator. CI's browser job runs `responsive-content`, `table` and `code`.

    🤖 Generated with [Claude Code](https://claude.com/claude-code)

    </details>

  2. MA
    Author

    **Review:** Looks good (one polish fix suggested, nothing blocking)

    Checked on a fresh build of `design/authored-block-polish` (fake-GitHub server, headless Chromium, desktop 1440x900 and phone 390x844) with the stress doc plus an extra doc covering images with no alt, very long alt, side-by-side inline images and images in a callout.

    **What works** - Tables start at the prose edge (left 691 = paragraph left 691). A wide table is 673px wide with `scrollWidth` 2093 and `overflow-x: auto`, so it scrolls inside the measure; page `scrollWidth` stays 1440 and 390. Row rails sit in the gutter (x 663-677), column rails above the table. A compact table shrink-wraps flush left; the empty right side reads as intentional (like a markdown table next to full-width blocks). - Unavailable images: block frame is exactly prose width, 96px tall, with icon and one line of text. Long alt truncates with an ellipsis on one line, with and without alt, on phone and desktop, and inside callouts. Inline chips wrap sensibly, and two chips sit side by side. Accessible name is the alt text or "Image unavailable". - Formula toggle: in the block's top-right corner, `opacity: 0` at rest and 1 on hover or `:focus-within` (fine pointer). Always visible on touch. It stays visible when source is open. It is a real `button` with `tabindex=0`, focusable, shows the focus ring and the tooltip ("Show Source"), and Enter toggles `aria-expanded`. The preview's side margins keep a long formula from running under it. - Icons: `LineIcon` sets `aria-hidden` unless `aria-label` or `aria-labelledby` is passed. No icon usage in `apps/` or `packages/` passes a label, so nothing relied on `<title>` for a name. Removing titles loses no accessible names, and it removes the raw-asset-name tooltips. The test now guards this. - Exception entry in `dynamic-packages.json`: `unresolved JSX props > ImageIcon > <LineIcon>` is identical to the neighbouring `CloseIcon`/`PlusIcon` entries (same shape, count 1, same forwarded-props pattern). It is justified. `check-design-contract.ts` fails only on "reviewed dynamic owner changed" hashes for `line.tsx`, `icon.tsx` and `system.tsx`, which is the allowed review-hash case. I did not touch the hashes.

    **Findings** 1. (polish, suggested) `packages/editor/src/styles.css`, new `p:has(> [data-plan-src]:first-child + br...)` block rule. The frame paragraph is 122px tall for a 96px frame: the trailing Lexical managed `<br>` still contributes a blank line (about 26px). So the space above a frame is 12px and the space below is about 38px (the callout case shows the same asymmetry before "Text with an..."). Suggested fix: in that same rule, set `> br[data-lexical-managed-linebreak] { display: none }` or give the frame `margin-block-end: -1lh` equivalent, and check the empty-paragraph placeholder is not affected. 2. (minor, uncertain) On a wide scrolling table the column rail strip overhangs the table's right edge by about 8px (strip ends at x=1372, table at 1364), and the rails for scrolled-away columns sit at x>1440. I did not compare with `main` and the rails were not obviously broken, but a clip to the scrollport would be tidier. Not caused by this PR if it already behaved that way. 3. (minor, unverified) On phone the comment button is 44px and pinned to the right edge (x 323-367), exactly where the formula toggle sits (x 338-366). I could not anchor a comment on a formula in the fixture, and formula blocks have no selectable text, so a collision looks unlikely, but it would collide if a comment anchored to a formula's first line. Worth a quick check if the Planner can anchor comments on math. 4. (nit) `.plan-image-missing` chip padding uses `0.125em 0.3em` and `gap: 0.3em`. If inline code uses a shared token for its chip padding, use that. 5. (nit) The new e2e test covers geometry well. It does not assert the toggle is reachable by keyboard or visible on focus. Consider adding that to the same test.

    No unrelated changes. Tabs, `let` and comment conventions are followed.

  3. MA
    Author

    Follow-ups from review, in fe4b8002:

    - **Space under the image frame**: the paragraph's managed `<br>` is now hidden while an unavailable-image frame is shown. The paragraph is now exactly as tall as the frame (96px). Measured gaps at desktop: 16px above / 12px below at top level, which is the normal block rhythm (before this fix: 12px / ~38px). In the callout the frame now ends at the callout's own padding. The empty-paragraph placeholder is unaffected: the selector requires `[data-plan-src]:first-child > .plan-image-missing`, so a paragraph holding only a `<br>` never matches. - **e2e**: `responsive-content` now checks that the paragraph height equals the frame height. It also checks that the formula toggle has opacity 0 when the pointer is away and 1 after a programmatic `focus()` (focus-within). - **Column-rail overhang**: this predates the PR. `rails.tsx` deliberately adds `pad = SEAM / 2` (8px) of slack at both ends so the outermost insert buttons aren't clipped. I measured 8px left and 8px right on a full-width table. I didn't change it.

    After, desktop: the block frame, then the frame inside a callout:

    ![Image frame after the follow-up](https://github.com/user-attachments/assets/3b5fa97e-678e-467e-866d-d96987a9f453) ![Callout image frame after the follow-up](https://github.com/user-attachments/assets/4e3d7c0b-6803-483a-a2d5-e989b650763a)

    CI: e2e and container pass. Validation fails only on the reviewed-owner hashes for the three icon source files, listed in the PR body.

  4. lavaman131referenced b2a01ceb2106last month on GitHub

  5. lavaman131referenced 168b4551040flast month on GitHub

  6. MA
    Author

    <!-- pr-rebase:fe4b800253a66fe910bed46d08a073830c7d6c68:964535bb81cb00d3583301d3ec90d55393c278ca --> 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.

  7. MA
    Author

    CI babysitter checked the recorded head and found no code defect to fix. The current failed `ci` run still only has `format, lint, types, tests` red, while `container` and `e2e` passed. The PR diff and existing notes point to the same blocker: `bun run ci` is failing on the reviewed design-contract hashes for `packages/icons/src/icon.tsx`, `packages/icons/src/line.tsx`, and `packages/icons/src/system.tsx`, which needs a human design-contract review rather than a speculative code change.

    No commit was queued. Next action: update the reviewed hash exceptions if those changes are approved, or adjust the icon changes if review rejects them.

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37618962845) · gpt54 · 48.8 AIC · ⌖ 5.99 AIC · ⊞ 16.4K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)

    <!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37618962845, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37618962845 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  8. MaggieAppletonauto-rebase-enabledlast month on GitHub

  9. MA
    Author

    <!-- pr-rebase:fe4b800253a66fe910bed46d08a073830c7d6c68:8675f8e57182b28d0b2dc34c8089d0d928d00191 --> 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.

  10. MA
    Author

    CI babysitter checked the recorded head and found no safe code fix to queue. The current failed `ci` run still stops in `check-design-contract` on reviewed-owner hash changes for `packages/icons/src/icon.tsx`, `packages/icons/src/line.tsx`, and `packages/icons/src/system.tsx`.

    No commit was queued. Next action: review and renew those design-contract exceptions if the icon changes are approved, or adjust the icon edits if review rejects them.

    > [!WARNING] > <details> > <summary>Firewall blocked 1 domain</summary> > > The following domain was blocked by the firewall during workflow execution: > > - `releaseassets.githubusercontent.com` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "releaseassets.githubusercontent.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details>

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37619025602) · gpt54 · 134 AIC · ⌖ 10 AIC · ⊞ 16.4K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)

    <!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37619025602, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37619025602 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  11. MA
    Author

    <!-- pr-rebase:fe4b800253a66fe910bed46d08a073830c7d6c68:e6b7bb333ce8948d0dac0c8aff61cf3d4fb8dc8b --> 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.

  12. MA
    Author

    <!-- pr-rebase:fe4b800253a66fe910bed46d08a073830c7d6c68:b919317f14f29799be257eee49ba5ab753592371 --> 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.

  13. MA
    Author

    CI babysitter rechecked the current failed `ci` run for `fe4b800253a66fe910bed46d08a073830c7d6c68`. The head still matches the recorded candidate, and the failure remains the reviewed design-contract hash check for `packages/icons/src/icon.tsx`, `packages/icons/src/line.tsx`, and `packages/icons/src/system.tsx`, while `e2e` and `container` are green.

    I did not queue a commit from this runner because `bun` and `mise` are not installed here, so I could not run the required local reproduction and verification commands (`bun install --frozen-lockfile`, `bun run fix`, `bun run ci`). Next action for a human: on a machine with Bun 1.4.2 available, rerun those commands on this head, review those design-contract owner changes, and renew the exact exceptions only if they are approved.

    `babysit-head:fe4b800253a66fe910bed46d08a073830c7d6c68`

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37627355771) · gpt54 · 18.2 AIC · ⌖ 4.44 AIC · ⊞ 16.4K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)

    <!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37627355771, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37627355771 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  14. MaggieAppletonhead-ref-force-pushed 2c993ec8fd56last month on GitHub

  15. MaggieAppletonhead-ref-force-pushed 5a49fa59879clast month on GitHub

  16. MaggieAppletonreferenced this in Put Research last in the slash menu and add icons and hints #290last month on GitHub

  17. MaggieAppletonhead-ref-force-pushed 19e3979ce53alast month on GitHub

  18. MaggieAppletonhead-ref-force-pushed 3f82c90f115clast month on GitHub

  19. MaggieAppletonmerged bb50010d59f1last month on GitHub

  20. MaggieAppletonclosedlast month on GitHub

  21. MaggieAppletonhead-ref-deletedlast month on GitHub

  22. MaggieAppletonreferenced this in Adapt tablet navigation to the measured workspace #288last month on GitHub

  23. MaggieAppletonreferenced this in Let short tables fill the prose width #386last month on GitHub