Repository navigation
fix(app): show the ClickStack name as text and update the favicon - #3579
Conversation
Show the logo symbol and the brand name as separate elements with an 8px gap, instead of the outlined SVG wordmark. The name comes from useBrandDisplayName so a downstream build can change it in one place. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the favicon PNGs, ICO, and apple touch icon with the new set. Drop the SVG favicon link: the new set has no SVG, and browsers that prefer SVG would keep showing the old mark. Co-authored-by: Cursor <cursoragent@cursor.com>
Set the expanded sidebar name to Inter 18px, weight 500, and size the symbol to the lockup's 23px height. The 8px gap is measured from the hexagon itself. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 6f260c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Duplicate code analysisNo new duplicate code introduced by this PR. jscpd compared this branch against |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| display: flex; | ||
|
|
||
| svg { | ||
| width: calc(23px * 38.7979 / 44.7998); |
There was a problem hiding this comment.
🔵 minor — The 23px mark size is set twice: once in the SCSS and once in useLogomark({ size: 23 })
The SVG's square width/height attributes come from size, and this rule then overrides both, so changing size in AppNav.tsx:112 does nothing until someone also edits this rule. Fix it in brand/Logomark.tsx instead: set width={size * 38.7979 / 44.7998} and height={size} so the box fits the hexagon. Then drop this CSS override. The collapsed rail's 22px mark also has the square box, so it benefits too.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in a9e02b8. Logomark now sets its width from the viewBox ratio (size is the height), and the CSS override is gone. The collapsed rail uses the same component, so it gets the hugged box too.
| style={{ minWidth: 0 }} | ||
| > | ||
| {wordmark} | ||
| <div className={styles.logoLockupMark}> |
There was a problem hiding this comment.
🔵 minor — The ClickStack lockup now has two versions: inline text in AppNav and the SVG Wordmark in LandingHeader
brand/index.tsx:28-30 says a downstream build replaces the brand module to change the name and marks without touching call sites. AppNav now builds its own lockup from useLogomark and useBrandDisplayName (hard-coded 8px gap, 18px/500 text), while LandingHeader.tsx:16 still renders the SVG wordmark. The two headers now look different, and replacing Wordmark no longer changes the sidebar. Move the text lockup into brand/ and have both callers use it, either by updating Wordmark or adding a useLockup hook.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in a9e02b8. The mark-and-text lockup is Wordmark, and both the sidebar and the landing header render useWordmark(). Replacing the brand module changes the sidebar again. AppNav is back to how it was on main, so this no longer edits it.
| <div className={styles.logoLockupMark}> | ||
| {expandedLogomark} | ||
| </div> | ||
| <Text fz={18} fw={500} truncate="end"> |
There was a problem hiding this comment.
🔵 minor — truncate="end" has no effect because the Text never shrinks below its content width
Inside the wrap="nowrap" Group, the Text is a flex item with the default min-width: auto, so it never gets narrower than its text and the ellipsis never shows. A longer downstream brand name would push the badge and collapse button out of the header instead. Add miw={0} (or style={{ minWidth: 0 }}) to the Text so truncate works. InstanceLabel.tsx:25 already does this for the badge in the same row.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in a9e02b8. The name now lives in Wordmark, and both the text and its row have miw={0}, so a long brand name ellipsizes instead of pushing the badge out of the header.
PR Review
2 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 2 minor 1 posted as inline comment(s) on the changed lines. 1 listed below. Findings outside the changed lines1 minor
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Deep Review✅ No critical issues found. This is a small, well-scoped frontend/branding change: 🔵 P3 nitpicks (3)
Reviewers (6): correctness, maintainability, testing, project-standards, kieran-typescript, previous-comments. Testing gaps: the symbol-fit width ratio is not pinned to an exact value; long-name truncation behavior is uncovered (and not observable in jsdom); the 8px mark-to-text gap and the favicon assets are not asserted (expected for static assets). No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
E2E Test Results✅ All tests passed • 463 passed • 1 skipped • 1482s
Tests ran across 4 shards in parallel. |
AppNav was building its own mark-and-text lockup, so replacing the brand module no longer changed the sidebar. The wordmark is that lockup now, and the mark's box follows the hexagon instead of a square CSS override. Co-authored-by: Cursor <cursoragent@cursor.com>
Browsers that prefer SVG had no icon after the raster set replaced the old mark. Serve the new artwork as an SVG alongside the PNG and ICO. Co-authored-by: Cursor <cursoragent@cursor.com>
| import React from 'react'; | ||
| import { Group, Text } from '@mantine/core'; | ||
|
|
||
| import { useBrandDisplayName } from '@/brand'; |
There was a problem hiding this comment.
🔵 minor — Circular import: Wordmark imports @/brand, which imports Wordmark
brand/index.tsx:6 imports ./Wordmark, and Wordmark now imports useBrandDisplayName from @/brand. This only works because the hook is called at render time, not at module load. Break the cycle: have useWordmark() in index.tsx pass the name as a prop (<Wordmark name={BRAND_DISPLAY_NAME} />), so Wordmark no longer imports @/brand. The name would still be defined in one place, as index.tsx says it should be.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in 6f260c6. useWordmark() passes the name in, so Wordmark no longer imports @/brand.
| return ( | ||
| <svg | ||
| width={size} | ||
| width={size * MARK_WIDTH_RATIO} |
There was a problem hiding this comment.
🔵 minor — Changing Logomark's width affects every caller, not just the wordmark
size used to set a square box and now sets only the height, so each existing caller gets a narrower mark: the collapsed rail at AppNav.tsx:109 (22 → ~19px wide), Spotlights.tsx:27 (16 → ~13.9px), and the Brand stories. The PR says the collapsed rail is unchanged, but its centered icon box shrinks. Either confirm this is intended and note it in the changeset, or keep the default box square and set the tight width only from Wordmark (for example, an optional width prop).
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in 6f260c6. The default box is square again. Only the wordmark passes fit="symbol", so the collapsed rail, Spotlights, and the Brand stories keep the width they had.
Pass the brand name into Wordmark so it does not import the brand barrel that imports it. Hug the hexagon only in the wordmark. Other callers keep the square box they sized against. Co-authored-by: Cursor <cursoragent@cursor.com>
| // which imports this file. | ||
| const MARK_HEIGHT = 23; | ||
|
|
||
| export default function Wordmark({ name }: { name: string }) { |
There was a problem hiding this comment.
🔵 minor — Wordmark rewrite also changes the landing header, which the PR says is unchanged
The PR description says brand/Wordmark.tsx is unchanged and that LandingHeader still uses it. This diff rewrites the component, and LandingHeader.tsx:16,40 renders it through useWordmark(). So the login, register and landing pages now show the 23px mark plus 18px text instead of the 121x25 outlined SVG. Either confirm and screenshot that change for those pages too, or give the sidebar its own lockup and leave the landing header alone. Then fix the description to match.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Updated the description. The shared wordmark is intentional: login and register use useWordmark() through LandingHeader, so they should match the sidebar. The description now says that, and asks for a screenshot of that header too. The collapsed rail stays on the square mark.
Summary
The brand name was the outlined SVG inside
brand/Wordmark.tsx. This PR rewrites that component as live text next to the hexagon, so the name is selectable and matches the ClickStack lockup.useWordmark()is the only caller path, so every header that uses it changes together:LandingHeader).useWordmark()passes the name in, so a downstream build still changes it in one place.Logomarkstays a square box everywhere except inside the wordmark, so Spotlights and the Brand stories keep their old width.The branch also replaces the favicon set with the new artwork (black rounded square, yellow hexagon, dark bolt):
favicon.ico,favicon-16x16.png,favicon-32x32.png, andapple-touch-icon.pngare replaced.favicon.svgis the same artwork, linked asimage/svg+xmlso browsers that prefer SVG use it. The PNG and ICO files stay as fallbacks.The favicon change is a separate commit from the wordmark. Each has its own changeset. Either can be split into its own PR if reviewers prefer.
Screenshots or video
Not captured. The Before/After table is blank because I did not run the app. Please add pairs, on
mainand this branch, for the expanded sidebar, the collapsed rail, the login or register header, and the browser tab icon.How to reproduce
Starting point:
yarn app:dev:localconnected to the demo server (onboarding → "Connect to Demo Server"), or play.hyperdx.io.On
main:/login. The header shows the same outlined logo.On this branch:
/login. The header shows the same text lockup as the sidebar.How to test on Vercel preview
Preview routes: /search
Steps:
Tests
npx tsc --noEmit, ESLint, and stylelint pass on the changed files.Wordmark.test.tsxrenders the lockup and checks that the name is "ClickStack" and that the mark is narrower than it is tall. A second case checks thatuseLogomarkstill returns a square box.References