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

Fix spacing around figure component #3259

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Fix spacing around figure component #3259

merged 1 commit into from
Jul 9, 2024

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Jul 9, 2024

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

What / why

This fixes the spacing around the government-frontend figure component, specifically on mobile.

  • on mobile, if a figure had a caption there was no space between it and the following content
  • however if there was no caption, there was a space, because the empty figcaption element was still being rendered, which included spacing at the top (to prevent it from being too close to the image)
  • this change fixes this problem and refactors the styling of the component in the following ways
  • document the undocumented feature of an image credit beneath the caption
  • only show the figcaption element if there is a caption or a credit
  • add spacing between caption and credit when present (haven't found any live examples of this, but can be passed on case study pages)
  • stop using the responsive mixin for component margin, as confusing and unnecessary
  • instead explicitly set margin bottom on the component to be the same for all screen sizes (it's a big space on desktop, looks fine everywhere else too)
  • retain the margin 0 for the component, as by default browsers give figure elements margin all around
  • increase the font size of the caption and credit using the govuk-font mixin (it was 14 on desktop, which is too small - it's still 14px on mobile, but this will automatically be increased to 16 when govuk-frontend v5 is rolled out)

Example page with the problem: https://www.gov.uk/government/news/secretary-of-state-meets-first-and-deputy-first-ministers

Visual changes

Before (desktop) After (desktop)
Screenshot 2024-07-09 at 09 24 30 Screenshot 2024-07-09 at 09 23 37
Before (mobile) After (mobile)
Screenshot 2024-07-09 at 09 23 45 Screenshot 2024-07-09 at 10 41 06

Trello card: https://trello.com/c/03XY4VZm/236-no-padding-below-the-image-caption

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3259 July 9, 2024 08:26 Inactive
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3259 July 9, 2024 08:35 Inactive
Copy link
Contributor

@maxgds maxgds left a comment

Choose a reason for hiding this comment

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

Could you get a designer eye on this - particularly with the post v5.x font size? To me the white space looks like a bit too much on mobile. Otherwise I'd be happy to approve this.

@andysellick
Copy link
Contributor Author

@maxgds here's a screenshot of the spacing reduced on mobile. Personally I think it looks a little tight (especially since the font size of the caption will increase from 14px to 16px once v5 is rolled out, making it the same size as the text of the article) but I'm not overly fussed, the important thing is having some space 😁

Now When v5 goes out
Screenshot 2024-07-09 at 10 41 06 Screenshot 2024-07-09 at 10 43 33

Copy link
Contributor

@maxgds maxgds left a comment

Choose a reason for hiding this comment

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

Looks improved to me, if you're happy then I am, but feel free to take an opinion from a designer if you can find one with spare time and revert if they agree it's squeezed.

- on mobile, if a figure had a caption there was no space between it and the following content
- however if there was no caption, there was a space, because the empty figcaption element was still being rendered, which included spacing at the top (to prevent it from being too close to the image)
- this change fixes this problem and refactors the styling of the component in the following ways
- document the undocumented feature of an image credit beneath the caption
- only show the figcaption element if there is a caption or a credit
- add spacing between caption and credit when present (haven't found any live examples of this, but can be passed on case study pages)
- stop using the responsive mixin for component margin, as confusing and unnecessary
- instead explicitly set margin bottom on the component to be the same for all screen sizes (it's a big space on desktop, looks fine everywhere else too)
- retain the margin 0 for the component, as by default browsers give figure elements margin all around
- increase the font size of the caption and credit using the govuk-font mixin (it was 14 on desktop, which is too small - it's still 14px on mobile, but this will automatically be increased to 16 when govuk-frontend v5 is rolled out)
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3259 July 9, 2024 10:07 Inactive
@andysellick andysellick merged commit 79dd58d into main Jul 9, 2024
11 checks passed
@andysellick andysellick deleted the fix-figure branch July 9, 2024 10:24
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.

3 participants