Fix footer alignment with grid classes #2428
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
lfdebrux
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
27a1265
to
ea0390d
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
lfdebrux
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
ea0390d
to
b3b32af
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
lfdebrux
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
b3b32af
to
1be08b8
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
lfdebrux
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
a231741
to
412b887
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
412b887
to
10b9930
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
0e9d2fb
to
5eb9bd5
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
5eb9bd5
to
79a21c3
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
This commit moves the column modifier (`govuk-footer__list--columns-{{ nav.columns }}`)
to the footer section instead (`govuk-footer__section--span-{{ nav.columns }}`)
This is so that the number of columns influences how far the overall section spans the grid.
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
a355ea5
to
fd10d14
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
fd10d14
to
453bfea
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
453bfea
to
d4b82f7
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
Based on [[1]], we can't use grid autoplacement in any version of Internet Explorer, so we have to go back to flexbox. We need to use border-box sizing and padding to make the gutter work the way we want, instead of using margins. [1]: https://css-tricks.com/css-grid-in-ie-faking-an-auto-placement-grid-with-gaps/ This also lets us support the (old) GOV.UK footer layout, where there were two rows, one with a single 2 column section that took 2/3rds of the space (followed by empty space), and a second with a 2 column section and a 1 column section. Having `flex-grow` would mean that the 2 column section in the first row would take up all the space. This is also closer to what the grid layout does out of the box, so that's also good. Note: we're using `width` instead of `flex-basis` to support IE11 See [flexbug #7] for more details on why this is needed. flexbug #7: https://github.com/philipwalton/flexbugs#flexbug-7
We want the footer to switch from a multi-section multi-column affair, to a one-section-per-row single-column-per-section layout on mobile. This commit adds a media query so that the switch happens at the tablet breakpoint. However, there's lots of space in between the mobile layout and the desktop layout for all sorts of responsive designs, which this commit eschews; so we might want to revisit this.
Adds clearfix and `float: left`, mainly for IE8, which doesn't support flexbox. Using this with flexbox is okay, because `display: flex` causes flex items to ignore floats and clears [[1]]. This makes our footer work even more like our page "grids". [1]: https://www.w3.org/TR/css-flexbox-1/#flex-containers
The original styling for the footer section has `display: inline-block`. Given our new styling and the `float` we apply in IE8, this is no longer needed.
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
d4b82f7
to
642be02
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
The previous implementation had the margin above the list set to 40px on desktop and 25px on mobile, and the margin below the list set to 30px at all breakpoints. This commit makes the margins equal - it reduces the top margin to 30px as it was felt that the 40px made the list too far away from the heading.
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
vanitabarrett
force-pushed
the
ldeb-spike-footer-flexbox
branch
from
31b2f99
to
609bf00
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2428
|
Note: this PR changes the behaviour slightly of what happens on screen sizes smaller than desktop. The previous behaviour gradually dropped the width of each column, e.g: from thirds, to halves, to full width. This PR changes that behaviour so it drops straight to full width. This means you can end up with quite a large full width footer on tablet, e.g: Example - before (tablet)
Example - after (tablet)
We could make a change to drop down to 50% width on tablet. However, this would mean that columns that have perhaps been given more visual importance (see example below of the 'Coronavirus' section in the GOV.UK footer) would in some ways have less importance on tablet as the column below would sit alongside it. Desktop - Coronavirus column sits on its own row
Tablet - Coronavirus column sits alongside "Services and Information"
|
vanitabarrett
changed the title
14 tasks
Do we need a designer to review this change? |
|
Superseded by #2462 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1539. It also partly addresses #1726, along with #2446
Unfortunately this PR does not fix any of the other bugs reported in #1680
What
Adjust the styles of the footer layout to better match up with the grid classes. The screenshot below shows the grid classes and the footer not aligning properly:
Note: It's worth comparing and contrasting these changes with those in branch
nickcolley-ldeb-spike-footer-grid, which achieves much the same effect with fewer lines and fewer commits by using CSS grids. The CSS grid version also behaves slightly better as the viewport changes size. However, CSS grids are not supported by all browsers we want to support, so I think this PR that uses flexbox should take precedence.Browsers to test in:
For each browser, test the two main footer examples:
and zoom in to 400%.
Screenshots
Note: this is just a sample of screenshots where the change is most visible
IE8 GOV.UK example - Before vs After
IE8 Two Column Left example - Before vs After
IE8 Two Column Right example - Before vs After
IE8 Equal Three Column example - Before vs After
IE10 Two Column Right example - Before vs After