fix: a11y low contrast in dark mode for code [AC-4348]

[Stefan3002]

Done

• Increased the contrast for the code element in dark mode.
• Screenshots show it was too low.
• The contrast is 5.13 now.

QA

• Open demo
• [Add QA steps]
• Review updated documentation:
  • [List any updated documentation for review]
    Look at a confirmation modal that says: Next time press SHIFT, the contrast of the SHIFT text should be increased and pass a11y guidelines

Check if PR is ready for release

If this PR contains Vanilla SCSS or macro code changes, it should contain the following changes to make sure it's ready for the release:

• PR should have one of the following labels to automatically categorise it in release notes:
  • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
• Vanilla version in package.json should be updated relative to the most recent release, following semver convention
  • if existing APIs (CSS classes & macro APIs) are not changed it can be a bugfix release (x.x.X)
  • if existing APIs (CSS classes & macro APIs) are changed/added/removed it should be a minor version (x.X.0)
  • see the wiki for more details
• Any changes to component class names (new patterns, variants, removed or added features) or macros should be listed on the what's new page.

Fixes: AC-4348

Screenshots

[webteam-app]

Demo

Jenkins

demos.haus

[jmuzina]

While indeed the code text now has greater contrast against its own background, this also decreases the contrast between code and non-code text - it seems quite a bit harder to distinguish them.

Before

After

@juanruitina does this new background color look OK to you?

[juanruitina]

It is more important to be able to read the text than to tell the semantics apart (and in this case, semantics are even reinforced by the monospace font). The contrast lower than 4.5 is not acceptable.

That said, @Stefan3002 it seems the case you are sharing in your screenshot is a corner case caused by the use of muted text. rgba($color-x-light, 0.15) would be sufficient to make the contrast greater than 4.5, see below. We could apply this to code tags only inside muted text, or for all code for the sake of simplicity and increased text contrast in all cases. Both options would be compliant.

Interestingly, this is not an issue in the light theme:

[Stefan3002]

Indeed, the problem is only present in dark mode 😄

[Stefan3002]

Hey all, sorry for the huge delay. I was in medical leave unfortunately.

Could I get another review on this please? 😄

[jmuzina]

No worries, apologies for the delay also, I was on sick leave!

I'll give it a look tomorrow :)

[jmuzina]

4.47.0 hasn't been released yet - so doing this would actually skip 4.47.0 in the release order.

I'd recommend to keep this as 4.47.0

Suggested change

	"version": "4.47.1",
	"version": "4.47.0",

[jmuzina]

This should be applied inside %muted-text placeholder, not the muted text class selector, so it applies to elements that extend %muted-text but don't have u-text--muted class (currently only notification timestamps).

[jmuzina]

$color-x-light is not color-theme sensitive. So, If you test this on light theme, the code background becomes indistinguishable from the background.

To solve this we'd need an opinion from design on what the right background color is for code inside muted text on light mode. But, @juanruitina mentioned this is fine, so I'm OK to leave this mostly as-is, but we should add a comment here explaining why we are using a non-themable variable here.

[juanruitina]

Little misunderstanding. The quick fix I suggest is to apply the subtlier background of rgba(255, 255, 255, 0.15) to code blocks in the dark theme, regardless of muted or not. I think the proposed code instead targets muted code regardless of theme, resulting in a regression in the light theme.

For the record, I think I was looking at the live version, where there is such a distinction because of the use of var(--vf-color-background-code), rgba(0, 0, 0, 0.03 and should remain unchanged:

This is what it looks like in the demo (which is a regression):

[jmuzina]

This PR produces no visual diffs because Vanilla has no examples that use code inside of muted text (I don't think Vanilla was aware of this as a use case). Could you add an example for Code that shows it inside an element with muted text? That way we can capture visual regressions of this use case in future.

It doesn't need to be shown on the docs page - just adding it to examples/ and the combined.html example for Code will cause it to be visually tested.

[jmuzina]

Explicitly targeting element tags causes a stylelint error (we try not to apply styles to tags unless we absolutely have to).

I think this is a valid use case for a tag selector though. To ignore the error and pass CI, see one of these examples to temporarily disable (then re-enable) this rule.