Skip to content

Central CTA + background bugfix - #4052

Merged
hzub merged 2 commits into
mainfrom
visual-fixes-central-cta
Aug 4, 2026
Merged

Central CTA + background bugfix#4052
hzub merged 2 commits into
mainfrom
visual-fixes-central-cta

Conversation

@hzub

@hzub hzub commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Related issue

https://linear.app/omnigent/issue/OMNI-2360/show-the-project-specific-new-session-design
https://linear.app/omnigent/issue/OMNI-2328/match-the-logo-and-headline
plus a follow-up fix for a regression from #4020.

Summary

  • Central CTA restyle. The landing headline moves to Hanken Grotesk at 400 weight, the copy changes to "What should we build?", and the project variant swaps the bare folder glyph for a pink rounded tile. The header also stops going side-by-side at sm, so the icon and headline stay stacked and centred.
  • Project-aware composer placeholder. On a project landing the placeholder and its aria-label now read "Start a new session in " instead of the generic task prompt, so the input says which project you're starting in.
  • Hanken Grotesk, self-hosted. Added via @fontsource-variable/hanken-grotesk — the same pattern the repo already uses for Geist Mono, so the woff2 files ship in our own bundle with no CDN or runtime dependency. Exposed as the font-display-alt token, which falls back to the UI stack if the font hasn't loaded.
  • New tag-pink token. rgba(240, 1, 150, 0.06) from the design's tag palette, generating bg-tag-pink. Dark mode uses a heavier 15% wash, since 6% is invisible against the dark canvas.
  • Bug fix — the mobile sidebar was see-through. Below md the sidebar is a full-screen overlay on top of the chat. feat(web): make the rails flush boxes and move the canvas gradient #4020 started painting its canvas with the background shorthand, which resets background-color to transparent and silently overrode Sidebar.tsx's max-md:bg-card-solid; the dark stack is entirely translucent, so the conversation showed straight through the open sidebar. An opaque fill is restored under the gradients below md only, so the desktop sidebar keeps the translucency it's meant to have. The override matches the theme rules' specificity and follows them — otherwise the shorthand wins the tie.

web/package.json also picks up two incidental tidies from pnpm add: pdfjs-dist moves into alphabetical order, and two duplicate @tiptap entries that were listed twice are collapsed.

Test Plan

  • npm run test src/index.css.test.ts — 13 passing, including two new guards for the mobile-sidebar contract. Verified they aren't vacuous: stripping the fix makes exactly those 2 fail, restoring it returns 13/13.
  • npm run test for the shell suites (Sidebar, AppShell) — passing.
  • npm run test src/shell/NewChatDialog* — 226 passing. Two landing-screen assertions still expected the old "What should we do?" headline and are updated; the visual-suite docstring and README references are updated too.
  • npm run build — the Hanken Grotesk woff2 files are emitted into our own assets, confirming nothing resolves to a CDN at runtime.
  • npm run type-check and prettier --check clean.
  • Manual: narrowed the viewport below 768px in both themes and opened the sidebar — solid, no chat bleeding through. Checked the landing screen with and without a project selected.

Demo

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

The sidebar fix has real regression cover: the new index.css.test.ts guards fail if the opaque-fill rule is removed, loses the cascade to the theme rules, or stops covering either theme — which is exactly how the bug slipped through in #4020 (a shorthand quietly resetting background-color, with nothing pinning it).

The placeholder change also moves the composer's aria-label, so it's behavioural for screen readers; no test queried that label (the suites use the data-testid), and it was checked by eye on both landing variants.

The CTA restyle is appearance only — font, colour, and layout classes with no behavioural surface — so it's verified by eye in both themes and both landing variants, and by the committed visual baselines.

Changelog

The landing screen headline has a new look, and the sidebar is no longer transparent over the conversation on narrow screens.

@github-actions github-actions Bot added the size/M Pull request size: M label Aug 4, 2026
@hzub hzub added update-ui-snapshot Used to tell CI that the baseline UI snapshot should be regenerated skip-e2e-ui-test labels Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

UI Snapshot doesn't match the committed baseline.

If this UI change is intentional, update the baseline — each path renders in the same pinned image, so the result matches this gate:

  • Label the PR (recommended): add the update-ui-snapshot label — the bot regenerates the baseline in the pinned image, pushes it back here, and re-runs the checks.
  • Locally with Docker: run tests/e2e_ui/visual/regen_baseline_docker.sh, review the PNG, then commit + push.

Diff PNGs (expected_=baseline, actual_=your render, diff_) are in the run artifact. Full guide: tests/e2e_ui/visual/README.md.

@omnigent-ci

omnigent-ci Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Review: Central CTA + background bugfix

1. Blocking issues

Headline copy change breaks two existing tests. NewChatDialog.tsx changes the hero heading from "What should we do?" to "What should we build?", but two tests in web/src/shell/NewChatDialog.test.tsx still assert the old string and will now fail:

  • Line 787: expect(screen.getByText("What should we do?")).toBeTruthy();
  • Line 1862: expect(screen.getByText("What should we do?")).toBeTruthy();

getByText throws when the text is absent, so both NewChatLandingScreen tests break. Either update these assertions to "What should we build?" or revert the copy. This is a real, present regression in the changed code.

2. Security vulnerabilities

None. No lockfile security concerns: @fontsource-variable/hanken-grotesk@5.3.0 is a newly declared dependency in web/package.json (^5.3.0) and the pin satisfies it. The @anthropic-ai/claude-code engines bump (>=18>=22) is upstream package metadata re-resolved into the lockfile, not a version/integrity change, and CI already runs Node 22. No new package-json extras added.

3. Non-blocking notes

  • Duplicate package.json cleanup looks correct but verify install. The diff removes the duplicated @tiptap/extension-link / @tiptap/extension-table keys and relocates pdfjs-dist into sorted order — good. Duplicate JSON keys were previously silently overwritten, so behavior is unchanged, but confirm pnpm install --frozen-lockfile still passes after the reorder.
  • --font-display-alt / bg-tag-pink are single-use. font-display-alt and bg-tag-pink are currently referenced only in the project landing branch of NewChatDialog.tsx. Fine as a design-token seam, just noting there's no other consumer yet.
  • Mobile sidebar fix cascade is sound. The new @media (width < 48rem) rule matches the same selectors (html:not(.dark) .conversations-sidebar, .dark .conversations-sidebar) at equal specificity and is declared after the theme rules (lines 384/406 → new rule later), so background-color: var(--card-solid) wins the tie as intended. --card-solid is defined for both themes. The added regression test guards ordering and both-theme coverage — good.

4. Summary

The CSS work (mobile sidebar opacity fix + regression test, pink tag surface token, Hanken Grotesk self-hosted font) is well-scoped, correctly reasoned about the cascade, and cleans up duplicate package.json entries with no security concerns. The one blocker is that the landing-screen headline copy change to "What should we build?" leaves two existing tests asserting the old "What should we do?" string, which will fail CI. Fix those assertions (or revert the copy) and this is ready to merge.


Automated review by Polly · workflow run

Landing screen:
- Headline moves to Hanken Grotesk at 400 weight ("What should we build?"),
  self-hosted via @fontsource-variable so no CDN is involved, exposed as the
  `font-display-alt` token.
- The project variant swaps the bare folder glyph for a pink rounded tile,
  using a new `tag-pink` token from the design's tag palette.
- The composer placeholder and its aria-label now name the selected project
  ("Start a new session in <project>") instead of always reading the generic
  task prompt.

Bug fix — the mobile sidebar was see-through. Below md the sidebar is a
full-screen overlay on top of the chat, but the per-theme canvas rules paint
it with the `background` shorthand, which resets background-color and silently
overrode Sidebar.tsx's max-md:bg-card-solid; the dark stack is entirely
translucent, so the conversation showed straight through. Restores an opaque
fill under the gradients below md only, at matching specificity and after the
theme rules, so desktop keeps its intended translucency.

Adds regression tests for that contract, and updates the landing-screen tests
and visual-suite docs for the new headline.

Co-authored-by: Isaac
Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
@hzub
hzub force-pushed the visual-fixes-central-cta branch from 4702e30 to 092d2ac Compare August 4, 2026 13:30
@hzub hzub added update-ui-snapshot Used to tell CI that the baseline UI snapshot should be regenerated and removed update-ui-snapshot Used to tell CI that the baseline UI snapshot should be regenerated labels Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

✅ Regenerated the visual baseline(s) in the pinned Playwright image and pushed to this PR. CI will re-run on the new commit.

@github-actions github-actions Bot removed the update-ui-snapshot Used to tell CI that the baseline UI snapshot should be regenerated label Aug 4, 2026
@hzub
hzub merged commit 5e9f947 into main Aug 4, 2026
67 checks passed
@hzub
hzub deleted the visual-fixes-central-cta branch August 4, 2026 13:55
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

This PR only tweaks UI copy ("What should we build?"), adds a font/color token, fixes a mobile sidebar transparency bug, and reorders dependencies — cosmetic/internal changes with no user-facing surface, integration, or documented behavior affected.

Auto-classified on merge. Set the label manually before merging to override. · run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-doc-update Merged PR does not need a docs update size/M Pull request size: M skip-e2e-ui-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants