Skip to content

Fix labs header details alignment #14539

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

Merged
cemms1 merged 5 commits into from
Sep 23, 2025
Merged

Fix labs header details alignment #14539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62544da
Indroduce LabsSectionHeader component for use in FrontSection for lab…
cemms1 Sep 19, 2025
66076e7
move LabsLogo to root level of component dir, remove hardcoded switch…
cemms1 Sep 19, 2025
7496862
replace accidental removal of styles, extract labs section styles int…
cemms1 Sep 19, 2025
1cf4165
update details alignment for `LabsSectionHeader` and suppress the for…
cemms1 Sep 22, 2025
e9ea4c8
use the default theme in LabsSectionHeader story for the split theme …
cemms1 Sep 22, 2025
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
32 changes: 29 additions & 3 deletions dotcom-rendering/.storybook/decorators/splitThemeDecorator.tsx
View file
Open in desktop

@cemms1 cemms1 Sep 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adds hideFormatHeading arg to the Split Theme decorator to allow omission of the format heading for non article stories

Optional arg and defaults to false so will not have an impact on any existing stories

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ interface Orientation {
orientation?: 'horizontal' | 'vertical';
}

type SplitThemeOptions = Orientation & {
/** Allows the format heading to be omitted on the split theme decorator */
hideFormatHeading?: boolean;
};

/**
* The `splitTheme` decorator displays a story simultaneously in both light and
* dark mode.
Expand Down Expand Up @@ -226,6 +231,11 @@ type ThemeProps = {
Story: Parameters<Decorator>[0];
context: Context;
colourScheme: ColourScheme;
/**
* For stories without article theming where we fallback to the default theme,
* we can choose to omit the format heading as it's not relevant
*/
hideFormatHeading?: boolean;
};

/**
Expand All @@ -242,7 +252,13 @@ type ThemeProps = {
* `colourSchemeBackground` and `colourSchemeTextColour` parameters from the
* story, or provides defaults when these are not supplied.
*/
const Theme = ({ formats, Story, context, colourScheme }: ThemeProps) => (
const Theme = ({
formats,
Story,
context,
colourScheme,
hideFormatHeading = false,
}: ThemeProps) => (
<div
css={{
color:
Expand All @@ -258,7 +274,12 @@ const Theme = ({ formats, Story, context, colourScheme }: ThemeProps) => (
<ThemeHeading colourScheme={colourScheme} />
{formats.map((format) => (
<>
<FormatHeading format={format} colourScheme={colourScheme} />
{!hideFormatHeading && (
<FormatHeading
format={format}
colourScheme={colourScheme}
/>
)}
<Palette
colourScheme={colourScheme}
context={context}
Expand Down Expand Up @@ -302,7 +323,10 @@ const Theme = ({ formats, Story, context, colourScheme }: ThemeProps) => (
export const splitTheme =
(
formats: ArticleFormat[] = [...defaultFormats],
{ orientation = 'horizontal' }: Orientation = {},
{
orientation = 'horizontal',
hideFormatHeading = false,
}: SplitThemeOptions = {},
): Decorator =>
(Story, context) => (
<div
Expand All @@ -318,12 +342,14 @@ export const splitTheme =
formats={formats}
Story={Story}
context={context}
hideFormatHeading={hideFormatHeading}
/>
<Theme
colourScheme="dark"
formats={formats}
Story={Story}
context={context}
hideFormatHeading={hideFormatHeading}
/>
</div>
);
Expand Down
42 changes: 26 additions & 16 deletions dotcom-rendering/src/components/LabsSectionHeader.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { css } from '@emotion/react';
import { from } from '@guardian/source/foundations';
import type { Meta, StoryObj } from '@storybook/react';
import { userEvent, within } from '@storybook/test';
import { splitTheme } from '../../.storybook/decorators/splitThemeDecorator';
import {
ArticleDesign,
ArticleDisplay,
ArticleSpecial,
} from '../lib/articleFormat';
defaultFormats,
splitTheme,
} from '../../.storybook/decorators/splitThemeDecorator';
import { LabsSectionHeader } from './LabsSectionHeader';

const meta = {
Expand All @@ -15,24 +15,34 @@ const meta = {
title: 'Container Title',
url: '/',
},
render: (args) => <LabsSectionHeader {...args} />,
render: (args) => (
<div
css={css`
margin: 20px auto 100px;
min-width: 320px;
${from.leftCol} {
min-width: auto;
max-width: 260px;
height: 400px;
}
Comment on lines +21 to +27

@cemms1 cemms1 Sep 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Centres the story and restricts the space available to bring it closer to how it looks when used inside the FrontSection container

`}
>
<LabsSectionHeader {...args} />
</div>
),
decorators: [
splitTheme(
[
{
theme: ArticleSpecial.Labs,
design: ArticleDesign.Feature,
display: ArticleDisplay.Standard,
},
],
{ orientation: 'vertical' },
),
splitTheme([defaultFormats[0]], {
orientation: 'vertical',
hideFormatHeading: true,
}),
],
} satisfies Meta<typeof LabsSectionHeader>;
export default meta;

type Story = StoryObj<typeof meta>;

export const Default: Story = {};

export const WithDetailsOpen: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
Expand Down
12 changes: 9 additions & 3 deletions dotcom-rendering/src/components/LabsSectionHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ const aboutStyles = css`
${textSans14}
`;

const positionStyles = css`
right: 0;
${from.leftCol} {
left: 0;
right: auto;
}
`;
Comment on lines +85 to +91

@cemms1 cemms1 Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main fix in this PR is to adjust the positioning of the opened details element


const detailsStyles = css`
background-color: ${schemePalette('--labs-about-dropdown-background')};
color: ${schemePalette('--labs-about-dropdown-text')};
Expand All @@ -101,9 +109,7 @@ export const LabsSectionHeader = ({ title, url }: Props) => (
<Details
label="About"
labelSize="xsmall"
positionStyles={css`
right: 0;
`}
positionStyles={positionStyles}
>
<div css={detailsStyles}>
<p>
Expand Down
Loading