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 column alignment in the "More on GOV.UK" homepage component #3253

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

matthillco
Copy link
Contributor

@matthillco matthillco commented Jun 1, 2022

In tablet view and above, the columns in this component were misaligned with the columns in the Featured component above. This is due to two incompatible layout approaches.

Several approaches to align these elements were investigated, using combinations of float based layouts and CSS grid. However these approaches did not yield any compatible alignments, possibly due to differences in how browsers calculate % widths vs fr widths.

The fix was to use CSS columns with a column-gap matching the gutter in the row/columns layout classes, and limiting the width of the list to two columns out of three (66.66%). This aligns the elements correctly while keeping the required top -> bottom then left -> right behaviour of the links.

Note that the break-inside: avoid-column and the padding-top and padding-bottom tweaks are used to prevent triggering a possible bug in Safari where focused anchors in a multi-column layout cause links to jump between columns. As this is not a long-term robust solution, we should investigate this further for a possible refactor.

Heroku preview

Trello card

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

Visual changes

Before

image

After

frontend-pull-3253 herokuapp com_

Copy link
Contributor

@patrickpatrickpatrick patrickpatrickpatrick left a comment

Choose a reason for hiding this comment

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

LGTM!

@injms
Copy link
Contributor

injms commented Jun 7, 2022

I'm seeing an odd behaviour in Safari1 - when tabbing through the list of links, the "Check your State Pension age" link moves to the left column:

Screenshot 2022-06-07 at 16 07 13

Then when tabbing to the next link, it reverts back to the second column:

Screenshot 2022-06-07 at 16 07 18

This behaviour doesn't happen on the live version of the homepage.

Footnotes

  1. Safari version 15.5 (16613.2.7.1.9, 16613)

@matthillco matthillco force-pushed the mh-homepage-fix branch 3 times, most recently from d5b49ad to ffd89d7 Compare June 8, 2022 16:12
In tablet view and above, the columns in this component were
misaligned with the columns in the Featured component above.
This is due to two incompatible layout approaches.

Several approaches to align these elements were investigated,
using combinations of float based layouts and CSS grid. However
these approaches did not yield any compatible alignments,
possibly due to differences in how browsers calculate `%`
widths vs `fr` widths.

The fix was to use CSS `columns` with a `column-gap` matching
the gutter in the row/columns layout classes, and limiting the
width of the list to two columns out of three (66.66%). This
aligns the elements correctly while keeping the required top ->
bottom then left -> right behaviour of the links.

Note that the `break-inside: avoid-column` and the padding-top
and padding-bottom tweaks are used to prevent triggering a
possible bug in Safari where focused anchors in a multi-column
layout cause links to jump between columns. As this is not a
long-term robust solution, we should investigate this further
for a possible refactor.
Copy link
Contributor

@injms injms left a comment

Choose a reason for hiding this comment

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

Looks good - and thanks for fixing that quite esoteric Safari bug 👍

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