💄 Redesign /products index + third-party brand card tokens - #4984
💄 Redesign /products index + third-party brand card tokens#4984hveraus wants to merge 19 commits into
Conversation
Bring /products in line with the /consulting index redesigned in #4939: same page background, container geometry, title scale and card interaction model. - Opt /products into home theming via THEMED_ROUTES, so it picks up the sunken-glow background and dark-mode surfaces. - Split the page into per-product card components under components/products/, with the shared card chrome (geometry, hover, focus) in shared.ts. - Give TinaCMS and YakShaver their own branded cards, spanning two columns from md up. Matched on product name so the treatment follows the product if an editor reorders the CMS list. - Add MoreProductsPanel to fill the trailing gap on the last grid row, sized from the occupied cell count rather than hardcoded. - Namespace TinaCMS brand colours as global --brand-tina-* tokens in styles.css, documenting the WCAG AA contrast measurement behind each value, and expose them through tailwind.config.js. - Fix the title's Tina visual-editing attribute: props-tina-field was never a real attribute, so the h1 never registered as a click target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage report
Test suite run success34 tests passing in 4 suites. Report generated by 🧪jest coverage report action from b725e3b |
There was a problem hiding this comment.
Pull request overview
Updates the /products index to match the redesigned /consulting index by opting into home theming, introducing a shared card interaction model, and adding branded card treatments for third-party products.
Changes:
- Opts
/productsinto home theming and updates page wrapper/geometry to match/consulting. - Replaces the old
PageCardgrid with dedicated product card components (standard + branded TinaCMS/YakShaver) and shared card chrome. - Adds global TinaCMS brand CSS tokens (documented for contrast) and exposes them to Tailwind utilities.
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tailwind.config.js | Exposes TinaCMS brand CSS variables via Tailwind color tokens. |
| styles.css | Introduces namespaced third-party brand tokens for TinaCMS (with contrast notes). |
| public/images/company-logos/TinaCms-Logo-Full-Default.svg | Adds TinaCMS full lockup SVG for branded product card. |
| components/products/yakShaverProductCard.tsx | Adds YakShaver branded card component (always-dark scoped surface + artwork). |
| components/products/tinaProductCard.tsx | Adds TinaCMS branded card component using new brand tokens. |
| components/products/shared.ts | Introduces shared card shell classes and destination label helper. |
| components/products/productCard.tsx | Adds standard product card component using shared chrome and tokens. |
| components/products/moreProductsPanel.tsx | Adds grid “More products coming” filler panel with computed spans. |
| components/layout/homeTheme.tsx | Adds /products to THEMED_ROUTES so it receives home theming. |
| app/(about)/products/products-index.tsx | Rebuilds /products index layout and renders new card components + filler panel. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // overlay is unnecessary, which also means the focus outline can sit on the | ||
| // card itself again instead of on a child that overflow-hidden would clip. | ||
| <CustomLink | ||
| href={product.url ?? ""} |
| // (/products/rewards) through next/link and adds target/rel to the | ||
| // genuinely external ones. | ||
| <CustomLink | ||
| href={product.url ?? ""} |
| }) => { | ||
| return ( | ||
| <CustomLink | ||
| href={product.url ?? ""} |
hveraus
added
the
no-issue
Re-exports the decorative bloom at 500x320 instead of 1670x1069, taking the file from 1.1 MB to 164 KB. Aspect ratio is unchanged (1.5625 vs 1.5622), so the artwork renders identically. Also corrects the hardcoded width/height on the <Image>. next/image derives its srcset from `width`, so leaving 1670 there would have requested 1670w/3840w candidates of a 500px source and given back most of the saving. Addresses the asset-weight finding from the multi-agent review of #4984. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re-exports the decorative bloom at 500x320 instead of 1670x1069, taking the file from 1.1 MB to 164 KB. Aspect ratio is unchanged (1.5625 vs 1.5622), so the artwork renders identically. Also corrects the hardcoded width/height on the <Image>. next/image derives its srcset from `width`, so leaving 1670 there would have requested 1670w/3840w candidates of a 500px source and given back most of the saving. Addresses the asset-weight finding from the multi-agent review of #4984. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
83efb6f to
8c0f6bc
Compare
Follow-up to the review of #4984. Fix - A product with no `url` lost its entire card. All three cards passed `product.url ?? ""` and CustomLink returns a bare fragment when href is falsy, discarding the wrapper along with its className, so the logo plate, copy and footer became loose grid items. `url` is optional in tina/collections/products.tsx, and the design this replaced guarded it via `{page.url && ...}` in pageCards.tsx. New ProductCardShell renders CustomLink when there is a href and the same shell as a plain div when there is not. Guarded here rather than in CustomLink on purpose: that component has ~99 call sites, several inside <p> elements, where emitting a <div> would be invalid HTML and a hydration mismatch. - Covered by __tests__/components/products/productCard.tsx, which fails on the previous implementation. jest.config.ts gains a `@/lib/*` moduleNameMapper entry so components importing cn() are testable at all. Brand cards (TinaCMS, YakShaver) - Dropped the arrow affordance and replaced the destination URL with a "Learn More" chip. The chip is a styled <span>, not a <button> or nested <a>, both of which are invalid inside the card's own <a>. - Chip grows to 105% on card hover; needed transition-all rather than transition-colors for the transform to ease. - Right padding raised to 40px (`pr-10`), kept after `p-6` in the cn() list because tailwind-merge drops a `pr-*` that precedes a `p-*`. - Tina llama watermark enlarged to h-3/4 and pushed to -bottom-4, so overflow-hidden crops the feet deliberately. Standard cards - Logo plate radius raised from rounded-utility (8px) to rounded-card (16px). - Arrow is now filled at rest and only turns and grows on hover, achieved by removing the overrides that forced an outline-to-fill effect and pinned group-hover:scale-100. ArrowCircle's own bg-foreground/text-background defaults give a dark circle in light mode and a white one in dark mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the copy on the /products TinaCMS card so it covers TinaDocs as well as the visual editor. Note the same sentence is still duplicated in content/megamenu/menu.json, which is deliberately left alone here - the mega menu is a separate surface, so the two now differ. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
/deploy |
|
Deployed changes to https://app-sswwebsite-9eb3-pr-4984.azurewebsites.net ℹ️ Staging slots are no longer created automatically - comment |
Tags - New optional `tags` string list on productsList (tina/collections/products.tsx) so the copy stays editable in the CMS like every other product field, with three authored per product. At most three render; blanks and extras are dropped, so an editor can add a fourth without changing a card's height. - Rendered as sentence-case chips with a brand-red stroke and a 16% red wash. The stroke is what makes them read as chips: the neutral hairline they started with measured 1.25:1 against the light card and 1.38:1 against the dark one, where red measures 4.53:1 and 3.99:1. - The label stays neutral rather than going red with the stroke: text-brand is only 3.99:1 on the dark card at 12px, which fails AA. The TinaCMS card keeps white outline chips for the inverse reason - red measures 1.15:1 against its orange field, and a white wash would drop its label to 4.33:1. Chrome - Both brand cards' Learn More chips now come from one shared constant, so the near-black label is identical on each. The TinaCMS one previously used the darkened brand orange. - The footer arrow's disc no longer uses ArrowCircle's full-strength inversion, which measured 16.7:1 against the light card and 19.0:1 against the dark one - more contrast than the card title, for an affordance. Now 1.27:1 / 1.20:1, with the glyph gaining contrast as the disc fades. Overridden at the call site only; the v3 blocks still get the strong version. All contrast figures measured in a browser against the rendered surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
…-page # Conflicts: # tina/tina-lock.json Co-authored-by: hveraus <4745861+hveraus@users.noreply.github.com>
Merge conflicts resolved — the only conflict was in |
…solution Co-authored-by: hveraus <4745861+hveraus@users.noreply.github.com>
Co-authored-by: hveraus <4745861+hveraus@users.noreply.github.com>
The merge of main at b1549ea hand-resolved the conflict in this generated file and left its `eventsCalendar` collection at the pre-#4978 shape, so the lock disagreed with tina/collections/events-calendar.tsx in the same tree: committed lock 26 fields, including an obsolete `headerLayout` source schema 32 fields Missing from the lock were availability, bannerImage, ctaLabel, entryCost, lead, price and venue. `tinacms build --content=local` checks the local schema against the one TinaCloud indexed, so it exited 1 and took `pnpm run build` with it - surfacing only as the Docker `buildx failed` wrapper in CI, since the job logs are not readable without admin rights. Every `PR - build` run since that merge failed this way; the two follow-up commits that hand-patched this file did not fully restore it. Regenerated rather than hand-edited, which is the only reliable way to touch a 30MB generated file. Verified structurally against origin/main's lock: all 46 collections, the entire lookup table and all 20035 GraphQL definitions are now identical to main's, with the sole exception of productsIndex gaining the `tags` field this PR adds (schema plus ProductsIndexProductsList, its Filter and its Mutation). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
/deploy |
|
Deployed changes to https://app-sswwebsite-9eb3-pr-4984.azurewebsites.net ℹ️ Staging slots are no longer created automatically - comment |
🚀 Lighthouse Report
|
Swap SophieHub/SSW Dory order and refresh several products' tag copy (TinaCMS, YakShaver, SophieHub, CodeAuditor, SSW Rewards) to better describe each product's capability at a glance. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three tags per card pushed most of the grid onto a second chip row at the 4-up tier; the first two already communicate what a product is, so drop MAX_VISIBLE_TAGS to 2 (editors can still store more — extras are just not rendered). Also drops MoreProductsPanel's second line, which read as filler copy once the heading said the whole thing, and updates a stale comment about SSW Rewards' URL now being absolute rather than site-relative. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A stray lockfile in a parent directory (e.g. ~/package-lock.json) made Turbopack infer the workspace root above this project, which broke resolution of instrumentation.ts and crashed `next dev` on startup. Pin turbopack.root explicitly to the project directory. tsconfig.json's accompanying diff is Next.js's own auto-update for Next 16 (moduleResolution: bundler, jsx: react-jsx, .next/dev/types). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
/deploy |
|
Deployed changes to https://app-sswwebsite-9eb3-pr-4984.azurewebsites.net ℹ️ Staging slots are no longer created automatically - comment |
🚀 Lighthouse Report
|
isaaclombardssw
left a comment
isaaclombardssw
left a comment
There was a problem hiding this comment.
🤖 Agent Review
Requesting changes on four things, all commented inline against the diff.
🚨 Must Fix / Address
- 💄 Comment volume - 477 of 888 lines in
components/products/are comment (54%), and two of them are already wrong. - SSW Rewards link - relative to absolute non-www, which costs the soft navigation and adds a 301 hop.
- tsconfig.json - no longer Prettier-clean where
mainwas, plus an undeclared repo-widemoduleResolution/jsxflip. - subTitle - still
required: trueand populated, but never rendered.
🤏 Small Feedback
- Make the brand-card ordering explicit rather than assumed.
BRAND_CARDSand the render branches state the same list twice.- "Fun Quizes" typo in the Rewards tags.
📞 Callouts
- The rest is in good shape - CI passes on this SHA, tests and lint are clean locally, and the
THEMED_ROUTESopt-in, thedata-tina-fieldfix and the falsy-href guard inProductCardShellare all nicely done. This is a request for changes on the list above only.
| @@ -0,0 +1,170 @@ | |||
| // Shared chrome for the three kinds of card on /products (standard, TinaCMS, | |||
There was a problem hiding this comment.
Comment volume. components/products/ is 477 of 888 lines comment (54%), and this file is 127 of 170 (74%). The baseline across components/ is about 4%.
The twMerge traps are worth keeping - I ran twMerge and confirmed outline-2 really does swallow the bare outline and silently kill the focus indicator. That is a genuine trap and the comment earns its place.
What I would cut is the revision narrative ("an earlier revision lifted the card 6px", "two intermediate revisions were still too strong") which git already stores, and the hand-measured contrast ratios, which nothing tests and which quietly become false the moment someone changes --card.
There was a problem hiding this comment.
done with Claude:
Removed revision narrative — sentences describing prior iterations (e.g. "an earlier revision lifted the card 6px," "two intermediate revisions were still too strong," the removed install-command chip / gradient blend-mode history). Git history already covers this.
Removed hardcoded contrast ratios — every manually-measured X.XX:1 value, since nothing tests them and they'd silently go stale if a --card/--brand token changes. Kept the accessibility intent instead (e.g. "clears WCAG 3:1," "would fail AA in dark mode").
Kept as-is: the outline-2/twMerge focus-indicator warning in shared.ts, plus all other why-comments explaining twMerge conflicts, library quirks, and non-obvious trade-offs.
Net diff: -142/+103 lines, comment-only (verified no non-comment lines touched), tests still pass. Also fixed a stale doc-comment in yakShaverProductCard.tsx that claimed the text column was width-capped when the actual code (and the per-element comment) shows it isn't.
| return ( | ||
| // ProductCardShell, not a raw <a>: for a product that has a url it wraps | ||
| // CustomLink, which routes the one on-site product (SSW Rewards, at | ||
| // https://www.ssw.com.au/products/rewards) through next/link and adds |
There was a problem hiding this comment.
This says https://www.ssw.com.au/products/rewards, but the content has no www - it is https://ssw.com.au/products/rewards. The missing www is exactly what causes the redirect hop flagged in index.json.
A second one at line 49: the chrome is not identical to ConsultingCard, which uses rounded-xl where this uses rounded-card. Two comments drifted from the code inside one PR is the argument for fewer of them.
There was a problem hiding this comment.
Addressed the comment and made the requested display change:
Fixed the stale/inaccurate comments they flagged (productCard.tsx):
The doc-comment referenced https://www.ssw.com.au/products/rewards — updated to https://ssw.com.au/products/rewards to match the actual content (no www).
The comment claiming ConsultingCard and this card have "identical card chrome" was wrong — ConsultingCard uses rounded-xl (12px), this uses rounded-card (16px). Reworded to state parity is on colour/motion only, not shape, so the comment can't drift from the code again.
Changed the displayed URL (shared.ts):
destinationLabel used to strip the path and show hostname only. It now keeps the path when the URL points below the domain root, so SSW Rewards' footer shows ssw.com.au/products/rewards instead of the ambiguous ssw.com.au (which read like a link to the homepage).
Side effect, not a regression: SophieBot's URL also has a non-root path (/sophiebot), so its footer label picked up the same fix and now shows sswsophie.com/sophiebot instead of just sswsophie.com. Every other product's URL is root-path, so their labels are unchanged.
| <PageCard page={product} key={index} /> | ||
| ))} | ||
| </div> | ||
| {/* The CMS `subTitle` is deliberately not rendered. It still holds |
There was a problem hiding this comment.
Please remove subTitle properly rather than dropping it at the render.
The reasoning here is that the field is required: true so it cannot be emptied - but this PR already edits tina/collections/products.tsx to add tags and regenerates tina-lock.json, so the collection is in play.
As it stands an editor sees a mandatory field, fills it in, and nothing happens on the page, with no signal anywhere in the CMS. Either drop required, or add a description saying it is not displayed - exactly what the new tags field does.
There was a problem hiding this comment.
Don't understand, but this is what Claude fixed:
Removed subTitle from the schema properly rather than leaving it dropped at render:
tina/collections/products.tsx — deleted the field definition entirely.
content/products/index/index.json — removed the orphaned subTitle key and its content.
app/(about)/products/products-index.tsx — removed the now-obsolete comment explaining the render-time drop.
tina/tina-lock.json — regenerated via tinacms build --content=local. Verified productsIndex now has ['seo', 'title', 'productsList'] (matches main minus subTitle), productsList still carries tags, and the total collection count is unchanged.
|
|
||
| // Each brand card occupies two grid cells at the tiers where it spans, so the | ||
| // trailing gap the panel fills is measured in cells, not products. | ||
| const cellCount = |
There was a problem hiding this comment.
Worth making the ordering explicit. This is exact only because TinaCMS and YakShaver happen to lead the CMS list.
A col-span-2 card that cannot fit the remaining columns is bumped to the next row and leaves the rest of the current row empty (sparse auto-placement, the default). cellCount never sees that hole. I simulated placement across orderings: with the current order the panel span is right at both tiers, but move any standard card in front of a brand card and real cells go 13 to 14, so at 4-up the panel computes span 3 into 2 free slots, cannot fit, and wraps to a row of its own - the ragged trailing row the panel exists to prevent.
Sorting brand cards to the front before the map is one line, makes this count provably exact (2 divides both 2 and 4, so brand-first never leaves a hole), and changes nothing visually today.
There was a problem hiding this comment.
Fixed the correctness bug the reviewer flagged in app/(about)/products/products-index.tsx: cellCount was only accurate because TinaCMS and YakShaver happen to lead the CMS list today. Under CSS Grid's default sparse auto-placement, a col-span-2 brand card that doesn't fit the remaining columns of its row gets bumped to a fresh row, leaving the skipped cell empty — cellCount's arithmetic can't see that gap, so a reorder in the CMS would make the trailing MoreProductsPanel compute the wrong span and wrap into its own ragged row.
Fix: sort brand cards to the front of the render order before computing cellCount and mapping the grid, using a stable sort so CMS order is otherwise preserved on both sides of the split. Verified this makes the count provably correct (2 divides both the 2-col and 4-col breakpoints, so brand-first can never leave a gap) and changes nothing visually today — confirmed the live-rendered grid order is exactly TinaCMS, YakShaver, SugarLearning, EagleEye, TimePro, SophieBot, SophieHub, SSW Dory, CodeAuditor, SSW Rewards, SmashingBarrier, unchanged from the CMS order.
Bonus catch along the way: while verifying this, I hit a page-breaking error (Cannot read properties of null (reading 'data')) that turned out to be unrelated to this edit — the manual tinacms build I ran earlier for the subTitle fix had left the running dev server's Turbopack/Tina cache stale. Traced it by querying the local GraphQL server directly and calling the generated query function standalone (both succeeded, proving the schema/content were fine), then restarted the dev server to clear the stale cache. It's now healthy and serving the page correctly.
| // The two products whose owners' media kits require their own card surface. | ||
| // Matched on name rather than list position so the treatment follows the | ||
| // product if an editor reorders the CMS list. | ||
| const BRAND_CARDS = ["tinacms", "yakshaver"]; |
There was a problem hiding this comment.
Minor: this list drives cellCount, but rendering branches on hardcoded name === "tinacms" / "yakshaver" literals at lines 106 and 114. Hardcoding these two is completely fine - it is just stated in two places, so adding a third name here would inflate the panel span against a card that never actually spans.
One Record<string, FC> that both the count and the render read from would close it.
There was a problem hiding this comment.
Consolidated the brand-card lookup into a single source of truth, per the reviewer's suggestion, since the previous BRAND_CARDS array plus the two hardcoded name === "tinacms" / name === "yakshaver" render branches were three separately-maintained lists that cellCount and the JSX could silently drift apart from.
Change in app/(about)/products/products-index.tsx:
Replaced BRAND_CARDS/isBrandCard with BRAND_CARD_COMPONENTS: Record<string, FC> mapping "tinacms" → TinaProductCard and "yakshaver" → YakShaverProductCard, plus a brandCardFor(name) lookup.
The sort (brand-first ordering), cellCount, and the render's brand-card branch all now read from this one map — the two if (name === ...) branches collapsed into a single if (BrandCard) return <BrandCard .../>.
Kept it as a plain object rather than reaching for anything fancier, since it's still just two entries.
- Trim comment bloat in components/products/*: drop revision-narrative and hardcoded contrast-ratio comments, keep twMerge/why comments. - Fix stale comments referencing the old absolute Rewards URL and the false "identical chrome" claim vs ConsultingCard. - Revert SSW Rewards' url to the relative /products/rewards path used on main; destinationLabel now appends the path for on-site links so the footer still shows the full destination. - Remove the CMS subTitle field properly (schema, content, generated lock) instead of leaving it unrendered. - Fix products-index.tsx's brand-card ordering: pin brand cards first so cellCount's grid-cell arithmetic can't be thrown off by sparse auto-placement if the CMS list is ever reordered, and consolidate the brand-card name checks into one Record<string, FC> both cellCount and the render read from. - Revert tsconfig.json to main: its diff was Next's own dev-server auto-rewrite, unrelated to this PR's turbopack.root fix. - Fix "Quizes" typo in SSW Rewards' tags. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
…-page Co-authored-by: hveraus <4745861+hveraus@users.noreply.github.com>
Resolved and pushed in |
Copilot's conflict resolution when merging main into this branch left tina-lock.json out of sync with the merged collections schema (main added the Pills block among other changes). tinacms build --content=local audits the local schema against this lock, so it exited 1 and took npm run build with it — the same failure mode as the earlier 4837779 incident, just surfaced through Copilot's merge this time. Regenerated via tinacms build rather than hand-edited, since it's a 30MB generated file. Verified structurally: still 46 collections, the new Pills block schema from main is present, and productsIndex still matches this branch's changes (no subTitle, tags on productsList). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
/deploy |
|
Deployed changes to https://app-sswwebsite-9eb3-pr-4984.azurewebsites.net ℹ️ Staging slots are no longer created automatically - comment |
🚀 Lighthouse Report
|
isaaclombardssw
left a comment
isaaclombardssw
left a comment
There was a problem hiding this comment.
Please cut the comments right down - this is the one item from last round that measurably didn't move. components/products/ went from 40.1% comment lines to 39.6%, and shared.ts is still 119 comment lines to 34 of code. The contrast ratios moved to styles.css rather than going away. components/consulting/ next door is 7 comment lines across 117 lines of code.
About 10 lines across the whole folder is what I'd expect. Worth keeping: the border-0.75 note (shared.ts:9), the twMerge outline-2 trap (shared.ts:26), why the spans are looked up in a map instead of interpolated, and the contrast values in styles.css. The rest is restating the code, aesthetic rationale that belongs in the PR description, or revision narrative git already stores.
The risk isn't just noise. Nothing compiles or tests a comment, so volume is exposure - and two are wrong right now. shared.ts:144 says every product is authored with an absolute URL, which your own Rewards fix falsified this round, and it sits directly above the new URL(url, base) fallback telling the next person the base isn't needed. shared.ts:136 lists duplicates as a case visibleTags handles, but it doesn't, and key={tag} throws on a repeated tag.
Drops the trailing dashed panel and the grid-cell math that only existed to size it.
Bring /products in line with the /consulting index redesigned in #4939: same page background, container geometry, title scale and card interaction model.
Affected routes:
Fixed #{{ ISSUE NUMBER }}