fix(color-contrast): get text stoke from offset shadows

[WilcoFiers]

If several text shadows with some offset and with minimal blur are used together, they can combine into a text stroke effect. This PR attempts to identify such cases and combine these shadows into a single color.

Closes: #4064

[dbjorge]

This test case assumes that 2 shadows on opposite corners would be recognized correctly as a normally-combined case; it'd be good to have a test case verifying that explicitly ahead of this one

[dbjorge]

Suggested change

	function getStrokeColor(thinShadows) {
	function getStrokeColors(thinShadows) {

[dbjorge]

Would be good to have a test that verifies that exactly 3 sides is sufficient to be not-skipped

[dbjorge]

Thanks Wilco! Some suggestions inline

[WilcoFiers]

TODO

[WilcoFiers]

TODO: figure out calling incomplete when shadows are not on all sides.

This should be good to go now

[dbjorge]

Suggested change

	* @returns {Array[]} Array of colors
	* @returns {Array|null} Array of colors or null if text-shadow was too complex to measure

[dbjorge]

Suggested change

	// Skip shadows that cover too much of the text to be ignored, but not enough to be tested
	// Bail immediately if any shadow group covers too much of the text to be ignored, but not enough to be tested

[dbjorge]

I think this is a bit off; if one shadow impacts 2 edges, but is only > OPAQUE_STROKE_OFFSET_MIN_PX on one of those edges, this will currently treat both edges as solid instead of just one like it should. Here is a test case that demonstrates this (the same as the current applies an alpha value if not all shadows are offset by more than 1.5px test, except with the 4 shadows collapsed into two corner shadows):

  it('applies an alpha value if not all sides are offset by more than 1.5px', () => {
    const shadows = parseTextShadows(`
      -1.5px -2px #000,
      -2px 1.5px #000
    `);
    const shadowColors = getStrokeColorsFromShadows(shadows);
    assert.lengthOf(shadowColors, 1);
    assert.equal(shadowColors[0].red, 0);
    assert.equal(shadowColors[0].green, 0);
    assert.equal(shadowColors[0].blue, 0);
    assert.closeTo(shadowColors[0].alpha, 0.46, 0.01);
  });

To fix this, I recommend updating getShadowColorsMap to produce a mapping from edge to list-of-thickness, instead of from edge to list-of-pixels-tuples (ie, replace all the borders.EDGE.push(pixels) statements with borders.EDGE.push(/* offsetX or offsetY as appropriate */)), and then update this check to be:

Suggested change

	isSolid &&= sides[edge].every(([x, y]) => {
	return (
	Math.abs(x) > OPAQUE_STROKE_OFFSET_MIN_PX ||
	Math.abs(y) > OPAQUE_STROKE_OFFSET_MIN_PX
	);
	});
	isSolid &&= sides[edge].every(
	offset => Math.abs(offset) > OPAQUE_STROKE_OFFSET_MIN_PX
	);

[dbjorge]

Suggested change

	// Average the number of shadows around the text
	// Detect whether any sides' shadows are thin enough to be considered
	// translucent, and if so, calculate an alpha value to apply on top of
	// the parsed color.

[dbjorge]

It feels off that this check is still here when the caller is now also doing an overlapping edgeCount check. Maybe more clear to move this check out of shadowGroupToColor and update the .filter on L32 to filter out based on edgeCount before mapping this function, instead of mapping this function and then filtering based on its return value.

[dbjorge]

This is the only test case that's covering shadow-on-corners behavior; it'd be good to have a case that verifies that "2 opposing corner shadows is enough to not return empty/null"

[dbjorge]

For the ?? [] to come up in color-contrast and not have already resulted in a complexTextShadows exit in color-contrast-evaluate, an element would need to have a "complex" (2-3 edge compound) shadow of blur radius between .001em and 0.2em. I think if that were to actually happen in practice and there was additionally a higher-blur-radius shadow(s), it probably doesn't make sense to silently throw away those higher-blur-radius shadow(s). I can be persuaded that either of "bail as incomplete" or "ignore those cases but still consider the thick shadow behind as a possible background color" would be reasonable.

link-in-text-block also uses getBackgroundColor and I think the ?? [] is even sketchier there, since it doesn't early-exit with incomplete for blur radius < .001 complex compound shadows like color-contrast does. Generally, it seems a bit strange to me that link-in-text-block-evaluate isn't reusing the same support for shadows providing contrast that color contrast is.

[WilcoFiers]

I can't sneak anything past you can I? I realised that too when I wrote this. Wasn't sure whether I should care, but fair enough.

Sorted

[dbjorge]