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

Change layout of homepage #3743

Merged
merged 6 commits into from
Sep 25, 2023
Merged

Change layout of homepage #3743

merged 6 commits into from
Sep 25, 2023

Conversation

patrickpatrickpatrick
Copy link
Contributor

@patrickpatrickpatrick patrickpatrickpatrick commented Sep 1, 2023

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

Change the layout of homepage so that:

  • 'Popular on GOV.UK' links now appear in a 3 column grid layout on desktop
  • 'Services and information' and "Featured" sections are now next to each other on desktop.
  • The 'Featured' section now uses the new "Two Thirds" variation of the image-card component

Why

As part of several incremental changes being made to the homepage.

Trello Card

Visual Changes

Before

homepage-desktop-before

After

homepage-after

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 1, 2023 10:21 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 1, 2023 10:57 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 1, 2023 11:04 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 1, 2023 13:37 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 1, 2023 13:52 Inactive
@patrickpatrickpatrick patrickpatrickpatrick changed the title Change layout of homepage Change layout of homepage [DO NOT MERGE] Sep 4, 2023
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 7, 2023 10:32 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 11, 2023 10:34 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 13, 2023 15:02 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 14, 2023 11:16 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 14, 2023 13:06 Inactive
@patrickpatrickpatrick patrickpatrickpatrick changed the title Change layout of homepage [DO NOT MERGE] Change layout of homepage Sep 14, 2023
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 18, 2023 09:29 Inactive
@govuk-ci govuk-ci had a problem deploying to govuk-frontend-app-pr-3743 September 18, 2023 15:08 Failure
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 18, 2023 15:11 Inactive
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Some minor comments on the CSS. Still need to review the markup changes.

app/assets/stylesheets/views/_homepage.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/views/_homepage.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/views/_homepage.scss Outdated Show resolved Hide resolved
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 20, 2023 08:11 Inactive
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from 57526f0 to 098aafc Compare September 21, 2023 15:29
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 21, 2023 15:29 Inactive
The CSS is used for the grid is taken from the `cards` component.
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from 098aafc to 42ccdc7 Compare September 21, 2023 16:06
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 21, 2023 16:06 Inactive
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Great, thanks both @MartinJJones and @patrickpatrickpatrick for getting this into such a good place 🙌 We should probably now update the PR description as it's now longer reflective of the work, there have been quite a few design changes since.

I tested the page on the browsers and devices we usually test in. Didn't spot any issues, apart from how on mobile there seems to be a bit too much grey space below the Popular links. I checked with Monica who agreed that we should reduce the amount of space so that's something to look at tomorrow.

Screenshot 2023-09-21 at 17 40 55

I'm giving this PR a tick to indicate that I'm happy with everything apart from the above mobile spacing issue so whoever reviews the PR tomorrow can focus on reviewing any small fixes we add tomorrow.

app/views/homepage/_links_and_search.html.erb Outdated Show resolved Hide resolved
@hannalaakso hannalaakso changed the title Change layout of homepage [DO NOT MERGE YET] Change layout of homepage Sep 21, 2023
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 09:08 Inactive
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from a7477d4 to 116017c Compare September 22, 2023 09:09
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 09:10 Inactive
@MartinJJones
Copy link
Contributor

MartinJJones commented Sep 22, 2023

@hannalaakso thanks for finding and reporting the spacing issue on mobile, I've fixed this issue on a recent commit:

Screenshot 2023-09-22 at 10 19 20

- Ensures that the total padding-bottom on desktop underneath Popular links is 50px (30px + 20px from the popular links)
- Removed grid-auto-rows: fraction(1) as it is not required
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from 116017c to ba43a98 Compare September 22, 2023 12:42
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 12:42 Inactive
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from ba43a98 to d0dd87f Compare September 22, 2023 12:52
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 12:52 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 13:11 Inactive
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from 553599a to 1feee47 Compare September 22, 2023 13:13
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 13:13 Inactive
Temp style overrides to the image card component from the gem to:

- Ensure the space between the image card and the text remains at 15px on mobile, this was previously 30px on mobile.
- Ensure the featured image cards have 15px padding to the left and right when this screen width is =< 319px.
@MartinJJones MartinJJones force-pushed the change-layout-of-homepage branch from 1feee47 to 4363b40 Compare September 22, 2023 14:09
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3743 September 22, 2023 14:09 Inactive
@matthillco
Copy link
Contributor

I've also completed a secondary review of changes made today and it looks good to go.

@MartinJJones MartinJJones changed the title [DO NOT MERGE YET] Change layout of homepage Change layout of homepage Sep 25, 2023
@MartinJJones MartinJJones merged commit f904d4f into main Sep 25, 2023
7 checks passed
@MartinJJones MartinJJones deleted the change-layout-of-homepage branch September 25, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants