Fix Accordion margin/padding inconsistencies #3094
Merged
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
Affects smaller screens
Fixes a minor regression in: * 879e37c The bottom margin should be increased at tablet size (not desktop) so it matches up with the “Show all” button font size change
During a slow page load, the accordion content was briefly visible *without* padding
dc49960 to
84e7049
Compare
querkmachine
approved these changes
Dec 12, 2022
There was a problem hiding this comment.
Looks good to me.
I wonder if the excess button padding-bottom is a layover from the older accordion design, where the following content didn't have any top padding, which was added in #2640? Either way, makes sense to reduce it now.
colinrotherham
commented
Dec 12, 2022
| @@ -79,7 +83,7 @@ | |||
| cursor: pointer; | |||
| -webkit-appearance: none; | |||
|
|
|||
| @include govuk-media-query ($from: desktop) { | |||
| @include govuk-media-query ($from: tablet) { | |||
There was a problem hiding this comment.
Font sizes increase from tablet so we can add this margin sooner too
|
Thanks @querkmachine 🙌 |
Merged
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.
Just a quick PR to fix a few tiny Accordion issues we've had for a long time in:
"Show all" button
margin-bottomfix applied too wideMoves legacy bug fix to 'tablet' (from 'desktop') which align with font size
Section button
padding-bottomjumps 10px taller on mobileNotice how the section button height appears to get taller (unlike tablet, desktop)
Trailing space in
classattributeNow using
{%-to trim whitespace as we do elsewhereSee original Accordion design PR
Plus a minor fix where accordion sections lack padding (during slow page loads) since:
hidden='until-found'#3053Reducing layout shift
Whilst we know additional layout shifts take place when the accordion controls and section headers are injected, I've shifted
.govuk-accordion__section-contentcontent padding so it's applied whilst the JavaScript loadsOur Accordion sections are now open during page load, where previously we only added padding when
[hidden]was added by componentinit()but in preparation for the--expandedmodifier being toggled