Skip to content

all-but-illegible text style needs fix for humans #777

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

Closed
TallTed opened this issue Apr 29, 2021 · 7 comments · Fixed by #785
Closed

all-but-illegible text style needs fix for humans #777

TallTed opened this issue Apr 29, 2021 · 7 comments · Fixed by #785
Assignees
Labels
editorial Purely editorial changes to the specification. errata Erratum for a W3C Recommendation pr exists

Comments

@TallTed
Copy link
Member

TallTed commented Apr 29, 2021

Brought to eye by #775, the <span class="color-text" style="color:#b4a7d6;"> and similar here in the source, and here in the preview (describing figures 7 and 8) is all-but-illegible in my browser (Chrome Version 90.0.4430.93 (Official Build) (x86_64) on macOS El Capitan (10.11.6)). I does't seem likely that it would be much more legible in any other browser on any other platform.

illegible_text

Regrettably, this styling didn't go through W3 CSS, so it probably counts as an errata for W3 purposes.

I am surprised it passed muster for the Accessibility review!

@brentzundel brentzundel added editorial Purely editorial changes to the specification. PossibleErratum WG should determine if this is Errata labels May 10, 2021
@brentzundel
Copy link
Member

We agree this needs fixing.

@kdenhartog kdenhartog added errata Erratum for a W3C Recommendation v1.1 and removed PossibleErratum WG should determine if this is Errata labels Jul 29, 2021
@kdenhartog kdenhartog self-assigned this Jul 29, 2021
@kdenhartog
Copy link
Member

Noting that the text coloing is intended to be aligned with the color coordination of the graph below it. Would we prefer to update this to the links of the definitions and include a paraentheses to indicate color association or drop the color association between the text and figure 8 all together?

@kdenhartog kdenhartog added this to the V1.1 Completion milestone Jul 29, 2021
TallTed added a commit that referenced this issue Jul 29, 2021
tries to partially (diagram changes are also needed) fix #777, by uncoloring and adjusting the text
@TallTed
Copy link
Member Author

TallTed commented Jul 29, 2021

Tangentially, I would split the descriptions of Figure 7 and Figure 8 into separate paragraphs. (Yes, Fig 7 will be short. That's OK.)

I would suggest a dotted-line in each color, surrounding the blocks of that color, extending to the right of the illustration, where the textual label for each collection should probably be in black.

With re-re-reading, I am thinking that the text should also be tweaked a bit, and either instead of or in addition to the "{first, second, third, fourth} graph", those graphs should be identified by the name used to label the current dashed-black-line blobs (i.e., "{Presentation, Credential, Credential Proof, Presentation Proof} Graph"). Without so doing, it can appear that the latter names are out of sync with the names used in the textual description -- which makes this already complex section even more difficult to comprehend. I've taken a whack at this in #785.

@TzviyaSiegman
Copy link
Contributor

I haven't looked at this document in a long time, but I did an accessibility review years ago. If the colors are conveying meaning, the meaning needs to be conveyed by more than just color for those who cannot perceive color. (I can perceive color, and I am unsure what is being conveyed by the color, and I find it hard to read.)

@chaals
Copy link
Contributor

chaals commented Jul 30, 2021

Yeah, I don't think this should have passed accessibility review - relying purely on colours to make the associations is a basic no-no.

IMHO the images themselves would not pass accessibility review (that they are SVG is great, but that they are just a collection of path elements with no structure even for text is really bad).

Fixing this probably means using different styles for the diagram, with a more explicit legend.

I'll do a couple of hours of work to see how much improvement that would make...

@kdenhartog kdenhartog assigned TallTed and chaals and unassigned kdenhartog and TallTed Aug 4, 2021
@kdenhartog kdenhartog removed this from the V1.1 Completion milestone Aug 13, 2021
kdenhartog pushed a commit that referenced this issue Aug 24, 2021
* Add PR review process for 2021 (#774)

* Add PR review process for 2021.

* Avoid GitHub id auto-linking.

Co-authored-by: David I. Lehn <[email protected]>

* Update README.md

Co-authored-by: Manu Sporny <[email protected]>

Co-authored-by: Manu Sporny <[email protected]>
Co-authored-by: David I. Lehn <[email protected]>
Co-authored-by: Brent Zundel <[email protected]>

* Update index.html

tries to partially (diagram changes are also needed) fix #777, by uncoloring and adjusting the text

* Apply suggestions from code review

Cleaned up markdown tags, and added word missed previously

Co-authored-by: wyc <[email protected]>
Co-authored-by: Manu Sporny <[email protected]>
Co-authored-by: David I. Lehn <[email protected]>
Co-authored-by: Brent Zundel <[email protected]>
@kdenhartog
Copy link
Member

#785 and #786 have both now been merged. closing this issue.

@kdenhartog
Copy link
Member

A second part of this issue was discovered after we addressed the first part. The details of the investigation and the PR can be found in issue #837 and PR #841 adding a comment here since they're directly related to follow up fixes that were meant to be addressed in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Purely editorial changes to the specification. errata Erratum for a W3C Recommendation pr exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants