-
Couldn't load subscription status.
- Fork 322
Target Size techniques update #4685
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?
Conversation
Revisiting [PR 3231](#3231), which was left in a “this could still be useful but has loads of merge conflicts” state.
so it’s easier to find
- multiple issues commenting that the 44px references relate to the Enhanced example so should be removed or changed. This update removes references to the AAA 44px requirement. - remove the first example - we don’t need two really similar examples for this. - remove the “search navigation” in the image (and working example) as it wasn’t relevant. - remove some `xml:space` attributes. - remove cruft: editors css link, metadata section
✅ Deploy Preview for wcag2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@fstrr is there an actual change to the |
techniques/css/technique-using-css-to-ensure-targets-are-at-least-44-by-44-css-pixels.html
Outdated
Show resolved
Hide resolved
techniques/css/technique-using-css-to-ensure-targets-are-at-least-44-by-44-css-pixels.html
Outdated
Show resolved
Hide resolved
techniques/css/technique-using-css-to-ensure-targets-are-at-least-44-by-44-css-pixels.html
Outdated
Show resolved
Hide resolved
These were reversed - test for 44px for C42, test for undersized for the new 44px one Also, the "hasn't had its size changed" was wrong. it should be "HAS had its size changed"
…ast-44-by-44-css-pixels.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few code suggestions to make the title for C42 and its working example more explicit.
Some wording and the test/expected results were the wrong way around for C42 and the new technique, and some of the wording was off. I was bold enough to push a fix to that directly here.
Apart from that (and my suggestion for more explicit title, and possibly making C42 example even more "just barely passes"), this looks good to me
Co-authored-by: Patrick H. Lauke <[email protected]>
Co-authored-by: Patrick H. Lauke <[email protected]>
Co-authored-by: Patrick H. Lauke <[email protected]>
Co-authored-by: Patrick H. Lauke <[email protected]>
Co-authored-by: Patrick H. Lauke <[email protected]>
Co-authored-by: Patrick H. Lauke <[email protected]>
|
@patrickhlauke Thanks for those edits. I didn't touch base.css, so I'm unsure about your comment about that one. It I look at my commit history locally, I don't see a reference to it, which would make sense as I didn't open it.
|
Co-authored-by: Dan Bjorge <[email protected]>
Co-authored-by: Dan Bjorge <[email protected]>
Co-authored-by: Dan Bjorge <[email protected]>
closes #2433
closes #3447
closes #4227
xref #2434
xref #4274
I stumbled across #3231, which got close to the finish line and then got forgotten about. It seems that the issues it was aiming to close are still open and there was an appetite to clean up C42 and create a new technique for Target Size (Enhanced), so I created a new branch and copied the old code into that, which seemed easier than dealing with two years of merge conflicts.
This PR:
@kfranqueiro When things get approved, all the various links will need updating. I tried to work out how everything works now so I could prep things with placeholder links, but where there was once simple HTML, there's now JSON and XML that should make sense, but doesn't (at the moment).
Preview links: