-
Notifications
You must be signed in to change notification settings - Fork 317
Replace raster images with SVG in Understanding SC 1.4.11: Non-Text Contrast #4573
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
base: main
Are you sure you want to change the base?
Replace raster images with SVG in Understanding SC 1.4.11: Non-Text Contrast #4573
Conversation
✅ Deploy Preview for wcag2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
alternatively...just use regular |
#4575 fixes the title issue. You should be able to shorten the includes from
This one has some strings attached, but tl;dr I am inclined to say "no" for reasons explained below. Currently, the build does not copy static assets in further subdirectories under I could fix the build to also copy subdirectories, but it is still going to fully flatten, so it will still be a good idea to have a distinguishable prefix on filenames. (I see that some of the SVG files in the PR start with Because you are inlining the SVGs, this isn't really a problem for this PR, but it would be something to keep in mind in general for files under
Discussed on backlog call on August 22; definitely +1 to avoiding the bloat that paths would add, but some checking will need to be done to ensure text remains within expected boundaries, etc.
|
Wow, thanks so much for #4575 and your incredibly speedy review, @kfranqueiro. I’ll put the new I’ll also do some more benchtesting to make sure the Onward and upward! |
Looks good to me. Thanks for the work @adampage |
Thanks, @iadawn! I re-illustrated the source artwork in Figma and wrote a custom Figma plugin to sanitize the exported SVG code and to structure each image’s alt text inside a |
@kfranqueiro to take a look and move to Ready for Approval if fine. |
I reviewed and see no problems with the SVG replacements themselves. However, I'm not sure how I feel about deferring the reflow problem to a separate issue, because I notice that this PR makes it significantly worse. Before, horizontal scrolling would be incurred around 412px; on this branch, it is incurred around 632px, which I would expect to affect more viewports. I presume this is because the Is there a reasonable way to avoid making the reflow problem worse in this PR? |
Closes #4549.
Replaces many of the raster images in Understanding Non-Text Contrast with inlined SVG assets. This gives the example visuals higher fidelity in varying viewport sizes, user zoom levels, and high-density displays.
Please compare before and after.
320px
viewports because of the non-responsive tables like “Passing User Interface Component Examples”. I’ll create a separate issue & PR to see about resolving that. #4641