Skip to content

Add MoreGalleries component with a story - #14538

Merged
marjisound merged 12 commits into
mainfrom
add-more-galleries-component
Oct 6, 2025
Merged

marjisound merged 12 commits into
mainfrom
add-more-galleries-component

Conversation

@marjisound

@marjisound marjisound commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

Paired with @JamieB-gu

What does this change?

This PR creates a new MoreGalleries component which will be used for the more galleries onward content section on gallery articles. For this component we were given new design that is different from how this section looks currently in production.

This new component is not currently used. The usage will be implemented in an upcoming PR.

Screenshots

Desktop light Desktop dark
desktop-light desktop-dark
Mobile light mobile dark
mobile-light mobile-dark

This PR fixes #14310

@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Sep 22, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 22, 2025
@marjisound
marjisound force-pushed the add-more-galleries-component branch from 85206ec to 3755b1f Compare September 22, 2025 10:37
@github-actions

github-actions Bot commented Sep 22, 2025

Copy link
Copy Markdown

@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Sep 22, 2025
@github-actions

github-actions Bot commented Sep 22, 2025

Copy link
Copy Markdown

@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 22, 2025
trailTextColour?: string;
/** Controls visibility of trail text on various breakpoints */
hideUntil?: 'tablet' | 'desktop';
hideUntil?: 'tablet' | 'desktop' | 'never';

@marjisound marjisound 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.

Why did I need a never value?
The hideUntil can be undefined. When it's undefined, the component will always add the trail text, but in css display is set to none for viewport below tablet.

When hideUntil is not undefined, the trail text will be wrapped within a Hide element and become hidden until the viewport that's given by the hideUntil. However, the css display: none; is also set for until.tablet in this case.

So no matter what, we always have display: none; on trail text for viewport below tablet. But in our case for more-galleries onward container, we want to always display trail text within the flexSplash card for all viewports.

That's why I added the never value in order to always display the trail for all viewports.

@arelra and @abeddow91 what do you think about this change?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if it might be simpler to introduce an optional prop such as:
hide: boolean which defaults to true

This then controls whether the trail text applies the <Hide> component or not.

In existing usages of TrailText this will default to true and apply <Hide> but for the galleries onwards container we could pass hide=false which would mean the hiding behaviour would not apply ?

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.

Great idea, I instead called the new param as hideOnSmallBreakpoints to make it clearer what it's doing, is that OK?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Having thought about this a little more I'm not sure I understand why we don't use the absence or presence of hideUntil to control whether hiding is activated?

@marjisound marjisound Oct 1, 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.

Thanks @arelra for your ideas and for pairing with me to do a proper refactoring on TrailText component.
The changes are in this commit ac6dc74 and there's also this PR that we made in order to cover the cases for hideUntil #14586

@marjisound
marjisound marked this pull request as ready for review September 22, 2025 10:52
@github-actions

Copy link
Copy Markdown

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

@marjisound marjisound added this to the Decommission AR milestone Sep 22, 2025
@marjisound
marjisound requested review from a team and abeddow91 September 22, 2025 10:58
Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com>
@marjisound
marjisound force-pushed the add-more-galleries-component branch from 3755b1f to 367a603 Compare September 22, 2025 11:01

@DanielCliftonGuardian DanielCliftonGuardian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good. Just one suggestion.

Comment thread dotcom-rendering/src/components/MoreGalleries.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.tsx
Comment thread dotcom-rendering/src/components/Card/Card.tsx
@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Sep 26, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 26, 2025
@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Sep 29, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 29, 2025
@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Sep 30, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 30, 2025
@JamieB-gu JamieB-gu added the run_chromatic Runs chromatic when label is applied label Sep 30, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 30, 2025
marjisound and others added 2 commits September 30, 2025 16:43
Instead always pass hideUntil when hiding is required and pass undefined when hiding is not required (trail text needs to be visible for all breakpoints)

Co-authored-by: Ravi <7014230+arelra@users.noreply.github.com>
@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Sep 30, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Sep 30, 2025

@arelra arelra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work! ✨

@arelra arelra added the run_chromatic Runs chromatic when label is applied label Oct 1, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Oct 1, 2025

@JamieB-gu JamieB-gu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Some questions and suggestions.

Comment thread dotcom-rendering/src/paletteDeclarations.ts Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.stories.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.stories.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.stories.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.tsx Outdated
Comment thread dotcom-rendering/src/components/MoreGalleries.tsx Outdated
uniqueId?: string;
/** The Splash card in a flexible container gets a different visual treatment to other cards */
/** The Splash card in a flexible or onward container gets a different visual treatment to other cards */
isFlexSplash?: boolean;

@abeddow91 abeddow91 Oct 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should rename this field or use a seperate one here. I'm not sure we should be coupling a flexible splash card with an onwards container splash, unless they are truely the same card. If they are then perhaps renaming to something more generic (ie removing the flexible part would be preferable).

Does the onwards "splash" card share the same characteristics as a flexible splash card?

@marjisound marjisound Oct 3, 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.

Thanks for pointing this out @abeddow91
You are right, it seems the characteristics of a flexible splash card is different from an onward splash card.
The isFlexSplash is primarily used to deal with positioning the sub links. But what's important in an onward splash card is that it needs to have the trail text while the non-splash cards do not have trail text.

I created a new prop isOnwardSplash for this in here 73cd910

@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Oct 3, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Oct 3, 2025
@marjisound marjisound added the run_chromatic Runs chromatic when label is applied label Oct 6, 2025
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Oct 6, 2025
@marjisound
marjisound merged commit a9a8403 into main Oct 6, 2025
31 checks passed
@marjisound
marjisound deleted the add-more-galleries-component branch October 6, 2025 12:59
@gu-prout

gu-prout Bot commented Oct 6, 2025

Copy link
Copy Markdown

Overdue on PROD (merged by @marjisound 47 minutes and 22 seconds ago) What's gone wrong?

@gu-prout

gu-prout Bot commented Oct 6, 2025

Copy link
Copy Markdown

Seen on PROD (merged by @marjisound 1 hour, 19 minutes and 8 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adapt the layout and cards from fronts for use in the article template

5 participants