refactor(ui): rebuild the web UI around the mobile app language - #191
refactor(ui): rebuild the web UI around the mobile app language#191rabble wants to merge 3 commits into
Conversation
Deploying divine-web with
|
| Latest commit: |
59deee0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f26a36bb.divine-web.pages.dev |
| Branch Preview URL: | https://mobile-css-design-update-to.divine-web.pages.dev |
🚀 Preview DeploymentLast updated:
|
|
Self-review note: title marks this as |
1ab620b to
8622c6e
Compare
|
Review note: Blocking before this is ready for review: The new layout primitives use semantic classes that are not defined in the PR. CI is green, but this broad UI draft still needs the missing styles plus mobile/tablet visual QA before it is reviewable. |
rabble
left a comment
There was a problem hiding this comment.
Blocking before this is ready for review:
The new layout primitives use semantic classes that are not defined in the PR. src/components/AppPage.tsx:30-31 renders app-page / app-page__inner, src/components/AppPage.tsx:50-58 uses app-page__header, app-eyebrow, app-title, app-subtitle, and src/components/AppSectionNav.tsx:22-30 uses app-chip-row, app-chip, app-chip-active. I checked the PR head CSS (src/index.css and src/styles/brand-utilities.css) and found no definitions for these classes, and neither CSS file is changed in the PR. Pages migrated to these primitives will render mostly unstyled beyond the small Tailwind max-width wrappers.
CI is green, but this broad UI draft still needs the missing styles plus mobile/tablet visual QA before it is reviewable.
|
Codex PR status review — 2026-07-07 Status: Draft / needs work What I checked:
Result: (Posted by Codex on behalf of @rabble.) |
f3161d1 to
59deee0
Compare
|
@rabble I took over the draft branch and pushed the cleanup in 59deee0. What changed:
Verification:
Please review the takeover changes and mark the PR ready / merge when you are comfortable. I left it as a draft. |
rabble
left a comment
There was a problem hiding this comment.
Review pass complete. Of the three UI drafts (#190, #191, #511) this is the strongest: small, clean, mergeable, recently rebased, brand-safe (no uppercase/gradient/lucide violations), with a primitives CSS test and screenshot verification.
One decision needed before this can land — sequencing:
- This rebuilds the shared shell/breakpoints/page primitives, the same territory as #190 (BottomNav/AppLayout) and partially moot if #511 showcase mode ships (it removes the web homepage feed this restyles).
MarketingHeader.tsxis also touched by #511 — that's the source of #511's current merge conflicts.
Recommend: decide landing order explicitly. If #511 wins, this PR should rebase onto it and scope down to the app surfaces showcase mode keeps. If full mode survives, land this first as the base and close #190.
|
Clearing the stale review request on jalcine — no longer an active contributor on Divine. This PR is still a draft, so per the review runbook I'm not re-requesting a team; request |
|
Correcting my comment above: I could not actually remove the review request. GitHub rejects it with Diagnosed it — it's not a permissions problem:
So the request can't be cleared without either putting someone else in that slot or changing the PR's state. Since this is a draft and the review runbook says not to request review on drafts, I'm not going to substitute a reviewer unilaterally. @rabble — your call between marking it ready (normal |
Review —
|
| File | Strings |
|---|---|
HashtagDiscoveryPage.tsx |
"Conversation clusters", "Hashtags", "Follow live topics, niches, and recurring community memes." |
ListsPage.tsx |
"Collect and curate", "Build collections, follow community curation, and move between creator workflows faster." |
UniversalUserPage.tsx |
"Profile lookup", "Finding user", "User not found", "Redirecting", + two descriptions |
The locale-parity test only checks that the 20 locale files agree with en; it can't see hardcoded JSX, so this passes CI and ships English to every locale. UniversalUserPage is the sharpest case — the new header copy sits directly above t('universalUserPage.userNotFound'), so the same screen is half-translated.
Copy reads corporate
Against docs/brand/TONE_OF_VOICE.md (Candid Simplicity, "casual-direct, never corporate"), "Conversation clusters" and "move between creator workflows faster" are the wrong register. Compare the house examples: "Nothing looping yet. Go find your people." Worth a pass before these get keys.
The new guardrail test has a gap
Nice to see tests/brand/app-primitives-css.test.ts added alongside the primitives. One weakness: it validates with
!brandUtilitiesCss.includes(`.${className}`)a substring check, so .app-page is satisfied by the presence of .app-page__inner alone. A word-boundary match (new RegExp(\.${className}\b)) would close it. Also, getReferencedPrimitiveClasses only scans lines literally containing className, so a class composed in a variable or split across lines by cn() is invisible to it.
Smaller
AppPage.tsx,AppSectionNav.tsx,CreatorSectionNav.tsx,DiscoverySectionNav.tsxhave noABOUTME:header. 69 of 114 components carry one, so it's a convention rather than a rule — flagging since these are new shared primitives..app-eyebrowcorrectly avoidstext-transform: uppercaseand setsletter-spacing: 0, which is the right call against the no-all-caps rule. Worth a comment in the CSS so nobody "fixes" it into an uppercase eyebrow later.
What's good
AppPage/AppPageHeader are cleanly typed, the width map is a sensible constraint, and moving pages onto shared primitives removes a lot of ad-hoc container mx-auto wrappers. Shipping a CSS guardrail test with new CSS primitives is the right instinct.
Summary: the miscasing is the only hard blocker; the i18n gap is the one I'd most want fixed before this reaches users, since it's the difference between the refactor being locale-neutral and it quietly regressing 19 locales' worth of surface area.
Summary
Related issue
Closes #497
Verification
npm run testnpx vitest run tests/brand/app-primitives-css.test.ts src/components/AppFooter.test.tsx/authenticityat 390x844/proofmodeat 820x1180/u/not-a-real-user-for-pr-191at 390x844/video/not-a-real-video-for-pr-191at 820x1180Notes
origin/main; the previous dirty/unmergeable state is resolved.