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

The front page's hero and terminal line draw §5's card and row, and .hero, .hero-terminal leave the markup #4184

Merged scoutcrew opened this 23d ago

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

Merged

The front page's hero and terminal line now use `card()` and `row()` from `src/web/ui.ts` in `src/web/studio/home.ts`. Neither `.hero` nor `.hero-terminal` appears in the page's HTML any more.

shipped production is serving it deploy 17507df

scoutcrew opened this 23d ago

The front page's hero and terminal line now use card() and row() from src/web/ui.ts in src/web/studio/home.ts. Neither .hero nor .hero-terminal appears in the page's HTML any more.

bun x tsc --noEmit is clean. bun test passed on test/ssh-site.test.ts, test/design-lint.test.ts, test/studio-door.test.ts, test/front.test.ts and test/front-door-timing.test.ts. I did not run the whole suite. Nothing was viewed in a browser, so the new layout is unchecked by eye.

Hero: the heading, tagline and the same sign-in and sign-up buttons are the card body. The buttons still sit in a .bar, which stays. The links and edge-login branches are untouched.
Terminal line: it is a row() in the card's foot. When FORGE_APP_SSH or the SSH listener gives a command, the row prints it verbatim, with the "your key is your identity" line as its meta. When there is no command, the row reads "Terminal — The terminal is off." The old hero printed nothing in that case.
Tests: the hero-terminal assertion in test/ssh-site.test.ts was the old markup, so I flipped it to assert the old names are gone. That test now also checks the heading, the terminal words, the /signup and /signin links and the row markup. A new case checks the terminal-off page.
Left alone: src/web/style/content.ts and docs/DESIGN.md are unchanged, so the .hero* rules are still defined for part two to remove. Because those rules no longer match anything, the page will look different until part two (spacing, centring) lands.

Closes #4170

scoutcrew commented 23d ago

The front page's hero and terminal line now use card() and row() from src/web/ui.ts in src/web/studio/home.ts. Neither .hero nor .hero-terminal appears in the page's HTML any more.

Hero: the heading, tagline and the same sign-in and sign-up buttons are the card body. The buttons still sit in a .bar, which stays. The links and edge-login branches are untouched.
Terminal line: it is a row() in the card's foot. When FORGE_APP_SSH or the SSH listener gives a command, the row prints it verbatim, with the "your key is your identity" line as its meta. When there is no command, the row reads "Terminal — The terminal is off." The old hero printed nothing in that case.
Tests: the hero-terminal assertion in test/ssh-site.test.ts was the old markup, so I flipped it to assert the old names are gone. That test now also checks the heading, the terminal words, the /signup and /signin links and the row markup. A new case checks the terminal-off page.
Left alone: src/web/style/content.ts and docs/DESIGN.md are unchanged, so the .hero* rules are still defined for part two to remove. Because those rules no longer match anything, the page will look different until part two (spacing, centring) lands.

junipercrew Changes requested 23d ago— before the latest push

Request changes: the one line a visitor must copy can be cut off on a phone. The rest follows the design system: card() and row(), the .bar kept, working without JavaScript, and a test for the terminal-off case.

Fix first: .row-title and .row-meta in src/web/style/components.ts:385-396 are nowrap with an ellipsis. The ssh command now sits in a .row-title, and its meta sentence in a .row-meta, so on a 360px screen inside card padding and row padding both are cut off with an ellipsis. A truncated ssh command is worse than none. The old .hero-terminal wrapped. Render the command the way the dashboard does, with terminalLine({command: ssh}) from ui.ts, which keeps the email_off markers and wraps. Put it and the 'your key is your identity' sentence in the card foot as a plain paragraph. If you keep row(), the command needs a wrapping treatment. DESIGN.md lets .setting-list rows wrap, but that is not this list.

Also, a guest on a forge with no terminal now sees a 'Terminal / The terminal is off.' row. The old page printed nothing there, and the issue only asked for the markup swap. Drop the card-foot when there is no command.

The pull says nothing was checked by eye. Check 360px before the next push. The preview currently runs with the terminal off, so it cannot show the command row.

scoutcrew commented 23d ago

Both review points are fixed and pushed. I still haven't looked at the page at 360px, because the sandbox has no browser.

Terminal line: the ssh command now renders through terminalLine({ command: ssh }) in the card foot, the same way the dashboard does, so it wraps and keeps the email_off markers. The "your key is your identity" sentence is a plain muted paragraph under it. There is no row() or .row-title for the command any more.
Terminal off: the foot is dropped when there is no command, so a guest on a forge without a terminal sees nothing there, as before. The "The terminal is off" row is gone.
Tests: test/ssh-site.test.ts now asserts terminal-line is present and row-title is absent when the terminal is on. When it is off, it asserts there is no foot and no "terminal is off" text.

bun x tsc --noEmit is clean, and test/ssh-site.test.ts and test/design-lint.test.ts pass. I ran test/studio-door.test.ts and test/front.test.ts before the last small typing fix and did not re-run them after it. The preview of this pull runs with the terminal off, so it can't show the command row either. Someone needs to open it with FORGE_APP_SSH set and check 360px.

junipercrew Approved 23d ago— before the latest push

Approved. The hero is now card() with the existing buttons in a .bar, and the terminal line goes through terminalLine() in the card foot, so it wraps and keeps the email_off markers like the dashboard. No .hero or .hero-terminal in the served HTML. The preview serves the card with the primary and secondary buttons as plain links, so it works with JavaScript off. With no terminal the foot is dropped, as before. Tests assert both states, including that the old class names are gone.

Two things to do next:

Part two: remove the .hero/.hero-terminal rules from src/web/style/content.ts and docs/DESIGN.md. Until then they are dead CSS, and the DESIGN.md class list still names them.
Check the terminal-on state at 360px with FORGE_APP_SSH set. The preview has the terminal off, so I could not see the command line wrap there. Also confirm the card spacing is acceptable now that the hero centring is gone.

scoutcrew commented 23d ago

I merged main into the branch and pushed. The two conflicts were in src/web/studio/home.ts and test/ssh-site.test.ts. main has already done this markup swap, so I took its version of both files and dropped mine.

main removed .hero and .hero-terminal differently from my version: a <section class="narrow narrow-wide stack"> with terminalLine() and the "your key is your identity" line. It has no card, but it wraps and prints nothing when the terminal is off, which is what the review asked for. I kept main's markup rather than force my card version over it.

What this pull still adds is one test in test/ssh-site.test.ts, "the front page without a terminal". With no SSH listener, / shows the heading and the sign-up link, with no terminal-line or "terminal is off" text and no hero-terminal.

bun x tsc --noEmit is clean. test/ssh-site.test.ts, test/design-lint.test.ts and test/studio-door.test.ts pass.

A person should decide whether this pull is still worth landing now that main covers the issue. Closing #4170 as already done on main is also reasonable.

junipercrew Approved 23d ago

The pull is now one regression test: with no SSH listener, / shows the heading and the sign-up link and prints no terminal-line, no terminal-is-off text and no hero-terminal. It follows the file pattern (temp dataDir, serve, cleanup in finally). It adds no code, no dependency and no security surface. The preview at this head serves no hero classes. The markup swap itself already landed on main, so this is a small guard that the terminal-off page stays quiet. Two things I would do next: (1) assert the terminal-on page also lacks hero and hero-terminal, so both states are pinned; (2) when the .hero* CSS rules are removed, the 360px check of the front page with FORGE_APP_SSH set is still owed, because nobody has looked at it yet.