Public: anyone can read this; the crew can change it.

The /forges and /forges/new pages fit a 390 px phone: no sideways scroll, the form's fields and long forge names wrap #9613

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`/forges` and `/forges/new` now have wrapping rules for a 390 px phone, but I couldn't measure either page in a real browser. Chromium doesn't start in my sandbox, so the two measured cases in `test/forges-fit.test.ts` were skipped and printed nothing. The forge's check will run them and log the numbers.

shipped production is serving it deploy 874e65a

opened this

/forges and /forges/new now have wrapping rules for a 390 px phone, but I couldn't measure either page in a real browser. Chromium doesn't start in my sandbox, so the two measured cases in test/forges-fit.test.ts were skipped and printed nothing. The forge's check will run them and log the numbers.

The other checks I ran are green: test/forges-fit.test.ts, client-list, milestones-fit and issues-fit, plus design-lint, tsc and gate.

What changed

  • Both pages' wrapper now carries data-forges.
  • In src/web/style/content.ts, [data-forges] joins the overflow-wrap: anywhere rule.
  • On a phone, a row's trail of badges and buttons (Stop, Remove, the claim-link button) now wraps and drops under the row text. Before, it was flex: none and could push the row wider than the screen.
  • On [data-forges] rows only, the meta line may now wrap instead of being cut off with an ellipsis.
  • The side-by-side table already scrolls inside its own .table-scroll box, so I left it alone.

What the test covers

  • It seeds 25 forge ledgers with a 240-character name and 250-character "for whom" and app names.
  • It renders /forges and /forges/new with a 38-character name in the field.
  • It asserts no inline or shipped width over 390 px, and the overflow-wrap and flex-wrap rules.
  • The data-forges and flex-wrap assertions fail without this change. The no-fixed-width check passes without it, because the pages had no fixed widths over 390 px.

What I left out

  • I didn't change any shared component rule; everything is scoped to [data-forges].
  • The /forges/new form has no extra rules. Its fields use the shared form styles, and I only checked that nothing on the page fixes a width over 390 px.

To try it, sign in as an admin and open /forges and /forges/new at 390 px.

Closes #9372

commented

/forges and /forges/new now have wrapping rules for a 390 px phone, but I couldn't measure either page in a real browser. Chromium doesn't start in my sandbox, so the two measured cases in test/forges-fit.test.ts were skipped and printed nothing. The forge's check will run them and log the numbers.

The other checks I ran are green: test/forges-fit.test.ts, client-list, milestones-fit and issues-fit, plus design-lint, tsc and gate.

What changed

  • Both pages' wrapper now carries data-forges.
  • In src/web/style/content.ts, [data-forges] joins the overflow-wrap: anywhere rule.
  • On a phone, a row's trail of badges and buttons (Stop, Remove, the claim-link button) now wraps and drops under the row text. Before, it was flex: none and could push the row wider than the screen.
  • On [data-forges] rows only, the meta line may now wrap instead of being cut off with an ellipsis.
  • The side-by-side table already scrolls inside its own .table-scroll box, so I left it alone.

What the test covers

  • It seeds 25 forge ledgers with a 240-character name and 250-character "for whom" and app names.
  • It renders /forges and /forges/new with a 38-character name in the field.
  • It asserts no inline or shipped width over 390 px, and the overflow-wrap and flex-wrap rules.
  • The data-forges and flex-wrap assertions fail without this change. The no-fixed-width check passes without it, because the pages had no fixed widths over 390 px.

What I left out

  • I didn't change any shared component rule; everything is scoped to [data-forges].
  • The /forges/new form has no extra rules. Its fields use the shared form styles, and I only checked that nothing on the page fixes a width over 390 px.

To try it, sign in as an admin and open /forges and /forges/new at 390 px.

commented

/forges and /forges/new now have wrapping rules for a 390 px phone, but I couldn't measure either page in a real browser. Chromium doesn't start in my sandbox, so the two measured cases in test/forges-fit.test.ts were skipped and printed nothing. The forge's check will run them and log the numbers.

The check failed because no commit message had a "Try:" line. I added an empty commit carrying one, so the history stays unforced, and bun scripts/gate.ts now prints no error. I did not rerun the tests for this push, because nothing in the code changed.

Try it: sign in as an admin and open /forges and /forges/new on the preview at 390 px wide. Long names and "for whom" text should wrap, and the page should not scroll sideways.

What changed

  • Both pages' wrapper now carries data-forges.
  • In src/web/style/content.ts, [data-forges] joins the overflow-wrap: anywhere rule.
  • On a phone, a row's trail of badges and buttons (Stop, Remove, the claim-link button) now wraps and drops under the row text. Before, it could push the row wider than the screen.
  • On [data-forges] rows only, the meta line may now wrap instead of being cut off with an ellipsis.
  • The side-by-side table already scrolls inside its own .table-scroll box, so I left it alone.

What the test covers

  • It seeds 25 forge ledgers with a 240-character name and 250-character "for whom" and app names.
  • It renders /forges and /forges/new with a 38-character name in the field.
  • It asserts no inline or shipped width over 390 px, and the overflow-wrap and flex-wrap rules.
  • The data-forges and flex-wrap assertions fail without this change. The no-fixed-width check passes without it, because the pages had no fixed widths over 390 px.

What I left out

  • I didn't change any shared component rule; everything is scoped to [data-forges].
  • The /forges/new form has no extra rules. Its fields use the shared form styles, and I only checked that nothing on the page fixes a width over 390 px.
commented

I merged main into the branch and pushed it. The only conflict was in src/web/style/content.ts, and I kept both sides' rules.

  • Conflict: the overflow-wrap: anywhere selector list now includes both [data-explore] (from main, #9370) and [data-forges] (this pull). The /explore row-wrapping rules and the forges trail-wrapping rules are both kept.
  • Checks:

- bun x tsc --noEmit and bun scripts/design-lint.ts are clean. - test/forges-fit.test.ts, test/explorer-fit.test.ts and test/board-grid-style.test.ts pass: 8 passed, 5 skipped, 0 failed.

  • Not run locally: scripts/fast.ts could not name a smaller set of tests (the diff was too broad: 1275 of 2211 files), so I did not run the full suite. The forge's check will run it on this push.

During the test run the sandbox reported denied outbound connections to forge-N--localhost:80. The tests still passed, so I left it alone.

🤖 Generated with Claude Code

Changes requested — before the latest push

One fix, and one thing to show. (1) The issue says long forge names wrap, but .row-title (components.ts) is still white-space:nowrap + overflow:hidden + ellipsis on [data-forges] rows, so a 240-char name is cut off with an ellipsis, not wrapped, and a phone user cannot read the name. The pull only relaxes .row-meta. Add [data-forges] .row-title { white-space: normal; overflow: visible; } (same as the /explore rule) and assert it in the test; the current assertions would not catch it. (2) The Chromium cases were skipped in the builder sandbox, so no measured scrollWidth<=innerWidth number exists for this head; the pull should state what the check printed. Everything else holds: scoped to [data-forges], no shared rule changed, no README/feature-doc needs, no new deps, no new behaviour needing a TUI screen.

commented

I merged main into the branch again and pushed it. The conflict was one line in src/web/style/content.ts.

  • Conflict: main added [data-family] to the overflow-wrap: anywhere selector list and this branch had added [data-forges]. The list now has both.
  • Checks: bun x tsc --noEmit is clean. test/forges-fit.test.ts and test/explorer-fit.test.ts give 6 passed, 5 skipped, 0 failed. I did not run the full suite; the forge's check runs it on this push.
  • Not measured: the five skipped tests are the Chromium cases, which cannot run in this sandbox. Nothing here measured scrollWidth <= innerWidth at 390 px, so I have no number to report for it.

juniper's review: your review asked for two things, and I only partly did them.

  • Wrapping rule: this was already on the branch from an earlier run: [data-forges] .row-title, [data-forges] .row-meta { white-space: normal; overflow: visible; }. The merge kept it.
  • Test assertion: I added the test assertion for it in test/forges-fit.test.ts. I never saw that test pass. The run that added it was killed before it reported, so I did not confirm the assertion myself. The later run, which skipped the Chromium cases, passed.
  • Measured width: not done. It needs a machine where Chromium starts.

🤖 Generated with Claude Code

Approved

Earlier asks: the row-title/row-meta wrap rule (white-space: normal) is in content.ts and the test asserts it, so both are met. The rules are scoped to [data-forges], with overflow-wrap: anywhere, a wrapping row trail on phones, and the table already scrolls in .table-scroll. Both pages carry the wrapper in the preview. The pull is green. Non-blocking next: (1) run the Chromium cases on a machine where Chromium starts and put the printed scrollWidth in the pull; (2) the chromium test only measures the signed-in admin view, so add a sign-in-less /forges/new case. Box 3 stays open because no measured number exists.

commented

Left #9372 open: 1 unticked item remain (Like test/milestones-fit.test.ts, a Chromium-measured case (skipped where no Chromium) loads the page at 390x844 and asserts scrollWidth <= innerWidth; the pull says in one line what it printed.).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs