Merge pull request #12 from refactorfirst/fix-popup-close-button

Improve Contrast and focus ring on popup close button

Author: jimbethancourt

app/globals.css

@@ -36,6 +36,10 @@
   --btn-disabled-bg: #f3f4f6;
   --btn-disabled-text: #6b7280;
   --popup-border: #cccccc;
+   /* Close-button red: white × on it passes AA (>= 4.5:1) and the square
+     clears 3:1 against the white popup surface (SC 1.4.11). */
+  --popup-close-bg: #c0392b;
+  --popup-close-fg: #ffffff;
   --toast-bg: #1f2933;
   --toast-text: #ffffff;
   --menu-max-height: 140px;
@@ -95,6 +99,10 @@ html:root {
   --btn-disabled-bg: #232a35;
   --btn-disabled-text: #8a97a8;
   --popup-border: #3a4b61;
+  /* Brighter red for the dark surface: keeps the white × at AA (4.6:1)
+     and the square at 3.4:1 against --card-bg (SC 1.4.11). */
+  --popup-close-bg: #cf4436;
+  --popup-close-fg: #ffffff;
   --toast-bg: #1f2933;
   --toast-text: #ffffff;
   --color-bg: var(--bg-color);
@@ -132,6 +140,10 @@ html:root {
     --btn-disabled-bg: #232a35;
     --btn-disabled-text: #8a97a8;
     --popup-border: #3a4b61;
+    /* Brighter red for the dark surface: keeps the white × at AA (4.6:1)
+       and the square at 3.4:1 against --card-bg (SC 1.4.11). */
+    --popup-close-bg: #cf4436;
+    --popup-close-fg: #ffffff;
     --toast-bg: #1f2933;
     --toast-text: #ffffff;
     --color-bg: var(--bg-color);
@@ -774,6 +786,29 @@ main#app .popup {
   border-color: var(--popup-border);
 }
 
+main#app .close-btn {
+  /* Red square around the template's bare × button; 2rem clears the 24px
+     pointer-target minimum (SC 2.5.8). */
+  width: 2rem;
+  height: 2rem;
+  display: inline-flex;
+  align-items: center;
+  justify-content: center;
+  padding: 0;
+  border: none;
+  border-radius: 4px;
+  background: var(--popup-close-bg);
+  color: var(--popup-close-fg);
+  line-height: 1;
+}
+
+main#app .close-btn:focus-visible {
+  /* The ring floats off the red fill (blue on red is ~1:1) so it
+     contrasts with the popup surface behind it instead. */
+  outline-color: var(--brand-accent);
+  outline-offset: 2px;
+}
+
 main#app .rf-page-btn {
   background: var(--card-bg);
   border: 1px solid var(--brand-color);

tests/unit/theme-css.test.js

@@ -205,6 +205,43 @@ describe('dark mode cascade structure', () => {
   });
 });
 
+describe('popup close button', () => {
+  it('is a red square re-driven from palette variables', () => {
+    // The template ships a bare × button; the red-square chrome must come
+    // from main#app overrides (1,1,1) so both palettes stay compatible.
+    const close = rules.get('main#app .close-btn');
+    expect(close).toBeTruthy();
+    expect(close).toMatch(/background:\s*var\(--popup-close-bg\)/);
+    expect(close).toMatch(/color:\s*var\(--popup-close-fg\)/);
+
+    // SC 2.5.8: the pointer target is at least 24x24 CSS px.
+    const width = Number(close.match(/width:\s*([\d.]+)rem/)[1]);
+    const height = Number(close.match(/height:\s*([\d.]+)rem/)[1]);
+    expect(width).toBeGreaterThanOrEqual(1.5);
+    expect(height).toBeGreaterThanOrEqual(1.5);
+  });
+
+  it('the focus ring floats off the red square onto the popup surface', () => {
+    // A ring hugging the red fill cannot use --brand-accent (blue on red
+    // is ~1:1); offset it so it contrasts with --card-bg instead.
+    const focus = rules.get('main#app .close-btn:focus-visible');
+    expect(focus).toMatch(/outline-color:\s*var\(--brand-accent\)/);
+    expect(focus).toMatch(/outline-offset:\s*2px/);
+  });
+
+  for (const [name, palette] of [['light', lightPalette], ['dark', darkPalette]]) {
+    it(`the white × keeps AA text contrast on the red square (${name})`, () => {
+      expect(contrastOf(palette, '--popup-close-bg', '--popup-close-fg'))
+        .toBeGreaterThanOrEqual(4.5);
+    });
+
+    it(`the red square is perceivable against the popup surface (${name})`, () => {
+      expect(contrastOf(palette, '--card-bg', '--popup-close-bg'))
+        .toBeGreaterThanOrEqual(3);
+    });
+  }
+});
+
 describe('toggle styles', () => {
   it('the toggle renders icon pills over appearance-none radios', () => {
     expect(rules.get('.theme-toggle')).toMatch(/display:\s*flex/);