💄 Redesign /consulting index + shared card color tokens - #4939
Conversation
Dark-theme surface tokens for card backgrounds (#101010 default, #151515 hover), exposed to Tailwind as bg-card / bg-card-hover so cards across the site can share one definition.
Replace the tag-filter grid with a sticky sidebar + sectioned card layout driven by the home theme (light/dark). Extend the HomeTheme boundary, pre-paint script and mega menu dark scope to /consulting, and give the shell the .dark class so design tokens (incl. the new --card tokens and --hairline border) resolve on this route.
Swap the dark-mode 60%-opacity background trick for the explicit --card / --card-hover tokens, matching the /consulting cards. Border already uses --hairline.
|
No linked issues found. Please add the corresponding issues in the pull request description. |
Coverage report
Test suite run success19 tests passing in 2 suites. Report generated by 🧪jest coverage report action from 6d4ed94 |
category/pageCard/tag were only used by the previous tag-filter layout; pageCard still imported the removed useFilterContext export, breaking the next build type check.
Also express the conditional dark class via cn() — prettier-plugin-tailwindcss strips the leading space inside a template-literal conditional, which mangled the class into 'shelldark'.
|
/deploy |
|
Deployed changes to https://app-sswwebsite-9eb3-pr-4939.azurewebsites.net ℹ️ Staging slots are no longer created automatically - comment |
New 'Popular' boolean on the consulting index page entries (Tina toggle). When on, the card shows a small uppercase pill next to the title, styled with the existing --consulting-accent token so it follows the light/dark theme.
|
/deploy |
|
Deployed changes to https://app-sswwebsite-9eb3-pr-4939.azurewebsites.net ℹ️ Staging slots are no longer created automatically - comment |
🚀 Lighthouse Report
|
There was a problem hiding this comment.
We stick away form using these .index.module.css files
Also creates a BUNCH of new vars that may just be used in this directory.
If we create css variables they should be global
| const gridRef = useRef(null); | ||
| const router = useRouter(); | ||
| const [selectedTag, setSelectedTag] = useState(allServices); | ||
| const slugify = (value: string) => |
There was a problem hiding this comment.
do we have a utility function for this all ready?
| .replace(/\s+/g, "-"); | ||
|
|
||
| const filterDidChange = useRef<boolean>(false); | ||
| const mapPageUrl = (page) => |
There was a problem hiding this comment.
do we have a utility function for this all ready?
| return pageAcc; | ||
| const [activeSectionId, setActiveSectionId] = useState<string>(""); | ||
|
|
||
| const tags = useMemo(() => { |
There was a problem hiding this comment.
(this wasn't you but https://www.ssw.com.au/rules/follow-boy-scout-rule)
all 4 memos can be combined into one useMemo hook instead of a chain of 4 of them
|
|
||
| <div className={styles.cardGrid}> | ||
| {section.pages.map((page) => ( | ||
| <a |
There was a problem hiding this comment.
I would move this and all of it's descendents into their own component that can be re-used... i.e. consulting card or something along those lines
joshbermanssw
added
the
no-issue
joshbermanssw
left a comment
joshbermanssw
left a comment
There was a problem hiding this comment.
Nice direction on the redesign, and the imageCards change is a clean simplification (opacity fade to a real colour transition on shared tokens).
Two things I'd want fixed before this merges, both content losses rather than styling:
- 8 cards stop rendering. 6 pages carry no
tagsat all, so they fall to the[allServices]default and now match no section. On top of that, the new URL dedup drops two distinct cards (Microsoft Azure, Document Management) because they share a page reference with a differently-titled card. Details inline. - The mega menu's two
/consulting?tag=...deep links are dead, since the query-param reader is gone.content/megamenu/menu.jsonlines 38 and 65 need rewriting to the new hashes.
Rest of the comments are smaller: the themed-route list is now duplicated in three places, /consulting has no pre-paint script so it will flash dark to light for light-mode users, and there's a nested <main>.
I've deliberately left the index.module.css file alone here given my earlier comment on it. Worth knowing for whatever replaces it: the inactive nav colour (#aaaaaa on #fafafa) is 2.2:1 in light mode against a 4.5:1 requirement, and the POPULAR badge lands around 3.2:1 in dark, so both need new values wherever those styles end up.
| }, [allMappedPages, tags]); | ||
|
|
||
| const contentSections = useMemo( | ||
| () => sections.filter((section) => section.name !== allServices), |
There was a problem hiding this comment.
6 services disappear from the page here.
Sections come from node.sidebar and this drops the All SSW Services entry, so a card only renders if it carries a real tag. But 6 pages in content/consulting/index/index.json have no tags key at all, so they hit the : [allServices] fallback on line 64 and now match no section:
- Azure DevOps to GitHub Migration
- Odoo
- Clean Architecture
- Vertical Slice Architecture
- ServiceNow
- Chinafy - Mini Apps
They render today because selectedTag defaulted to allServices, so the old page showed them under their category heading.
Either tag those 6 in the content, or keep a fallback section for anything that matched nothing. Making tags required in the Tina schema would stop this recurring silently the next time someone adds a card.
| const unique = new Map(); | ||
| pages.forEach((page) => { | ||
| if (!unique.has(page.url)) { | ||
| unique.set(page.url, page); | ||
| } | ||
| return acc; | ||
| }, []); | ||
| }, [node, selectedTag]); | ||
|
|
||
| const tags = useMemo( | ||
| () => | ||
| node.sidebar?.map((item) => { | ||
| return { | ||
| label: item.label, | ||
| name: item.tag?.name, | ||
| }; | ||
| }), | ||
| [node] | ||
| }); | ||
| return Array.from(unique.values()); |
There was a problem hiding this comment.
This dedup drops distinct cards, not duplicates.
Keying on url alone assumes one card per page reference, but two entries in the content share content/consulting/azure.mdx with different titles, descriptions, logos and tags:
- "Azure AI" (tags:
ai-development) - "Microsoft Azure" (tags:
cloud-and-infrastructure,platform-development)
First-wins keeps Azure AI, so Microsoft Azure vanishes from both the Cloud and Infrastructure and Platform Development sections. Same shape for content/consulting/sharepoint.mdx: "SharePoint" wins and "Document Management" disappears from Other SSW Services.
Keying on the rendered card instead of the page (${page.url}|${page.title}) fixes it. Worth checking whether cross-category dedup is needed at all, since sections are tag-driven now rather than category-driven.
| setActiveSectionId(contentSections[0].sectionId); | ||
| } | ||
|
|
||
| const tagParam = searchParams.get("tag"); |
There was a problem hiding this comment.
Removing the ?tag= reader breaks two live links in the primary nav. content/megamenu/menu.json still points at:
- line 38:
/consulting?tag=Platform-Development - line 65:
/consulting?tag=Cloud-and+Infrastructure
Both now land at the top of the page with no filter and no scroll. They want updating to the new anchors (/consulting#consulting-platform-development and /consulting#consulting-cloud-and-infrastructure).
These URLs are public and indexed, so a small ?tag= to hash shim on mount would also be worth it for anyone arriving from an external link or a bookmark.
| <Container className="flex-1 pt-2"> | ||
| <div className="min-h-8 w-full max-w-9xl md:min-h-12"> | ||
| <div | ||
| className={cn(styles.shell, isDark && "dark")} |
There was a problem hiding this comment.
This shell gets .dark from React state only, and nothing renders HomeThemePrePaint inside it. HomeThemePrePaint only appears in HomeThemeShell (homepage) and in MegaMenuWrapper, so the menu is protected but this page is not: the SSR'd HTML ships with DEFAULT_THEME = "dark" and light-mode users will see the page body flip on hydration.
Wrapping the content in HomeThemeShell instead of hand-rolling the shell fixes the flash, and gets you bg-background text-foreground plus one theming mechanism rather than two (.dark and data-theme are both being set here).
| className={cn(styles.shell, isDark && "dark")} | ||
| data-theme={isDark ? "dark" : "light"} | ||
| > | ||
| <main className={styles.page}> |
There was a problem hiding this comment.
Nested <main>. components/layout/layout.tsx:130 already wraps every page in <main className="grow bg-white">, so this is a second main landmark inside the first, which isn't valid HTML. A div does the job here.
While you're in there: that outer wrapper's bg-white is unconditional, and .shell's min-height: 100% resolves against a flex grow parent, which is fragile. Worth a look in dark mode on a tall screen to confirm no white band shows below the last section.
| <aside className={styles.sidebar}> | ||
| <h1 className={styles.heading}>Consulting Services</h1> |
There was a problem hiding this comment.
The page's <h1> is inside <aside>, so the document title sits in the complementary landmark and <main> has no heading at all. Screen reader users navigating by landmark will find "Consulting Services" under "complementary".
Moving the <h1> out to sit above the layout grid (or making this wrapper a plain div) sorts it.
| onClick={(event) => | ||
| onNavClick(event, section.sectionId) | ||
| } | ||
| aria-current={isActive ? "true" : undefined} |
There was a problem hiding this comment.
For in-page anchor nav this should be aria-current="location" rather than "true". "true" is valid ARIA but it reads as "current item" generically; "location" is the value specified for "the current position within a flow", which is what the scroll-spy is expressing.
| // choice or `prefers-color-scheme`. | ||
| const DEFAULT_THEME: HomeThemeMode = "dark"; | ||
|
|
||
| const isThemedRoute = (pathname: string) => |
There was a problem hiding this comment.
The themed-route list now lives in three places that have to stay in sync: this helper, the PRE_PAINT_SCRIPT string on line 71, and a hand-rolled copy of the same OR in MegaMenuWrapper.tsx:21.
Exporting a single THEMED_ROUTES array, deriving both this helper and the script's inlined check from it, and importing isThemedRoute in MegaMenuWrapper would leave one place to edit when the next route gets themed.
| // utilities (bg-background/text-foreground/border-hairline) flip via the CSS | ||
| // variables under the `dark` class on this element. | ||
| const isHome = pathName === "/"; | ||
| const isHome = pathName === "/" || pathName === "/consulting"; |
There was a problem hiding this comment.
isHome is no longer accurate now that it covers /consulting, and it's a duplicate of isThemedRoute in homeTheme.tsx:22 (not currently exported). Importing that helper and renaming this to isThemedRoute would keep the two lists from drifting apart, since this one also gates HomeThemePrePaint on line 38.
| --card: #101010; | ||
| --card-hover: #151515; |
There was a problem hiding this comment.
These two are defined only in the dark block, with no light counterpart, unlike every token around them (--hairline is set in both, at line 108 and line 177). So bg-card resolves to nothing in light mode.
The only consumer today guards it (bg-white dark:bg-card), so nothing is visibly broken, but for a PR whose headline is shared card tokens it's worth adding the light values now. Otherwise the next person to reach for bg-card gets a transparent card and no clue why.
2 participants
Unifies card styling between the /consulting index and the homepage service cards behind shared design tokens.
Changes:
--card: #101010,--card-hover: #151515(styles.css), exposed to Tailwind asbg-card/bg-card-hover. Card borders use the existing--hairline: #212121.