highlightjs / highlight.js

JavaScript syntax highlighter with language auto-detection and zero dependencies.
https://highlightjs.org/
BSD 3-Clause "New" or "Revised" License
23.73k stars 3.6k forks source link

(themes) "faded" comments should be optional, not default. #3483

Open joshgoebel opened 2 years ago

joshgoebel commented 2 years ago

I think perhaps for accessibility that a pass over ALL the comment styles in all (1st party, not Base16) themes and pulling them up to a 3 or 4.5 contrast and then recommend that those who want the common "i can barely see the comments" look/feel should apply opacity of their desired choosing to hljs-comment. That way we equally support both accessibility and user choice.

I'm going to split this into a separate issue and add it to the v12 list.

Update: Definitely seems like we should first update the theme checker tool to use APCA.

Originally posted by @joshgoebel in https://github.com/highlightjs/highlight.js/issues/3482#issuecomment-1040565915

allejo commented 2 years ago

I feel like it'd be appropriate to have some discussion/thoughts regarding how we define an "accessible theme." WCAG is flawed in some cases and there's a new player in town: APCA (it even comes with its own npm package we can use!). So I'd want to explore these options before we make a decision.

stealing my own comment from here

Myndex commented 2 years ago

Hi @allejo @joshgoebel

This year marks the third anniversary of the research and development project that underlies APCA (and it is significantly more extensive than just the APCA tool). I wanted to drop a note as to status, and resources.

Quick Status Update

The Basic APCA math/algorithm is stable, and has not changed in over a year. This base algorithm is probably not going to change, however, there are more advanced versions that are separate and may be introduced due to the development of new display technologies, but that is not relevant to the Basic sRGB version that is donated to the W3C for use in guidelines.

I just published APCA W3 0.1.2, which includes some key new features, such as an integrated dynamic font lookup table that automatically indicate the appropriate font/weight, alpha blending, displayP3 support, and a "reverseAPCA" where you can specify a contrast value, and either the bg or text color, and it generates the missing color.

It is on npm, npm i apca-w3

It has one dependency, which I've also placed on npm: npm i colorparsley

Also, Bridge-PCA is a drop-in replacement for WCAG 2's math. See that tool at https://www.myndex.com/BPCA/

Bridge-PCA is backwards compatible with WCAG 2, meaning it meets or exceeds WCAG 2 for compliance purposes, (including the areas where WCAG 2 rejects colors incorrectly). That said, Bridge-PCA is less flexible than the basic APCA guidelines.

Please let me know of any questions. The main repo discussion area is open with active discussions on APCA and related technologies: https://github.com/Myndex/SAPC-APCA/discussions

Also, there is this Linktree of all the main resources listed together: https://linktr.ee/Myndex

Thank you!

Andy

joshgoebel commented 2 years ago

@Myndex Thanks so much. Just at a glance it seems we should add the APCA "score" to our theme checking tool and perhaps prefer that since it seems it's on the table for being in WCAG 3, yes?

I think a PR to do that would definitely be welcome.

"reverseAPCA" where you can specify a contrast value, and either the bg or text color, and it generates the missing color.

How would it do this? Can we specify a hue or the "exact color we'd prefer" and then it adjusts the contrast? What we have here in the themes are the BG and FG colors, just one (or both) may need to be tweaked to improve the contrast without changing the visible hues.

joshgoebel commented 2 years ago

@Myndex A key part of APCA seems to be font weight and and size (key deficiencies of the prior standard for sure)... but sadly these are not something we know when designing themes (I'd wager often people drop in their own font choices, weights, sizes)... should we just pick a lower threshold number for the font (say 12-14px) and a middling weight value (400) and then just use the final contrast value as a rough guideline?

joshgoebel commented 2 years ago

Sadly it seems we currently can't use your packages because they aren't setup properly for use from nodejs... your distributable seems to be intended for client-side usage (where all the dependencies are global), but your actual source code doesn't import the necessary dependencies. So requiring it via nodes breaks because colorParsley is not defined.

ReferenceError: colorParsley is not defined

Update: I was able to make it work by hacking the globals: global.colorParsley = colorParsley

Update: Opened issue https://github.com/Myndex/apca-w3/pull/3

joshgoebel commented 2 years ago

Though I admit I'm a bit confused what perhaps our "goal" should be given "Lc 90 is preferred for body text". Our default theme currently:

Accessibility Report
│ ratio │ apca    │ selector                               │ fg       │ bg       │
│ 8.78  │ 85.46   │ .hljs                                  │ #444     │ #F3F3F3  │
│ 4.56  │ 67.81   │ .hljs-comment                          │ #697070  │ #F3F3F3  │

So both are greater than the 4.5 specified but the WCAG v2, yet not even the default body text reaches 90 Lc and the comments are at a very middling 67 Lc.


Also confusing that alpha seems to have no effect on the score?

│ 8.78  │ 85.46   │ .hljs-tag,.hljs-punctuation            │ #444a    │ #F3F3F3  │
│ 8.78  │ 85.46   │ .hljs-tag .hljs-name,.hljs-tag .hljs-… │ #444     │ #F3F3F3

This is just using calcAPCA with the pairs:

colorParsley error: unable to parse string
'#F3F3F3' [ 0, 0, 0, '', false, 'parsleyError' ]
'#444a' [ 68, 68, 68, 0.6666666666666666, true, 'sRGB' ]
colorParsley error: unable to parse string
'#F3F3F3' [ 0, 0, 0, '', false, 'parsleyError' ]
'#444' [ 68, 68, 68, 1, true, 'sRGB' ]

(I'm temporarily using another function to parse the #f3f3f3 until we figure out why parsley doesn't like it...

I just realized the function I'm using short term only returns a 4 pair tuple, not the last two... so perhaps that's the issue there... nope, doesn't seem to be the issue since I updated it to return a 6 item tuple.

joshgoebel commented 2 years ago

Work so far: https://github.com/highlightjs/highlight.js/pull/3525

Myndex commented 2 years ago

Hi Josh @joshgoebel

I commented on the PR, and thank you for that and these several comments. Just FYI, we do have a discussions area at the main repo: https://github.com/Myndex/SAPC-APCA/discussions Which also contains additional documentation-in-development.

Replies for this thread

Just at a glance it seems we should add the APCA "score" to our theme checking tool and perhaps prefer that since it seems it's on the table for being in WCAG 3, yes?

Just to be clear, WCAG 3 is not yet "official" and WCAG 2 will continue to be used (independently) even after WCAG 3 is a recommendation. To that end, even WCAG 1 is still in use in some places, and was only deprecated last year. (!!)

APCA is not backwards compatible to WCAG 2, but BridgePCA is. DEMO: https://www.myndex.com/BPCA/

That said, APCA is a substantial improvement over WCAG 2 for actual accessibility.

A key part of APCA seems to be font weight and and size (key deficiencies of the prior standard for sure)... but sadly these are not something we know when designing themes (I'd wager often people drop in their own font choices, weights, sizes)... should we just pick a lower threshold number for the font (say 12-14px) and a middling weight value (400) and then just use the final contrast value as a rough guideline?

Font weight and size are more important to contrast than the color pair.

Wait Wut?

Spatial frequency is the primary driver of human perception of contrast. Abstracted to design, Spatial frequency relates to font weight, size, spacing, etc. As such, consider the likely use case for a set of colors, and consider the needed contrast therein. Blocks of bodytext needs the most luminance contrast.

For an outline of use cases, see also: https://github.com/Myndex/SAPC-APCA/discussions/39

Alpha

Also confusing that alpha seems to have no effect on the score?

Only the text (the first parameter) has alpha calculated, against the background which must be solid (no alpha).

What can not be done is alpha for a background unless it is affirmatively known what that background is going to be over. If the BG is to be transparent, there is no way to know what the contrast is, without knowing what the BG is over.

On the otherhand, text transparency can have a known contrast if the text is over a solid background.

colorParsley bug

colorParsley error: unable to parse string
'#F3F3F3' [ 0, 0, 0, '', false, 'parsleyError' ]

Weird, thank you looking into it now. colorParsley() should make everything lowercase, I'm wondering if that's it. Will advise

Thank you!!

joshgoebel commented 2 years ago

On the otherhand, text transparency can have a known contrast if the text is over a solid background.

That is the case here.

│ ratio │ apca    │ selector                               │ fg       │ bg       │
│ 8.78  │ 85.46   │ .hljs-tag,.hljs-punctuation            │ #444a    │ #F3F3F3  │

Right, in this case the background is opaque. So I'd expect to see a difference between fg #444 and fg #444a, but I do not.

joshgoebel commented 2 years ago

That said, APCA is a substantial improvement over WCAG 2 for actual accessibility.

That's what we care about. We have no mandate to support any given accessibility standard - that would be up to the actual implementors of the library to decide... so whatever tooling/standard actually provides the most actual accessibility is what we would want to use internally. And one would hope WCAG 3 is going to be an improvement on WCAG 2 so skating to where the puck is heading seems a reasonable goal.