fix(landing): make the install command readable and the copy button real - #286
Merged
Conversation
Found by driving the page in a browser at 375px and 1440px. The hero hands the visitor one command, and on a phone it did neither of the two things it exists to do. **The command was cut off.** `<code className="flex-1 truncate">` is `overflow: hidden` with an ellipsis, not a scroll container, so at 375px 107px of the 370px string sat behind the ellipsis with no way to reach it — not by scrolling, not by selecting. A visitor on a phone could not read `uvx --from simple_module_cli smpy new my-app`. It now wraps, so the whole command is on screen at every width (two lines at 375px, one at 1440px). **The copy button did nothing.** It carried `aria-label="Copy command"` and no handler at all — not a broken handler, no `onClick` property on the element. That is worse than having no button, because the label asserts an action to assistive tech that the control cannot perform. It now writes to the clipboard, swaps to a check for two seconds, and announces through an `aria-live` region, since a silent icon swap tells a screen-reader user nothing. A rejected `writeText` (insecure origin, permission policy) stays quiet rather than claiming success — the command is on screen and selectable, which is the fallback either way. Extracted to `host/client_app/components/CopyCommand.tsx` rather than inlined: Landing.tsx was at 271 of its 300-line budget, and the widget now holds state. It lives outside `pages/` deliberately — `pages.ts` globs `./pages/**/*.tsx` and registers every match as an Inertia page, so a component under there would have become an SM003 orphan. Two more untranslated-text bugs found while in here: - `authCta()` returned a hardcoded English `'Sign up'`/`'Sign in'`, which the landing CTA rendered without `t()`. Every non-English visitor got English there. `make ci-check-untranslated` cannot see this — a string reaching the screen through a returned object is exactly the taint analysis blind spot that check documents. It now returns a catalog key, so omitting the translation is a type error rather than a silent one. Verified with `es` enabled: the CTA reads "Iniciar sesión". - `<html lang>` was hardcoded `"en"` and nothing ever updated it, so every translated page told assistive tech to pronounce it with English phonetics. Now reads `request.state.locale`, which LocaleMiddleware has already resolved before any route runs. App-wide rather than landing-only, but one line and the same defect class. Five tests cover the widget, including the inert-button regression directly. Confirmed they catch it: reverting `onClick` fails three of them, restoring it passes all five. `host/client_app/tsconfig.json` now excludes `*.test.tsx`, matching packages/ui — these specs rely on jest-dom matcher augmentation registered through vitest.setup.ts, not the compiler's `types`, so a plain `tsc -p` reports every `toBeInTheDocument` as missing. This is the first spec under host/client_app, which is why the gap had not surfaced. Verified: make lint clean, 2152 passed / 2 skipped, 127 JS passed (was 122), make doctor 0 errors (1 pre-existing unrelated SM003). Browser- checked at 375px and 1440px, in English and Spanish, with the copy button exercised and the announcement observed setting and clearing. Claude-Session: https://claude.ai/code/session_01JJtbN97VhtDr28Fuy5JKEF
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Deploying simple-module-python with
|
| Latest commit: |
fe9940d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a538f6ac.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-landing-text.simple-module-python.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The hero hands the visitor one command. On a phone it did neither of the two things it exists to do. Found by driving the page at 375px and 1440px.
The command was cut off
<code className="flex-1 truncate">isoverflow: hiddenwith an ellipsis — not a scroll container. At 375px that hid 107px of the 370px string with no way to reach it: not by scrolling, not by selecting past the clip.It now wraps: the whole command is on screen at every width (two lines at 375px, one at 1440px).
The copy button did nothing
It carried
aria-label="Copy command"and no handler at all — not a broken one, noonClickproperty on the element. Worse than having no button, because the label asserts an action to assistive tech that the control cannot perform.It now writes to the clipboard, swaps to a check for two seconds, and announces through an
aria-liveregion — a silent icon swap tells a screen-reader user nothing. A rejectedwriteText(insecure origin, permission policy) stays quiet rather than claiming success; the command is on screen and selectable either way.Two more untranslated-text bugs found while in here
authCta()returned hardcoded English.'Sign up'/'Sign in'rendered withoutt(), so every non-English visitor got English in the landing CTA.make ci-check-untranslatedcannot see this — a string reaching the screen through a returned object is exactly the taint-analysis blind spot that check documents. It now returns a catalog key, making an omission a type error instead of a silent one. Verified witheson: the CTA reads "Iniciar sesión".<html lang>was hardcoded"en"and nothing updated it, so every translated page told assistive tech to use English phonetics. Now readsrequest.state.locale, already resolved by LocaleMiddleware before any route runs. App-wide rather than landing-only, but one line and the same defect class.Notes on the shape of the change
Extracted to
host/client_app/components/CopyCommand.tsxrather than inlined — Landing.tsx was at 271 of its 300-line budget and the widget now holds state. It sits outsidepages/deliberately:pages.tsglobs./pages/**/*.tsxand registers every match as an Inertia page, so a component under there would have become an SM003 orphan.host/client_app/tsconfig.jsonnow excludes*.test.tsx, matchingpackages/ui. These specs rely on jest-dom matcher augmentation registered viavitest.setup.tsrather than the compiler'stypes, so a plaintsc -preports everytoBeInTheDocumentas missing. This is the first spec underhost/client_app, which is why the gap had not surfaced.Verification
Five tests cover the widget, including the inert-button regression directly. Confirmed they catch it: reverting
onClickfails three of five; restoring it passes all five.make lintuv run pytestnpm testmake doctorBrowser-checked at 375px and 1440px, in English and Spanish, with the copy button exercised and the announcement observed both setting and clearing.
https://claude.ai/code/session_01JJtbN97VhtDr28Fuy5JKEF