Expand a11y testing, fix inaccessible colors

andyfeller · andyfeller · commit bde34d5cca49 · 2025-03-13T12:38:19.000-04:00
This commit is focused on expanded accessibility testing and addressing discoveries from them:

1. `TestAccessibleDarkStyleConfigIs4Bit` and `TestAccessibleLightStyleConfigIs4Bit` will walk the Glamour StyleConfig type and check all elements
2. `Test_RenderAccessible` has been expanded with anchor and image elements

In addition, there are several fixes to logic and tests:

1. `Test_RenderAccessible` Go codeblock was missing replacement value, resulting in inaccurate test input
2. Dark and light accessible style logic was missing link text, image text, and H6 styling
3. Handling the `StyleBlock` of codeblocks which is unclear how it interacts with Chroma
diff --git a/pkg/markdown/markdown_test.go b/pkg/markdown/markdown_test.go
@@ -28,6 +28,14 @@ const (
 // It works by parsing the rendered markdown for ANSI escape sequences and checking their display attributes.
 // Test scenarios allow multiple color mode / depths because `ansi.Parse()` considers `\x1b[0m` sequence as part of `ansi.Default`.
 func Test_RenderAccessible(t *testing.T) {
+	anchor := heredoc.Doc(`
+		[GitHub CLI repository](https://github.com/cli/cli)
+	`)
+
+	img := heredoc.Doc(`
+		[Animated Mona for loading screen](https://github.com/user-attachments/assets/a43e7ce6-8360-466c-b2d5-0c74a60c30a4)
+	`)
+
 	goCodeBlock := heredoc.Docf(`
 		%[1]s%[1]s%[1]sgo
 		package main
@@ -59,7 +67,7 @@ func Test_RenderAccessible(t *testing.T) {
 			}
 		'
 		%[1]s%[1]s%[1]s
-	`)
+	`, "`")
 
 	tests := []struct {
 		name              string
@@ -69,6 +77,7 @@ func Test_RenderAccessible(t *testing.T) {
 		wantColourModes   []ansi.ColourMode
 		allowDimFaintText bool
 	}{
+		// Go block
 		{
 			name:              "when the light theme is selected, the Go codeblock renders using 8-bit colors",
 			text:              goCodeBlock,
@@ -99,6 +108,7 @@ func Test_RenderAccessible(t *testing.T) {
 			wantColourModes:   []ansi.ColourMode{ansi.Default},
 			allowDimFaintText: false,
 		},
+		// shell block
 		{
 			name:              "when the light theme is selected, the Shell codeblock renders using 8-bit colors",
 			text:              shellCodeBlock,
@@ -129,6 +139,68 @@ func Test_RenderAccessible(t *testing.T) {
 			wantColourModes:   []ansi.ColourMode{ansi.Default},
 			allowDimFaintText: false,
 		},
+		// image text and link
+		{
+			name:              "when the light theme is selected, the image text and link rendered using 8-bit colors",
+			text:              img,
+			theme:             "light",
+			wantColourModes:   []ansi.ColourMode{ansi.Default, ansi.TwoFiveSix},
+			allowDimFaintText: true,
+		},
+		{
+			name:              "when the dark theme is selected, the image text and link rendered using 8-bit colors",
+			text:              img,
+			theme:             "dark",
+			wantColourModes:   []ansi.ColourMode{ansi.Default, ansi.TwoFiveSix},
+			allowDimFaintText: true,
+		},
+		{
+			name:              "when the accessible env var is set and the light theme is selected, the image text and link render using 4-bit colors without dim/faint text",
+			text:              img,
+			theme:             "light",
+			accessible:        true,
+			wantColourModes:   []ansi.ColourMode{ansi.Default},
+			allowDimFaintText: false,
+		},
+		{
+			name:              "when the accessible env var is set and the dark theme is selected, the image text and link render using 4-bit colors without dim/faint text",
+			text:              img,
+			theme:             "dark",
+			accessible:        true,
+			wantColourModes:   []ansi.ColourMode{ansi.Default},
+			allowDimFaintText: false,
+		},
+		// anchor text and link
+		{
+			name:              "when the light theme is selected, the anchor text and link rendered using 8-bit colors",
+			text:              anchor,
+			theme:             "light",
+			wantColourModes:   []ansi.ColourMode{ansi.Default, ansi.TwoFiveSix},
+			allowDimFaintText: true,
+		},
+		{
+			name:              "when the dark theme is selected, the anchor text and link rendered using 8-bit colors",
+			text:              anchor,
+			theme:             "dark",
+			wantColourModes:   []ansi.ColourMode{ansi.Default, ansi.TwoFiveSix},
+			allowDimFaintText: true,
+		},
+		{
+			name:              "when the accessible env var is set and the light theme is selected, the anchor text and link render using 4-bit colors without dim/faint text",
+			text:              anchor,
+			theme:             "light",
+			accessible:        true,
+			wantColourModes:   []ansi.ColourMode{ansi.Default},
+			allowDimFaintText: false,
+		},
+		{
+			name:              "when the accessible env var is set and the dark theme is selected, the anchor text and link render using 4-bit colors without dim/faint text",
+			text:              anchor,
+			theme:             "dark",
+			accessible:        true,
+			wantColourModes:   []ansi.ColourMode{ansi.Default},
+			allowDimFaintText: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/x/markdown/accessibility.go b/pkg/x/markdown/accessibility.go
@@ -1,6 +1,7 @@
 package markdown
 
 import (
+	"fmt"
 	"strconv"
 
 	"github.com/charmbracelet/glamour/ansi"
@@ -31,11 +32,50 @@ const (
 	brightWhite
 )
 
-func (a glamourStyleColor) code() *string {
-	s := strconv.Itoa(int(a))
+func (gsc glamourStyleColor) code() *string {
+	s := strconv.Itoa(int(gsc))
 	return &s
 }
 
+func parseGlamourStyleColor(code string) (glamourStyleColor, error) {
+	switch code {
+	case "0":
+		return black, nil
+	case "1":
+		return red, nil
+	case "2":
+		return green, nil
+	case "3":
+		return yellow, nil
+	case "4":
+		return blue, nil
+	case "5":
+		return magenta, nil
+	case "6":
+		return cyan, nil
+	case "7":
+		return white, nil
+	case "8":
+		return brightBlack, nil
+	case "9":
+		return brightRed, nil
+	case "10":
+		return brightGreen, nil
+	case "11":
+		return brightYellow, nil
+	case "12":
+		return brightBlue, nil
+	case "13":
+		return brightMagenta, nil
+	case "14":
+		return brightCyan, nil
+	case "15":
+		return brightWhite, nil
+	default:
+		return 0, fmt.Errorf("invalid color code: %s", code)
+	}
+}
+
 func AccessibleStyleConfig(theme string) ansi.StyleConfig {
 	switch theme {
 	case "light":
@@ -55,22 +95,28 @@ func accessibleDarkStyleConfig() ansi.StyleConfig {
 
 	// Link colors
 	cfg.Link.Color = brightCyan.code()
+	cfg.LinkText.Color = brightCyan.code()
 
 	// Heading colors
 	cfg.Heading.StylePrimitive.Color = brightMagenta.code()
 	cfg.H1.StylePrimitive.Color = brightWhite.code()
 	cfg.H1.StylePrimitive.BackgroundColor = brightBlue.code()
+	cfg.H6.StylePrimitive.Color = brightMagenta.code()
 
 	// Code colors
 	cfg.Code.BackgroundColor = brightWhite.code()
 	cfg.Code.Color = red.code()
 
 	// Image colors
 	cfg.Image.Color = brightMagenta.code()
+	cfg.ImageText.Color = brightMagenta.code()
 
 	// Horizontal rule colors
 	cfg.HorizontalRule.Color = white.code()
 
+	// Code block colors
+	cfg.CodeBlock.StyleBlock.StylePrimitive.Color = nil
+
 	return cfg
 }
 
@@ -82,6 +128,7 @@ func accessibleLightStyleConfig() ansi.StyleConfig {
 
 	// Link colors
 	cfg.Link.Color = brightBlue.code()
+	cfg.LinkText.Color = brightBlue.code()
 
 	// Heading colors
 	cfg.Heading.StylePrimitive.Color = magenta.code()
@@ -94,9 +141,13 @@ func accessibleLightStyleConfig() ansi.StyleConfig {
 
 	// Image colors
 	cfg.Image.Color = magenta.code()
+	cfg.ImageText.Color = magenta.code()
 
 	// Horizontal rule colors
 	cfg.HorizontalRule.Color = white.code()
 
+	// Code block colors
+	cfg.CodeBlock.StyleBlock.StylePrimitive.Color = nil
+
 	return cfg
 }
diff --git a/pkg/x/markdown/accessibility_test.go b/pkg/x/markdown/accessibility_test.go
@@ -1,6 +1,7 @@
 package markdown
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/charmbracelet/glamour/ansi"
@@ -132,7 +133,7 @@ func TestAccessibleStyleConfig(t *testing.T) {
 	}
 }
 
-func Test_accessibleDarkStyleConfig(t *testing.T) {
+func TestAccessibleDarkStyleConfig(t *testing.T) {
 	cfg := accessibleDarkStyleConfig()
 	assert.Equal(t, white.code(), cfg.Document.StylePrimitive.Color)
 	assert.Equal(t, brightCyan.code(), cfg.Link.Color)
@@ -148,7 +149,12 @@ func Test_accessibleDarkStyleConfig(t *testing.T) {
 	assert.Equal(t, styles.DarkStyleConfig.H2, cfg.H2)
 }
 
-func Test_accessibleLightStyleConfig(t *testing.T) {
+func TestAccessibleDarkStyleConfigIs4Bit(t *testing.T) {
+	cfg := accessibleDarkStyleConfig()
+	validateColors(t, reflect.ValueOf(cfg), "StyleConfig")
+}
+
+func TestAccessibleLightStyleConfig(t *testing.T) {
 	cfg := accessibleLightStyleConfig()
 	assert.Equal(t, black.code(), cfg.Document.StylePrimitive.Color)
 	assert.Equal(t, brightBlue.code(), cfg.Link.Color)
@@ -163,3 +169,56 @@ func Test_accessibleLightStyleConfig(t *testing.T) {
 	// Test that we haven't changed the original style
 	assert.Equal(t, styles.LightStyleConfig.H2, cfg.H2)
 }
+
+func TestAccessibleLightStyleConfigIs4Bit(t *testing.T) {
+	cfg := accessibleLightStyleConfig()
+	validateColors(t, reflect.ValueOf(cfg), "StyleConfig")
+}
+
+// Walk every field in the StyleConfig struct, checking that the Color and
+// BackgroundColor fields are valid 4-bit colors.
+//
+// This test skips Chroma fields because their Color fields are RGB hex values
+// that are downsampled to 4-bit colors unlike Glamour, which are 8-bit colors.
+// For more information, https://github.com/alecthomas/chroma/blob/0bf0e9f9ae2a81d463afe769cce01ff821bee3ba/formatters/tty_indexed.go#L32-L44
+func validateColors(t *testing.T, v reflect.Value, path string) {
+	if v.Kind() == reflect.Ptr {
+		if v.IsNil() {
+			return
+		}
+		v = v.Elem()
+	}
+
+	switch v.Kind() {
+	case reflect.Struct:
+		for i := range v.NumField() {
+			field := v.Field(i)
+			fieldType := v.Type().Field(i)
+
+			// Construct path for better error reporting
+			fieldPath := path + "." + fieldType.Name
+
+			// Ensure we only check Glamour "Color" and "BackgroundColor"
+			if fieldType.Name == "Chroma" {
+				continue
+			} else if (fieldType.Name == "Color" || fieldType.Name == "BackgroundColor") &&
+				fieldType.Type.Kind() == reflect.Ptr && fieldType.Type.Elem().Kind() == reflect.String {
+
+				if field.IsNil() {
+					continue
+				}
+				color := field.Elem().String()
+				_, err := parseGlamourStyleColor(color)
+				assert.NoError(t, err, "Failed to parse color '%s' in %s", color, fieldPath)
+			} else {
+				// Recurse into nested structs
+				validateColors(t, field, fieldPath)
+			}
+		}
+	case reflect.Slice:
+		// Handle slices of structs
+		for i := range v.Len() {
+			validateColors(t, v.Index(i), path+"[]")
+		}
+	}
+}
