Skip to content

Directory subnav polishes and wider rollout #15310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
frederickobrien wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Directory subnav polishes and wider rollout #15310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions dotcom-rendering/src/components/DirectoryPageNav.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ import { DirectoryPageNav } from './DirectoryPageNav';
const meta = {
component: DirectoryPageNav,
title: 'Components/Directory Page Nav',
argTypes: {
selected: {
options: ['fixtures', 'tables', 'none'],
control: { type: 'select' },
},
},
parameters: {
chromatic: {
modes: {
Expand All @@ -28,14 +22,12 @@ type Story = StoryObj<typeof meta>;

export const WomensEuro2025 = {
args: {
selected: 'fixtures',
pageId: 'football/women-s-euro-2025/table',
},
} satisfies Story;

export const OtherCompetition = {
args: {
selected: 'none',
pageId: 'football/premierleague/table',
},
} satisfies Story;
52 changes: 26 additions & 26 deletions dotcom-rendering/src/components/DirectoryPageNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,20 @@ import {
palette,
} from '@guardian/source/foundations';
import { grid } from '../grid';
import type { TagType } from '../types/tag';

type Props = {
selected: Selected;
pageId: string;
pageTags?: TagType[];
};

interface DirectoryPageNavConfig {
pageIds: string[];
tagIds: string[];
textColor: string;
backgroundColor: string;
title: { label: string; link: string };
links: { label: string; href: string; selectedSlug: string | undefined }[];
title: { label: string; id: string };
links: { label: string; id: string }[];
backgroundImages?: {
mobile: string;
mobileLandscape: string;
Expand All @@ -39,32 +41,29 @@ const configs = [
'sport/ng-interactive/2026/feb/04/winter-olympics-results-milano-cortina-2026',
'sport/ng-interactive/2026/feb/04/winter-olympics-2026-latest-medal-table-milano-cortina',
],
tagIds: ['sport/winter-olympics-2026'],
textColor: palette.neutral[7],
backgroundColor: '#CCCCCC',
title: {
label: 'Winter Olympics 2026',
link: 'https://www.theguardian.com/sport/winter-olympics-2026',
id: 'sport/winter-olympics-2026',
},
links: [
{
label: 'Schedule',
href: 'https://www.theguardian.com/sport/ng-interactive/2026/feb/04/winter-olympics-full-schedule-milano-cortina-2026',
selectedSlug: 'schedule',
id: 'sport/ng-interactive/2026/feb/04/winter-olympics-full-schedule-milano-cortina-2026',
},
{
label: 'Results',
href: 'https://www.theguardian.com/sport/ng-interactive/2026/feb/04/winter-olympics-results-milano-cortina-2026',
selectedSlug: 'results',
id: 'sport/ng-interactive/2026/feb/04/winter-olympics-results-milano-cortina-2026',
},
{
label: 'Medal table',
href: 'https://www.theguardian.com/sport/ng-interactive/2026/feb/04/winter-olympics-2026-latest-medal-table-milano-cortina',
selectedSlug: undefined,
id: 'sport/ng-interactive/2026/feb/04/winter-olympics-2026-latest-medal-table-milano-cortina',
},
{
label: 'Full coverage',
href: 'https://www.theguardian.com/sport/winter-olympics-2026',
selectedSlug: undefined,
id: 'sport/winter-olympics-2026',
},
],
backgroundImages: {
Expand All @@ -80,10 +79,6 @@ const configs = [
},
] satisfies DirectoryPageNavConfig[];

type Configs = typeof configs;
type SelectedSlug = Configs[number]['links'][number]['selectedSlug'];
type Selected = Exclude<SelectedSlug, undefined>;

const backgroundImageStyles = (
images?: DirectoryPageNavConfig['backgroundImages'],
) => {
Expand All @@ -109,8 +104,14 @@ const backgroundImageStyles = (
};
};

export const DirectoryPageNav = ({ selected, pageId }: Props) => {
const config = configs.find((cfg) => cfg.pageIds.includes(pageId));
export const DirectoryPageNav = ({ pageId, pageTags }: Props) => {
const config = configs.find(
(cfg) =>
cfg.pageIds.includes(pageId) ||
cfg.tagIds.some(
(tagId) => pageTags?.some((tag) => tag.id === tagId),
),
);

if (!config) {
return null;
Expand Down Expand Up @@ -225,24 +226,23 @@ export const DirectoryPageNav = ({ selected, pageId }: Props) => {

return (
<nav css={nav}>
<a href={config.title.link} css={largeLinkStyles}>
<a href={config.title.id} css={largeLinkStyles}>
{config.title.label}
</a>
<ul css={list}>
{config.links.map((link, i) => (
<li key={link.label} css={listItem}>
<li
key={link.label}
css={listItem}
style={pageId === link.id ? selectedStyles : {}}
>
<a
href={link.href}
href={`/${link.id}`}
Copy link
Contributor Author

@frederickobrien frederickobrien Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is pageId a sound way of linking internally? It seemed to work locally but wanted to double check.

Copy link
Contributor

@JamieB-gu JamieB-gu Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth discussing with @JustinPinner the differences between this vs webURL vs canonicalUrl?

This id is being set by you here in this file, in the config object? So it's a predefined list of paths that you've tested, as opposed to something coming in dynamically from upstream data?

Copy link
Contributor Author

@frederickobrien frederickobrien Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly yeah, so we can easily change it to something else if it's more appropriate. I'm currently using the pageId value of each page.

Copy link
Contributor Author

@frederickobrien frederickobrien Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have spoken with @JustinPinner about this @JamieB-gu and it seems pageId is the way to go: "the content's ID is the most stable CAPI reference you could use"

css={
i === config.links.length - 1
? lastSmallLink
: smallLink
}
style={
link.selectedSlug === selected
? selectedStyles
: {}
}
>
{link.label}
</a>
Expand Down
5 changes: 1 addition & 4 deletions dotcom-rendering/src/components/FootballMatchesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ export const FootballMatchesPage = ({
pageId,
}: Props) => (
<>
<DirectoryPageNav
selected={kind === 'FootballFixtures' ? 'fixtures' : 'none'}
pageId={pageId}
/>
<DirectoryPageNav pageId={pageId} />
<main
id="maincontent"
data-layout="FootballDataPageLayout"
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/FootballTablesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const FootballTablesPage = ({
guardianBaseUrl,
}: Props) => (
<>
<DirectoryPageNav selected="none" pageId={pageId} />
<DirectoryPageNav pageId={pageId} />
<main
id="maincontent"
data-layout="FootballDataPageLayout"
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/layouts/FrontLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export const FrontLayout = ({ front, NAV }: Props) => {
/>
</Island>
)}
<DirectoryPageNav selected="none" pageId={pageId} />
<DirectoryPageNav pageId={pageId} />

{filteredCollections.map((collection, index) => {
// Backfills should be added to the end of any curated content
Expand Down
5 changes: 4 additions & 1 deletion dotcom-rendering/src/layouts/InteractiveLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,10 @@ export const InteractiveLayout = (props: WebProps | AppsProps) => {
</>
)}
<main data-layout="InteractiveLayout">
<DirectoryPageNav selected="none" pageId={article.pageId} />
<DirectoryPageNav
pageId={article.pageId}
pageTags={article.tags}
/>
<Section
fullWidth={true}
showTopBorder={false}
Expand Down
5 changes: 5 additions & 0 deletions dotcom-rendering/src/layouts/ShowcaseLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ArticleTitle } from '../components/ArticleTitle';
import { Border } from '../components/Border';
import { Carousel } from '../components/Carousel.importable';
import { DecideLines } from '../components/DecideLines';
import { DirectoryPageNav } from '../components/DirectoryPageNav';
import { DiscussionLayout } from '../components/DiscussionLayout';
import { Footer } from '../components/Footer';
import { GridItem } from '../components/GridItem';
Expand Down Expand Up @@ -356,6 +357,10 @@ export const ShowcaseLayout = (props: WebProps | AppsProps) => {
<AdPortals />
</Island>
)}
<DirectoryPageNav
pageId={article.pageId}
pageTags={article.tags}
/>
<Section
fullWidth={true}
showTopBorder={false}
Expand Down
5 changes: 5 additions & 0 deletions dotcom-rendering/src/layouts/StandardLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { ArticleTitle } from '../components/ArticleTitle';
import { Border } from '../components/Border';
import { Carousel } from '../components/Carousel.importable';
import { DecideLines } from '../components/DecideLines';
import { DirectoryPageNav } from '../components/DirectoryPageNav';
import { DiscussionLayout } from '../components/DiscussionLayout';
import { Footer } from '../components/Footer';
import { GetMatchNav } from '../components/GetMatchNav.importable';
Expand Down Expand Up @@ -437,6 +438,10 @@ export const StandardLayout = (props: WebProps | AppProps) => {
<AdPortals />
</Island>
)}
<DirectoryPageNav
pageId={article.pageId}
pageTags={article.tags}
/>
<Section
fullWidth={true}
showTopBorder={false}
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/layouts/TagPageLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const TagPageLayout = ({ tagPage, NAV }: Props) => {
</div>

<main data-layout="TagPageLayout" id="maincontent">
<DirectoryPageNav selected="none" pageId={tagPage.pageId} />
<DirectoryPageNav pageId={tagPage.pageId} />
{isAccessibilityPage && (
<Island priority="critical" defer={{ until: 'visible' }}>
<Accessibility />
Expand Down
Loading