Fix flash of unstyled content with conditional reveals #885

NickColley · 2018-07-11T11:02:21Z

If the conditional reveal JavaScript is slow to execute it can result in showing the user their contents briefly which can be jarring.

See the original issue for an example: #787

Fixes #787

Tested in:

IE 8 ✅
IE 9 ✅
IE 10 ✅
IE 11 ✅
Edge 17 ✅
Firefox 61 ✅
Chrome 67 ✅
Safari 11.1 ✅

Assistive technologies:
I've tested this in VoiceOver and since display: none has the same behaviour as aria-hidden=true (styled with display: none), I've decided not to do a full test of assistive technologies.

Pages to review:

https://trello.com/c/o9krC0SR/1195-conditional-reveals-result-in-a-flash-of-unstyled-content-as-the-page-loads

NickColley · 2018-07-11T11:04:31Z

src/components/radios/template.njk

@@ -61,7 +61,7 @@
      }) | indent(6) | trim }}
    </div>
    {% if item.conditional %}
-      <div class="govuk-radios__conditional" id="{{ conditionalId }}">
+      <div class="govuk-radios__conditional{% if not item.checked %} is-hidden{% endif %}" id="{{ conditionalId }}">


For users that do not use macros, how do we document this?

Could we handle toggling this in JS somehow? If we add .js-hidden to all conditionals markup by default, no FOUC. When JS loads we check the preceeding .govuk-checkboxes__item sibling. If the input inside of it is checked, remove .js-hiddenfrom the conditional.

Actually I realised that this still wouldn't work because we need to we do .js-enabled .js-hidden in CSS so that style doesn't kick in until JS has loaded 😞 I'll keep on thinking.

That's currently how the JavaScript logic works. The logic in the template is for the initial render.

I think this might not be a big issue since the example we use in the Design System website has a pre-checked input which means you'd see it in the generated HTML.

NickColley · 2018-07-11T11:42:32Z

src/components/checkboxes/_checkboxes.scss

@@ -139,7 +139,7 @@
    padding-left: $conditional-padding-left;
    border-left: $conditional-border-width solid $govuk-border-colour;

-    &[aria-hidden="true"] {
+    .js-enabled &.is-hidden {


Should this class be called 'js-hidden'?

Could this be a common utility class?

#815

I think a common utility class would make sense.

(On a separate note, I think we should probably prefix our JS classes. But let's discuss that another time, I don't want to derail this PR 😁 )

You thinking govuk-js-hidden?

It's OK to raise things like this as a blocker since classes are part of our public API, so you're not derailing things.

Yes I was thinking of govuk-js-hidden. I was even thinking of js-hidden-govuk as it's handy to recognise JS classes at a glance. But it just sounds too weird... 😅

But js-hidden just seems like it would very easily conflict with something else.

Agreed - makes sense, looks like we already use js-hidden, so maybe raise an issue for us to think about it after all 😬

https://github.com/alphagov/govuk-frontend/search?q=js-hidden&unscoped_q=js-hidden

I'm going to change the 'is-hidden' to 'js-hidden' to be consistent with tabs.

NickColley · 2018-07-13T16:10:36Z

src/components/tabs/_tabs.scss

      }
    }

  }
-
-  .js-hidden {


good news is the tests found this selector being defined globally

i imagine I should fix this behaviour in a different PR first...?

Makes me think having this class used with slightly different behaviours is indeed dangerous.

So we should either

Continue doing js-hidden for specific components but namespacing with their component names: govuk-tabs-js-hidden or .govuk-tabs .js-hidden?

Pull js-hidden out into a utility class that is used by tabs and conditional reveals

I feel 2. is probably the best way, since others can use this class too.

js-hidden is something from the old world too, so it'll always be a 'helper' that's defined globally in gov.uk template.

Pushed up some scrappy working to see what that might look like, will do something clean next week.

Uses the presence of the `js-enabled` class to know when to hide or show content.

NickColley · 2018-07-19T10:10:24Z

src/components/checkboxes/checkboxes.js

@@ -40,7 +41,7 @@ Checkboxes.prototype.setAttributes = function ($input) {
  $input.setAttribute('aria-expanded', inputIsChecked)

  var $content = document.querySelector('#' + $input.getAttribute('aria-controls'))
-  $content.setAttribute('aria-hidden', !inputIsChecked)
+  $content.classList.toggle('govuk-checkboxes__conditional--hidden', !inputIsChecked)


In the case that CSS fails but JS succeeds, should we also include 'aria-hidden'? Or is this a bit too over the top?

I reckon this is fine as is.

hannalaakso

This looks good to me 👏

36degrees · 2018-07-24T08:40:37Z

@NickColley is there anything blocking this from being merged?

NickColley · 2018-07-24T08:51:42Z

@36degrees just wanted to get more eyes on it since it's a big change

36degrees

LGTM, though tests could be more focussed.

36degrees · 2018-07-24T08:55:47Z

src/components/checkboxes/template.test.js

+      const $component = $('.govuk-checkboxes')
+
+      const $firstConditional = $component.find('.govuk-checkboxes__conditional').first()
+      expect($firstConditional.html()).toContain('Conditional content that should be hidden')


It'd be good to split this out into its own test and just have the one assertion per test.

36degrees · 2018-07-24T08:56:13Z

src/components/checkboxes/checkboxes.js

@@ -40,7 +41,7 @@ Checkboxes.prototype.setAttributes = function ($input) {
  $input.setAttribute('aria-expanded', inputIsChecked)

  var $content = document.querySelector('#' + $input.getAttribute('aria-controls'))
-  $content.setAttribute('aria-hidden', !inputIsChecked)
+  $content.classList.toggle('govuk-checkboxes__conditional--hidden', !inputIsChecked)


I reckon this is fine as is.

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 11, 2018 11:02 Inactive

NickColley commented Jul 11, 2018

View reviewed changes

NickColley force-pushed the fix-fouc-with-conditional-reveals branch from 7a3f0e2 to 6e67929 Compare July 11, 2018 11:36

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 11, 2018 11:37 Inactive

NickColley changed the title ~~WIP: Fix fouc with conditional reveals~~ Fix flash of unstyled content with conditional reveals Jul 11, 2018

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 11, 2018 11:40 Inactive

NickColley commented Jul 11, 2018

View reviewed changes

NickColley force-pushed the fix-fouc-with-conditional-reveals branch from feb5ddd to 4a3cb14 Compare July 13, 2018 15:44

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 13, 2018 15:44 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 13, 2018 16:09 Inactive

NickColley commented Jul 13, 2018

View reviewed changes

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 13, 2018 16:24 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 13, 2018 16:25 Inactive

NickColley changed the title ~~Fix flash of unstyled content with conditional reveals~~ [Do not merge] Fix flash of unstyled content with conditional reveals Jul 13, 2018

NickColley added the 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) label Jul 16, 2018

NickColley force-pushed the fix-fouc-with-conditional-reveals branch from b76f36d to 4a3cb14 Compare July 17, 2018 16:27

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 17, 2018 16:27 Inactive

NickColley force-pushed the fix-fouc-with-conditional-reveals branch from 4a3cb14 to 08d21f7 Compare July 17, 2018 16:35

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-885 July 17, 2018 16:35 Inactive

NickColley changed the title ~~[Do not merge] Fix flash of unstyled content with conditional reveals~~ Fix flash of unstyled content with conditional reveals Jul 17, 2018

NickColley added 4 commits July 17, 2018 17:36

Remove FOUC for Checkbox conditional reveals

530d91b

Uses the presence of the `js-enabled` class to know when to hide or show content.

Remove FOUC for Radio conditional reveals

d02466e

Uses the presence of the `js-enabled` class to know when to hide or show content.

Apply classList polyfill to Radios and Checkboxes

d10ca39

Add CHANGELOG entry for conditional fouc fixes

745c008

NickColley force-pushed the fix-fouc-with-conditional-reveals branch from 08d21f7 to 745c008 Compare July 17, 2018 16:37

govuk-design-system-ci deployed to govuk-frontend-review-pr-885 July 17, 2018 16:37 View deployment

NickColley changed the title ~~Fix flash of unstyled content with conditional reveals~~ [Do not merge] Fix flash of unstyled content with conditional reveals Jul 18, 2018

NickColley commented Jul 19, 2018

View reviewed changes

NickColley changed the title ~~[Do not merge] Fix flash of unstyled content with conditional reveals~~ Fix flash of unstyled content with conditional reveals Jul 19, 2018

hannalaakso approved these changes Jul 20, 2018

View reviewed changes

kr8n3r added this to the [NEXT] milestone Jul 20, 2018

NickColley merged commit e41a9b3 into master Jul 24, 2018

NickColley deleted the fix-fouc-with-conditional-reveals branch July 24, 2018 08:54

36degrees reviewed Jul 24, 2018

View reviewed changes

kr8n3r mentioned this pull request Jul 26, 2018

Release v1.2.0 (feature release) #928

Merged

NickColley mentioned this pull request Aug 13, 2018

Conditional reveal aria-hidden/display none being used #686

Closed

colinrotherham mentioned this pull request Dec 13, 2022

Hide Accordion content (again) during .js-enabled page load #3095

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flash of unstyled content with conditional reveals #885

Fix flash of unstyled content with conditional reveals #885

NickColley commented Jul 11, 2018 •

edited

Loading

NickColley Jul 11, 2018

hannalaakso Jul 13, 2018

hannalaakso Jul 13, 2018

NickColley Jul 13, 2018

NickColley Jul 11, 2018

hannalaakso Jul 13, 2018

NickColley Jul 13, 2018 •

edited

Loading

hannalaakso Jul 13, 2018

NickColley Jul 13, 2018 •

edited

Loading

NickColley Jul 13, 2018

NickColley Jul 13, 2018 •

edited

Loading

NickColley Jul 13, 2018 •

edited

Loading

NickColley Jul 13, 2018

NickColley Jul 19, 2018

36degrees Jul 24, 2018

hannalaakso left a comment

36degrees commented Jul 24, 2018

NickColley commented Jul 24, 2018

36degrees left a comment

36degrees Jul 24, 2018

36degrees Jul 24, 2018

Fix flash of unstyled content with conditional reveals #885

Fix flash of unstyled content with conditional reveals #885

Conversation

NickColley commented Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

NickColley Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannalaakso left a comment

Choose a reason for hiding this comment

36degrees commented Jul 24, 2018

NickColley commented Jul 24, 2018

36degrees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley commented Jul 11, 2018 •

edited

Loading

NickColley Jul 13, 2018 •

edited

Loading

NickColley Jul 13, 2018 •

edited

Loading

NickColley Jul 13, 2018 •

edited

Loading

NickColley Jul 13, 2018 •

edited

Loading