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 details expanded state not announced on iOS #5089

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

36degrees
Copy link
Contributor

For some reason, setting display: inline-block on the summary means that VoiceOver on iOS no longer announces the expanded state of the <details> element.

We were using display: inline-block so that the interactive area and the focus state (when visible) were constrained to the text within the <summary>.

We can achieve the same thing by using display: block with width: fit-content.

However, now that we’re no longer using display: inline-block 1, the the 5px bottom margin on the summary and the 30px bottom margin on the details now collapse when the <details> is closed except in Chrome which has been updated to use content-visibility.

Preserve the existing margin on the component and make the behaviour across browsers consistent by establishing a new block formatting context using display: flow-root, preventing the margins from collapsing.

Safari < 13 does not support display: flow-root and so the margins will collapse, which means these older versions of Safari will have 5px less margin than when we were still using display: inline-block.

Fixes #2349

Footnotes

  1. ‘margins of inline-block boxes do not collapse (not even with their in-flow children)’ – https://www.w3.org/TR/CSS21/box.html#collapsing-margins

Copy link

github-actions bot commented Jun 18, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 118.58 KiB
dist/govuk-frontend-development.min.js 43.63 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 90.19 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 84.69 KiB
packages/govuk-frontend/dist/govuk/all.mjs 1.05 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 118.56 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 43.62 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 4.97 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 81.8 KiB 41.48 KiB
accordion.mjs 23.5 KiB 12.39 KiB
button.mjs 5.98 KiB 2.69 KiB
character-count.mjs 22.4 KiB 9.92 KiB
checkboxes.mjs 5.83 KiB 2.83 KiB
error-summary.mjs 7.89 KiB 3.46 KiB
exit-this-page.mjs 17.1 KiB 9.26 KiB
header.mjs 4.46 KiB 2.6 KiB
notification-banner.mjs 6.26 KiB 2.62 KiB
password-input.mjs 15.15 KiB 7.25 KiB
radios.mjs 4.83 KiB 2.38 KiB
service-navigation.mjs 4.46 KiB 2.69 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.05 KiB 6.06 KiB

View stats and visualisations on the review app


Action run for 8cdeef4

Copy link

github-actions bot commented Jun 18, 2024

Stylesheets changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
index 7fbe9acd4..336471ff9 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -2942,7 +2942,7 @@ screen and (forced-colors:active) {
     line-height: 1.25;
     color: #0b0c0c;
     margin-bottom: 20px;
-    display: block
+    display: flow-root
 }
 
 @media print {
@@ -2973,7 +2973,7 @@ screen and (forced-colors:active) {
 }
 
 .govuk-details__summary {
-    display: inline-block;
+    display: block;
     margin-bottom: 5px
 }
 
@@ -3026,6 +3026,8 @@ screen and (forced-colors:active) {
 @supports not (-ms-ime-align:auto) {
     .govuk-details__summary {
         position: relative;
+        width: -webkit-fit-content;
+        width: fit-content;
         padding-left: 25px;
         color: #1d70b8;
         cursor: pointer

Action run for 8cdeef4

Copy link

github-actions bot commented Jun 18, 2024

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/components/details/_index.scss b/packages/govuk-frontend/dist/govuk/components/details/_index.scss
index 296fcfdb0..903d81b00 100644
--- a/packages/govuk-frontend/dist/govuk/components/details/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/details/_index.scss
@@ -4,13 +4,13 @@
     @include govuk-text-colour;
     @include govuk-responsive-margin(6, "bottom");
 
-    display: block;
+    // Prevent bottom margins on summary and details from collapsing in Firefox
+    // and Safari, matching Chrome's behaviour (https://crbug.com/40815464)
+    display: flow-root;
   }
 
   .govuk-details__summary {
-    // Make the focus outline shrink-wrap the text content of the summary
-    display: inline-block;
-
+    display: block;
     margin-bottom: govuk-spacing(1);
   }
 
@@ -72,6 +72,10 @@
       // Absolutely position the marker against this element
       position: relative;
 
+      // Make the focus outline shrink-wrap the text content of the summary
+      width: -webkit-fit-content;
+      width: fit-content;
+
       // Allow for absolutely positioned marker and align with disclosed text
       padding-left: govuk-spacing(4) + $govuk-border-width;
 

Action run for 8cdeef4

@36degrees
Copy link
Contributor Author

Spreadsheet for testing:
https://docs.google.com/spreadsheets/d/1Z7SxQcK-HcwScI_dbSDLwidL7mDctsN0aFPLHaMPmK8/edit?gid=0#gid=0

So far I've not observed any changes other than in Safari on iOS 17.5.1 (21F90) where we see the desired improvement, with the expanded state announced on toggle and when returning to the summary:

Scenario Before After
When summary is focused (closed) Help with nationality, button, collapsed. Double-tap to expand. Help with nationality, button, collapsed. Double-tap to expand.
When toggling open Help with nationality. Help with nationality, expanded.
When summary is focused (open) Help with nationality, button. Help with nationality, button, expanded. Double-tap to collapse.

I don't have access to test with TalkBack on Android – it'd be fab if someone else was able to help with that.

For some reason, setting `display: inline-block` on the summary means that VoiceOver on iOS no longer announces the expanded state of the `<details>` element [1].

We were using `display: inline-block` so that the interactive area and the focus state (when visible) were constrained to the text within the `<summary>`.

We can achieve the same thing by using `display: block` with `width: fit-content`.

[1]: https://bugs.webkit.org/show_bug.cgi?id=230408
Now that we’re no longer using `display: inline-block` [^1], the the 5px bottom margin on the summary and the 30px bottom margin on the details now collapse when the <details> is closed *except* in Chrome which has been updated to use `content-visibility` [2].

Preserve the existing margin on the component and make the behaviour across browsers consistent by establishing a new block formatting context using `display: flow-root`, preventing the margins from collapsing.

Safari < 13 does not support `display: flow-root` and so the margins will collapse, which means these older versions of Safari will have 5px less margin than when we were still using `display: inline-block`.

[^1]: ‘margins of inline-block boxes do not collapse (not even with their in-flow children)’ – https://www.w3.org/TR/CSS21/box.html#collapsing-margins
[2]: https://issues.chromium.org/issues/40815464
[3]: https://html.spec.whatwg.org/multipage/rendering.html#the-details-and-summary-elements
@owenatgov
Copy link
Contributor

I've had a quick look at the margin issue mentioned in the description and I'm going to make the bold case that this isn't worth worrying about. As Ollie mentions, Safari <13, so 12 and lower, for both mac and iOS will functionally have 5px less margin. You can see this illustrated in the below screenshot of Safari mac 12.1 where the margin collapses into the focus state bottom border:

screenshot of the details component on Safari mac 12.1 where the margin collapses into the focus state bottom border

Safari 11 and 12 fall under Grade C in our browser support model which states the following about bugs in those browsers:

For grade C browsers we:

  • might remove support for individual features in these browsers at any time without considering them a breaking change
  • will not regularly test in these browsers
  • will not fix bugs affecting these browsers unless they prevent a user from being able to complete their task, or we can fix the bug by disabling a feature

I'm using this policy and the low impact to visual rhythm for Safari 12 and below users to justify not implementing a fix for this.

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.

Details expanded state is not announced by iOS VoiceOver
3 participants