-
Notifications
You must be signed in to change notification settings - Fork 250
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
Update text alternative for target size (minimum) understanding illustrations #3190
Update text alternative for target size (minimum) understanding illustrations #3190
Conversation
patrickhlauke
commented
May 15, 2023
- corrects the woefully incomplete alt for the target-spacing-toolbar example
- expands the alt for a few of the following illustrations
…trations * corrects the woefully incomplete alt for the target-spacing-toolbar example * expands the alt for a few of the following illustrations
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.
The alt
value seems overly long, IMO. I'd say it would be better to keep the alt
content itself short and referring to the figcaption
text directly underneath.
@detlevhfischer assuming you just mean that first one, i'm easy either adding it to the alt, or to the figcaption. but currently neither of them mention specifically what's going on there. i'll see if i can rejig it... |
I'd treat all of them the same way: short |
i've generally tried to keep the idea along the lines of: alt describes what the image is, figcaption describes what the image represents/tries to show... but as said, i'll mull over how to possibly rejig this. |
Co-authored-by: Mike Gower <mikegower@gmail.com>
Co-authored-by: Mike Gower <mikegower@gmail.com>
756a1d2
to
00d112a
Compare
ok, tweaked these a little bit. thoughts? (keeping in mind that currently some of the alts/descriptions are plain wrong, so we may want to prioritise correcting them now and refining this...at some point in the future) |
|
||
<figure id="target-spacing-toolbar"> | ||
<img src="img/target-spacing-toolbar.svg" width="560" height="265" alt="Three rows showing a horizontal toolbar of icon-based controls with measurements. In the first row, the targets have a dimension of 24 by 24 CSS pixels, so pass. In the second row, the same toolbar is shown again, but overlaid with the 24 CSS pixel circles – the circles don't intersect, so pass. In the last row, the toolbar is shown again with the circles, but the circles intersect, so fail." /> | ||
<img src="img/target-spacing-toolbar.svg" width="560" height="265" alt="The first toolbar's targets have a dimension of 24 by 24 CSS pixels, so pass. The second toolbar, with smaller targets, shows 24 CSS pixel circles drawn on each target for assessment. The circles do not intersect and so the targets have enough space to pass. The third toolbar shows similar circles on the targets, but the circles intersect due to the lack of spacing between targets, so the toolbar fails." /> |
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.
Would it make sense to have the verdict PASS or FAIL at a consistent place inside figcaption
? On the image id="target-text-buttons-two-rows"
the figcaption
text starts with <strong>Fail:</strong>
but not on the others. Those images that pass might start with <strong>Pass:</strong>
.
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.
As that one with the FAIL was the only one that had it, and most other examples have a mix of both pass and fail, it might make more sense to remove that one <strong>Fail:</strong>
@patrickhlauke I see the point about the illustrations where both pass and fail occur so leaving it out in the |
@detlevhfischer for all other points not directly related to the core "tweak the text alternatives/explanations" of this PR, I'd suggest opening a new issue/PR for them (keeping this one small and more easily mergeable, so that the inaccurate/incomplete alt we currently have can get quickly fixed first before we go down longer rabbithole discussions on the other aspects) |
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.
Nice changes, thanks!