Skip to content

Commit b831543

Browse files
authored
Fix winter olympics nav styles (#15353)
There are two visual bugs in the new Winter Olympics header: 1. The mobile height of `116px` is being applied at all breakpoints. 2. The `phablet` image is being used at the `tablet` breakpoint and above, rather than the separate images designed for those breakpoints. The styles to apply the correct heights and images exist in the source code, but they're not being built into the output CSS correctly. This is happening due to fields being overridden when composing styles with object spread syntax. The solution is to use Emotion's `css` function to apply composition instead. More details ------------ The issue is with styles being applied to the `nav` element specifically, as that's where both the height and background image are configured. The main nav styles have two fields used to apply different heights using media queries: `from.tablet` and `from.desktop`. In isolation this works. However, the styles for the background images are merged into the nav styles using object spread syntax. These also contain `from.tablet` and `from.desktop` fields, and because they are spread after the earlier fields, they override them. This deletes the earlier height styles and they do not appear in the output CSS, leaving only the mobile height. The resulting object does now contain entries for all five background images. However, the order is important, as later media queries will override earlier ones. In the original background images object the fields are defined in increasing size order, `mobileLandscape` up to `desktop`, but merging the objects *changes* this order. When two JS objects are merged, any keys that appear only in the second object will appear after any keys from the first object in the result. However, if any fields are shared between the two objects, the **values** from the second object override those in the first, but the **keys** retain their original position in the result. In the nav styles this means that when the following merge occurs: ```ts const navStyles = { [from.tablet]: "tablet height", [from.desktop]: "tablet height", }; const backgroundImageStyles = { [from.mobileLandscape]: "background image", [from.phablet]: "background image", [from.tablet]: "background image", [from.desktop]: "background image", }; const result = { ...navStyles, ...backgroundImageStyles }; ``` the resulting object looks like this: ```ts const result = { [from.tablet]: "background image", [from.desktop]: "background image", [from.mobileLandscape]: "background image", [from.phablet]: "background image", }; ``` The values from the second object are all there, but their order has been rearranged due to the presence of some of the same keys in the first object. This means that the phablet media query will appear _last_ in the output CSS. As these are all "min-width" queries, they all apply at the wider breakpoints, so the one that appears last takes precedence. When Emotion's `css` function is used instead it applies a different algorithm for composing styles, and ensures that they all appear in the output in the expected order.
1 parent f912e23 commit b831543

2 files changed

Lines changed: 20 additions & 12 deletions

File tree

dotcom-rendering/src/components/DirectoryPageNav.stories.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,9 @@ export const OtherCompetition = {
3131
pageId: 'football/premierleague/table',
3232
},
3333
} satisfies Story;
34+
35+
export const WinterOlympics = {
36+
args: {
37+
pageId: 'sport/winter-olympics-2026',
38+
},
39+
} satisfies Story;

dotcom-rendering/src/components/DirectoryPageNav.tsx

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,19 +119,21 @@ export const DirectoryPageNav = ({ pageId, pageTags }: Props) => {
119119

120120
const { textColor, backgroundColor } = config;
121121

122-
const nav = css({
123-
backgroundColor,
124-
'&': css(grid.paddedContainer),
125-
alignContent: 'space-between',
126-
height: 116,
127-
[from.tablet]: {
128-
height: 140,
129-
},
130-
[from.desktop]: {
131-
height: 150,
122+
const nav = css(
123+
{
124+
backgroundColor,
125+
'&': css(grid.paddedContainer),
126+
alignContent: 'space-between',
127+
height: 116,
128+
[from.tablet]: {
129+
height: 140,
130+
},
131+
[from.desktop]: {
132+
height: 150,
133+
},
132134
},
133-
...backgroundImageStyles(config.backgroundImages),
134-
});
135+
backgroundImageStyles(config.backgroundImages),
136+
);
135137

136138
const largeLinkStyles = css({
137139
...headlineBold24Object,

0 commit comments

Comments
 (0)