-
Notifications
You must be signed in to change notification settings - Fork 327
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 flash of unstyled content in tabs component #1000
Conversation
Prior to this change the only way to avoid a flash of unstyled content was to add the aria-selected attribute into the markup. But if this attribute requires JavaScript to make sense, since if JavaScript is disabled this markup is not semantically a tabs component.
db39241
to
b69abb9
Compare
b69abb9
to
28a8f3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work !
@@ -80,7 +80,7 @@ Find out when to use the tabs component in your service in the [GOV.UK Design Sy | |||
|
|||
</section> | |||
|
|||
<section class="govuk-tabs__panel" id="past-week"> | |||
<section class="govuk-tabs__panel govuk-tabs__panel--hidden" id="past-week"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need both the hidden modifier and the selected modifier? Can the tabs not be hidden by default unless they have the --selected
modifier?
Only having one modifier simplifies the code a little bit and also avoids the possibility of a weird 'undefined' state where a tab has neither the selected modifier nor the hidden modifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hidden modifier for the panel is consistent with the conditional reveals, so not sure if it's worth breaking consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly… for context ,I know that I ended up refactoring the mobile nav in the Design System so that it didn't have both active
and hidden
modifiers, because it was confusing (and broken, because sometimes a menu item had neither modifier, and that scenario was poorly defined – see the first commit) so a little wary of this pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it seems there could be a less error prone way of doing this, but given this is consistent with another component and making tab panels hidden by default would be a breaking change, could we raise that as a separate issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Closes #927
On slow connections or devices users can see a flash of the unstyled version of the tabs.
This PR updates the tab component so that it's initial state can be set without the need for JavaScript.
I experimented with being able to set a specific panel, which I thought would allow rendering of selected tabs without any flashing, but since the hash set in the URL is not available to the server this would not be practical. - https://stackoverflow.com/a/318581
Since I could not think of an immediate need for someone to set a default tab other than the first one it seemed over the top and something we could add later on.
Demonstrations
Before, flash of unstyled content
After, no flash with default tab
Flash when changing tabs and reloading
https://trello.com/c/sNeuUPNH/1386-2-fix-flashes-of-unstyled-content-caused-by-the-tabs-component