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: use inline SVGs #360

Merged
merged 1 commit into from
May 18, 2022
Merged

fix: use inline SVGs #360

merged 1 commit into from
May 18, 2022

Conversation

gregtyler
Copy link
Contributor

@gregtyler gregtyler commented May 17, 2022

Rather than embedding SVGs as background images, use them inline. This allows them to keep their parents' fill colour, so they can always adapt to match the text.

When forced color mode is active (e.g. Windows High Contrast mode), we need to force the fill colour to match the context (i.e. text or link colour), otherwise it doesn't pass through properly on Windows.

This is an accessibility enhancement as it means that users who use tools like high-contrast mode to force changes in background and foreground colours will continue to be able to see iconography. Without it, icons are invisible if they're the same colour as the forced background colour.

Fixes #162

@gregtyler gregtyler requested a review from a team as a code owner May 17, 2022 10:02
@euhyung
Copy link

euhyung commented May 17, 2022

https://hidde.blog/making-single-color-svg-icons-work-in-dark-mode/ and https://adrianroselli.com/2018/03/svg-filtering-for-windows-high-contrast-mode.html - and svg needs to meet Success Criterium 1.1.1, ie either it is decorative (hide) or functional (wrap with accompanying link or standalone needs accessible text with it.

Rather than embedding SVGs as background images, use
them inline. This allows them to keep their parents' fill
colour, so they can always adapt to match the text.

When forced color mode is active (e.g. Windows High
Contrast mode), we need to force the fill colour to match
the context (i.e. text or link colour), otherwise it doesn't
pass through properly on Windows.

This is an accessibility enhancement as it means that
users who use tools like high-contrast mode to force
changes in background and foreground colours will
continue to be able to see iconography. Without it, icons
are invisible if they're the same colour as the forced
background colour.

Fixes #162
@gregtyler gregtyler force-pushed the 89-fix-embedded-svg-colour branch from df8feb7 to 07ce8d9 Compare May 17, 2022 14:16
@gregtyler
Copy link
Contributor Author

gregtyler commented May 17, 2022

Updated to include the icon in the link: I've had to float it to avoid some display issues with SVG+underline+whitespace.

I've also added rules for "forced color mode" which is the CSS terminology for High Contrast. This ensures that embedded SVGs are filled with the colour of their context (i.e. windowText or linkText) because fill colours don't cascade properly in Microsoft Edge. I've tested this in High Contrast mode on Windows using both Firefox and Microsoft Edge.

Demo available here: https://moj-frontend-7lot9zxwh-vercel-gregtylerco.vercel.app/components

Copy link

@euhyung euhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMG_3240
thanks, works in Edge browser as well now

@gregtyler gregtyler merged commit 1f89878 into main May 18, 2022
@gregtyler gregtyler deleted the 89-fix-embedded-svg-colour branch May 18, 2022 09:16
gregtyler pushed a commit that referenced this pull request May 18, 2022
## [1.4.2](v1.4.1...v1.4.2) (2022-05-18)

### Bug Fixes

* markup Banner components as regions ([#359](#359)) ([d38fa96](d38fa96))
* use inline SVGs ([#360](#360)) ([1f89878](1f89878)), closes [#162](#162)
@gregtyler
Copy link
Contributor Author

🎉 This PR is included in version 1.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SVGs don't match colour of parent text
2 participants