search

scottish-government-design-system / design-system

Design System for the Scottish Government and other Scottish public sector bodies
https://designsystem.gov.scot
MIT License
25 stars 11 forks source link

Header failing online accessibility checker #29

Closed 72gm closed 9 months ago

72gm commented 1 year ago

Hi,

When checking the accessibility of my app, using the online tools, the header was failing. Checking the SH header component on the SG website, it also fails e.g.

image

Thanks

jsutcliffe commented 1 year ago

This should be fixed as of version 0.0.349.

72gm commented 1 year ago

I've updated but appear to still be getting the same issue? My node modules also has the correct 349 version

image

jsutcliffe commented 1 year ago

Looking at your screenshot it looks like you're not using the latest version. You should see a "Menu" label that controls the menu checkbox still in place next to the "Menu" button when styles are turned off (the fix to your original issue makes it so that that label is now hidden with display:none instead of removed from the DOM). Do you need to rebuild your scripts, or clear a cache?

Screen Shot 2023-05-25 at 16 03 05
72gm commented 1 year ago

Yeah as you said, the issue was resolved with an update to the html (update of button to a label)... as opposed to the CSS ( it is only the CSS that we use dynamically) .. so in our React header component this fix didn't replicate...

To fix this we had to update the html in our React component with the new header html.. which sort identifies a flaw for people who have generated React/Angular/Vue components from the pattern library

i.e. when do we know that we should update our html as we won't necessarily know if there has been an update to the html (and it isn't a straight like for like copy as we will have optimised it for our framework)

72gm commented 1 year ago

Actually I'll need to look into this

The code that is in the sample html on your website fixes that issue but actually breaks the keyboard navigation on mobile... has the js changed as well?

The sample code is also different from what is running on the website...

jsutcliffe commented 1 year ago

There were no changes to the base HTML, only to some DOM transformations that occur when the component's JavaScript is initialised (the labels are now given a CSS class to hide them, instead of being removed from the DOM).

One way we try to avoid breaking changes at the moment is to avoid making changes to the base HTML unless it is truly unavoidable. Modifications to published components are almost always restricted to CSS and/or JavaScript, so that users only need to update their styles and scripts to get any improvements we make to components, rather than needing to also modify their templates.

As we are a small team with no in-house React experience it is difficult to offer support to people implementing the Design System in a React context, but perhaps something you could try is basing your React components' HTML on the DS components' base HTML (i.e. what we provide in the "sample HTML" blocks for each component) instead of the components' HTML post-initialisation, and let the Design System JavaScript carry out the DOM transformations.

72gm commented 1 year ago

Yeah we normally use the sample html code as our baseline.

I imagine, in this instance, that perhaps we decided to use the transformed code as it was easier for writing our own JS ( as we can't get your JS to work for previously advised reasons)

It looks like the change we are interested in is in 348 so we will try implementing this in our component library