Update accordion design #2257
Merged
Update accordion design #2257
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
chris-gds
force-pushed
the
update-accordion-design
branch
from
09b03c9 to
bd3f1da
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
bd3f1da to
cf05754
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
cf05754 to
3925a54
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
3925a54 to
c80a2b0
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
c80a2b0 to
9c38bbf
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
9c38bbf to
5d9fd04
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
5d9fd04 to
7484968
Compare
hannalaakso
reviewed
Jul 12, 2021
hannalaakso
reviewed
Jul 13, 2021
chris-gds
force-pushed
the
update-accordion-design
branch
from
7484968 to
4169b05
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
4169b05 to
271685c
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
271685c to
f40755b
Compare
chris-gds
force-pushed
the
update-accordion-design
branch
from
f40755b to
02aa99f
Compare
|
Browser testing - Edge - Windows High Contrast Mode accordion_edge_hcm.mp4 |
In `index.scss`: - Remove `top-border` rule from button as this is already set on `section-heading`. - Remove `govuk-em` for spacing as we tend to set spacing in pixels. - Remove variable definitions when the variable is only used once. - Only set `padding-top`, not all padding properties, for `.govuk-accordion__section` as it was the only one initially set. - Restore `.govuk-accordion__section-content` and `.govuk-accordion__section--expanded .govuk-accordion__section-content` to the top of the file to make the PR easier to review (we could open another PR to do some wider refactoring of the file if we felt that was useful). - Remove redundant `position: relative` - Add a couple of code comments. In `campaign-page/index.njk`: - Remove unintended punctuation changes.
Add a transparent bottom border to each section to visually separate the section from
the one underneath when user changes colours in their browser ("high contrast mode").
Follow the style of the cookie banner transparent bottom border which solves a similar issue.
Move the top border style from the section heading to the button elements so that it blends
into the bottom border when user changes colours in their browser.
Reduce the padding in the button to account for the increased spacing created by the
transparent border.
Remove the transparent border when a section is expanded to make it clear that the heading relates to the content below. See #2257 (comment)
The fact that we remove the native spacing and border from `<button>` makes the enclosed elements (which are aligned left and flush against the edge of the component) break out of the button container by 1px which is just about visible when user changes the colours in Firefox (this issue is actually present in the current Design System accordion too: the heading inside the button protrudes very slightly out of the container but this isn't noticeable since in that design the button is only around the heading element).
I had initially considered adding a Firefox-high-contrast-mode only hack to hide the overflow of the container but this interferes with the rendering of the focus state, so I think we'll have to leave it as is, it's a minor issue anyway.
This impacts Firefox High Contrast mode since it shows the outline of `<button>` elements by giving them a background colour. Windows High Contrast Mode does not do this, so the issue isn't
visible there.
We were explicitly setting line-heights on the button and the toggle - normally the govuk-font mixin will do this for us and will choose a line-height value appropriate to the font size. We were seeing an issue with the focus state on smaller screens where there was a thin black line between lines. See #2322 for more detail. This commit removes the explicitly set line-height so that the focus state displays as expected.
Adjust text in default examples to more realistically demonstrate text alignment. Update test to reflect new example content. Remove accordion <sup> examples. These examples were added to test a different combination of HTML elements in the accordion component. However, the <sup> element specifically pushes text in the accordion out of line and therefore requires additional styling if it's going to be used (vertical-align: top). We also don't think it's likely that a user would want to use <sup> inside the summary line.This commit removes the <sup> from the accordion examples. The existing examples still test the accordion when passing in HTML content, so that is still covered. Add an example of an accordion using the summary line that reflects the upcoming guidance update which advises keeping the summary line text short.
Some small adjustments to align with GOV.UK design. Increase bottom padding on buttons to 25px. Note that we only need 15px padding here, as another 10px comes from the invisible bottom border that improves high contrast mode presentation. Add more spacing between the elements inside the button on mobile. Increase padding below the 'Show' on tablet/desktop to about 30px. On mobile it should be about 20px. Also fix alignment of IE8 show/hide icon. Fixes #2324
Insert a comma after the summary line in the markup, similar to the punctuation inserted after the heading. The GDS accessibility clinic gave us the feedback that the punctuation symbol which was originally introduced to improve NVDA announcements also improves the semantics of the button text in general by dividing up its content thematically. See #2327 (comment)
Fixes #2371 This commit removes the font and line-height being set on the accordion button, which we were relying on to style the accordion heading text. It seemed like this was affecting the other text elements inside the accordion sections and impacting the focus state. Instead, we are directly setting the font and line height on the accordion heading text. Although browsers will set their own font styles for buttons, this shouldn't affect the component because we're setting the text styles on the individual span elements within the button. These should override any browser styles being set. We also need to adjust the spacing between text elements a bit as a result, otherwise the height of the accordion (and therefore the touch target is reduced). Read more about that here #2371 (comment)
Co-authored-by: Chris Yoong chris-gds@users.noreply.github.com Remove the CSS adjustment that made the NVDA browse mode perceive the button content as one element. Whilst this was potentially an improvement to how NVDA users perceived the button, we found in testing that it caused long announcements to be cut off. More details of the initial fix here: 6eb5bf7 The reason for the announcement being cut off appears to be related to the character limit for line length that NVDA users can set for the browse mode. The issue was that whilst the browse mode default behaviour is to read out the subsequent line(s) when user presses down again (so that eventually the user will hear all of the content), here those subsequent lines were not being read out - the reason for this could be related to the amount of content inside the button and NVDA perceiving that there was more content (namely the button element itself) to announce. @chris-gds has done a more thorough write up of the issue of the NVDA announcements being cut off as it was observed in: #2373 (comment)
The CSS adjustment removed in #132c12f had the favourable side effect of limiting the width of the focus state for the three text items (heading, summary, show/hide), stopping the focus state from expanding beyond the text to the full width of the component. To limit the width of the focus state without the NVDA adjustment, this commit: - adds focus containers in the markup inside the three items using JavaScript - makes the focus containers inline items - moves the focus styles to those classes; the inline style sets the width of the focus state to the width of the text - renames `.govuk-accordion__section-heading-focus-wrapper` to `.govuk-accordion__section-heading-text` since the class no longer has the focus styles Change the text containers to set bottom margin instead of top margin as this: - resolves an issue with the summary line margin: since the summary line container is now a block level one, it renders the `.govuk-body` bottom margin; moving the margin to the bottom of the container makes the container margin collapse with the `.govuk-body` bottom margin - is more consistent with how rest of GOV.UK Frontend sets margins As the last text container inside the butotn now has a bottom margin, we also don’t need set the bottom margin on the button anymore. Also remove a now redundant rule that would add an underline to improve affordance for text inside pseudo elements since we no longer use pseudo elements for text content inside the button.
Also remove the rule for Windows High Contrast mode as this issue only impacts Firefox when user changes colours.
This commit does not change any code, only reorders it.
We originally intended to remove the ID from the template markup as it's no longer required by `aria-describedby` in the accordion section header. Revert that change since we've decided not to ask teams to make this breaking change yet. We want to wait to see if we receive any feedback from teams regarding the issue flagged in https://design-system.service.gov.uk/components/accordion/#known-issues-and-gaps (refer to https://deploy-preview-1961--govuk-design-system-preview.netlify.app/components/accordion/#known-issues-and-gaps until we publish the v4 guidance in the Design System).
The data-nosnippet attribute stops the 'show' text from appearing in any Google Search text snippets. We have successfully used a similar approach for the [cookie banner](#2192) and on [GOV.UK Publishing components](alphagov/govuk_publishing_components#1185)
Copy the section heading text ID directly from the span to the heading text, instead of constructing it in the JS, since the span ID attribute comes directly from the component template, as does the expandable content area `aria-labelledby` which references the ID. I considered, but decided against, programmatically adding the summary line ID (if it exists) to the expandable content area `aria-labelledby` in addition to the heading ID since in the current component the heading text, which the expandable content `aria-labelledby` refers to, in turn has an `aria-labelledby` which references the summary. However, at least in the Chrome accessibility tree it appears that this link doesn't mean that the current component expandable area gets access to the summary line via the heading.
hannalaakso
force-pushed
the
update-accordion-design
branch
from
dcfff7f to
9e15ea0
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2257
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.
What
Update the Design of the Accordion component to improve accessibility.
See a summary in the Accordion backlog issue.
Why
The existing iteration of the Accordion: the section headings in accordions can be mistaken for links, but are treated as buttons - this fails WCAG 2.1 success criterion 1.3.1 Info and Relationships
this update addresses this.
Also, there was initially 3 different Accordion Designs on different parts of GOV.UK Publishing. This impacted WCAG 3.2.4 as the differences were not consistent they were ultimately confusing for users.
(This iterative update within the Design System (GOV.UK Frontend) will also be updated on GOV.UK Publishing Gem)
Visuals
(Before: Left, After: Right)
alphagov/govuk-design-system#1706
Browser testing
Selected Results:
(Trigger warning: GIFs to follow do not pause after 3 loops)
IE8
IE9
IE10
IE11
Edge
Chrome zoom
chrome-zoom300.mov
Firefox Zoom-text only
firefox-zoom-text-only.mov
Android Pixel 5 Chrome
Android Samsung 20 Chrome
Android Samsung 20 Samsung Internet
Safari macOS Catalina
Safari macOS BigSur
iPadPro
iPhone 11 Pro Max
iPhone 12
iPhone SE 2020
Chrome - iPhone 12
Assistive Technology testing
Known issues
Untested
Results:
JavaScript Disabled
CSS Disabled
CSS + JS Disabled
VoiceOver Safari
safari-macos-voiceover.mov
VoiceOver iOS (14.6)*
*
mainlandmark present in testing environmentVoiceOver-iOS.mp4
NVDA 2020.4 / Firefox 88
NVDA-firefox.mov
NVDA 2020.4 / Chrome
NVDA-CHROME.mov
JAWs / Chrome
jaws-chrome.mov
JAWs / IE
Jaws-IE.mov
Chrome (Blurred vision, protanopia, deuteranopia, tritanopia and achromatopsia)
Blurred-protanopia-deuteranopia-tritanopia-achromatopsia.mov
Chrome (Increase contrast, Greyscale, Inverted colour, Inverted Greyscale, Yellow on black)
Chrome-colour-changes.mov
Windows Magnifier
Chrome (Windows High Contrast Modes)
Chrome-WindowsHighContrast.mov
Edge (Windows High Contrast Mode)
Edge-windows-highcontrast.mov
IE11 (Windows High Contrast Mode)
Anything else
Breaking Change as the ID has now been removed from the summary element within the markup
User preferred colours
User's may adjust their colour schemes. Testing with different colour schemes revealed some problems, having nested elements within a button required additional adjustments in order to improve the experience.
There is further conversation about this use and an opinion piece can be found here
w3c/csswg-drafts#5433 (comment)
NVDA fix
Testing videos of NVDA
Before - the sections are broken up as user navigates.
Within reader-mode the user requires multiple keypresses to navigate, also the audio is heard in an disjointed manner.
After - read out as one whole block.
NVDA-fix.mov
Additional Details and screenshots:
The issue before:
After:
To replicate:
Firefox > preferences > Language and appearance > Colours:Update to high contrast scheme
Update to reduced contrast scheme
Details of breaking change