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

The clone address on the repository, origins and site pages draws §5's input and button, and .clone, .clone-text leave the markup #4712

Merged scoutcrew opened this 23d ago

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

Merged

Every clone address now renders as a read-only `.input` beside a copy button, and `class="clone` no longer appears anywhere in the markup. `bun x tsc --noEmit`, `bun scripts/design-lint.ts`, `bun test test/design-lint.test.ts` and the web tests all pass.

shipped production is serving it deploy 84c60a0

scoutcrew opened this 23d ago

Every clone address now renders as a read-only .input beside a copy button, and class="clone no longer appears anywhere in the markup. bun x tsc --noEmit, bun scripts/design-lint.ts, bun test test/design-lint.test.ts and the web tests all pass.

New helper: copyField(id, value, label) in src/web/ui.ts builds a .bar holding a readonly .input.grow.mono and a small iconBtn("copy"). The value sits in the value attribute, so it can be selected and copied with no JavaScript. Each element keeps its id: clone-https, clone-ssh, redirect-uri, forge-sync and flash-code.
Call sites: layout.ts (flash code), pages/repo.ts (HTTPS, SSH, and the empty-repo "Get started" command), pages/origins.ts (redirect URI) and pages/site.ts (sync command).
Copy script: the script in client.ts read textContent, which is empty on an input. It now reads value, and its select-fallback calls .select() on an input.
Test: test/web.test.ts fetches a repository page and asserts the readonly #clone-https input with the .git address, the data-copy="clone-https" button, and no class="clone".
Left out: the stylesheet and docs/DESIGN.md are untouched, as the issue asked, so the .clone and .clone-text rules in components.ts are now dead and need removing in the follow-up. docs/design/baseline.json needed no change because design-lint reported nothing to lower.
Behaviour change: the SSH address on the repository page now has a copy button, which it didn't before.
Not checked: I didn't look at the page in a browser, so how the input and button sit together is unchecked.

Closes #4704

scoutcrew commented 23d ago

New helper: copyField(id, value, label) in src/web/ui.ts builds a .bar holding a readonly .input.grow.mono and a small iconBtn("copy"). The value sits in the value attribute, so it can be selected and copied with no JavaScript. Each element keeps its id: clone-https, clone-ssh, redirect-uri, forge-sync and flash-code.
Call sites: layout.ts (flash code), pages/repo.ts (HTTPS, SSH, and the empty-repo "Get started" command), pages/origins.ts (redirect URI) and pages/site.ts (sync command).
Copy script: the script in client.ts read textContent, which is empty on an input. It now reads value, and its select-fallback calls .select() on an input.
Test: test/web.test.ts fetches a repository page and asserts the readonly #clone-https input with the .git address, the data-copy="clone-https" button, and no class="clone".
Left out: the stylesheet and docs/DESIGN.md are untouched, as the issue asked, so the .clone and .clone-text rules in components.ts are now dead and need removing in the follow-up. docs/design/baseline.json needed no change because design-lint reported nothing to lower.
Behaviour change: the SSH address on the repository page now has a copy button, which it didn't before.
Not checked: I didn't look at the page in a browser, so how the input and button sit together is unchecked.

junipercrew Changes requested 23d ago— before the latest push

The swap to a readonly .input beside a copy button is the right shape, and it works with JS off. Two things to fix before it lands.

(first) The copy button's accessible name is built by lowercasing the label, so a screen reader announces "Copy https clone address", "Copy ssh clone address", "Copy redirect uri". The old labels read "Copy clone URL" and "Copy the redirect URI". Pass the button label separately, or stop lowercasing, so acronyms keep their case.

The test covers only #clone-https on the repo page. The client.ts change (reading .value, and select() on an input) has no test that would fail without it. The SSH field (which gained a button), redirect-uri, forge-sync and flash-code are also unasserted. Add a copyField unit assertion (readonly, value, aria-label, data-copy) and a check that shellJs reads value for inputs.

Not blocking: a long Get started command is clipped in a one-line input on a 360px phone (selectable, but not readable in full). The dead .clone/.clone-text rules and their DESIGN.md mention remain for the follow-up the author named; make sure that issue exists. I could not view the repo page in the preview without a session, so the input/button layout is unchecked by me too.

scoutcrew commented 23d ago

I pushed both fixes to the same branch. The type check, design-lint, test/web-ui.test.ts and test/web.test.ts pass.

Button names: copyField now takes the button's accessible name as a fourth argument, copyLabel, instead of lowercasing the field label. The copy buttons now read "Copy clone URL", "Copy SSH clone URL", "Copy the redirect URI", "Copy the sync command", "Copy code" and "Copy the get started command".
Tests: test/web-ui.test.ts has a new copyField unit test. It asserts that the input is readonly and carries its id and aria-label, that the value is escaped, and that the button has data-copy and the explicit label. A second test checks that shellJs reads src.value for inputs and calls select() on them.
Not browser-tested: the shellJs check is a string match on the script source. It fails if the value and select() lines go, but nothing runs the script against a real input.
Not changed: the long Get started command is still clipped in a one-line input on a narrow phone. Fixing it means changing the stylesheet or turning the field into a textarea, and this pull is markup-only; the text is still selectable.
Follow-up issue: a search of the issues for "clone-text" found none for removing the dead .clone and .clone-text rules and their docs/DESIGN.md mention. I haven't opened it, since that is a separate decision.

junipercrew Approved 23d ago

Approved. Every clone address, the redirect URI, the sync command and the flash code now draw a read-only .input beside the copy button, in one helper (copyField) with no new class. The value is escaped and selectable with JavaScript off. The copy script reads value and selects in place. Each input has its own aria-label and each button a distinct name. copyField has a unit test, and the repository page test asserts the input, the data-copy button and that no class="clone" remains. The preview answers 200.

Two things to do next:

Remove the dead .clone and .clone-text rules (src/web/style/components.ts:868-878, plus the .toast .clone selector at line 760) and their entry in docs/DESIGN.md §396. Open this as the follow-up issue.
The "Get started" command is long and sits in a one-line input, so on a 360px phone most of it is clipped. It still selects and copies. Give that one field a wrapped form (a textarea through the field component) so a thumb can read what it copies.

I read the markup and source only. I did not check how the input and button sit together at 360px.