Skip to content
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.

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

Add new "highlights" container layout #1582

Merged
merged 4 commits into from
May 30, 2024
Merged

Conversation

cemms1
Copy link
Contributor

@cemms1 cemms1 commented May 15, 2024

Co-authored-by: Wai Sing Yiu wai.sing.yiu@guardian.co.uk
Co-authored-by: Frederick O'Brien frederick.obrien@guardian.co.uk

What's changed?

Adds new container layout for use as the "highlights" container.

This is a new container layout intended to only be used once per front, in the first position only. This is intended to be guided by training and not enforcement in the tool.

If the first container on a front has this highlights layout, it will be positioned above the title piece/nav sections on both web and apps to showcase feature stories across the site.

The display on desktop is to show three content items and a "peek" of the fourth item.
On mobile and apps it will display two content items and a "peek" of the third item.

How to test

We deployed the changes to CODE. The Config Tool displayed the new layout type fixed/highlights in the drop down list for layout type properly:

Screenshot 2024-05-30 at 10 42 03

The new layout type fixed/highlights could be selected:

Screenshot 2024-05-30 at 10 42 25

After saving the container, I could see the new layout type in the output file config.json in S3:

Screenshot 2024-05-30 at 10 45 29

I reopened the config tool and the container was reloaded with the correct fixed/highlights type.

Implementation notes

When this PR is merged, the new layout type fixed/highlights will be available on the config tool, but it should not be used until we support this type of container on web/app.

We discussed this issue with the team (homepage redesign) and Jonathan Haynes. Given that the number of people using the tool in production is quite low, Jonathan believed that it should be sufficient just to make front editors aware that this new layout type should not be used for now.

We should inform users (and Jonathan) before we merge this PR.

Checklist

General

  • 🤖 Relevant tests added
  • ✅ CI checks / tests run locally
  • 🔍 Checked on CODE

Client

  • 🚫 No obvious console errors on the client (i.e. React dev mode errors)
  • 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
  • 📷 Screenshots / GIFs of relevant UI changes included

@waisingyiu waisingyiu marked this pull request as ready for review May 30, 2024 10:13
@waisingyiu waisingyiu requested a review from a team as a code owner May 30, 2024 10:13
Copy link
Contributor

@Georges-GNM Georges-GNM left a comment

Choose a reason for hiding this comment

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

Had a play on code - seems to be working as intended, and comparing with other PRs adding container layouts, this looks good to me!

@davidfurey
Copy link
Member

Looks good to me, but please let Central Production know before merging

@waisingyiu waisingyiu merged commit 4f11831 into main May 30, 2024
9 checks passed
@waisingyiu waisingyiu deleted the add-highlights-container branch May 30, 2024 15:48
@prout-bot
Copy link

Seen on PROD (created by @cemms1 and merged by @waisingyiu 6 minutes and 23 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.

5 participants