From 0fe26782fb250b80c3c4a92eb17b400f9153da78 Mon Sep 17 00:00:00 2001
From: cheton <cheton@gmail.com>
Date: Sun, 8 Oct 2023 21:47:12 +0800
Subject: [PATCH] feat: implement `:focus-visible` for the Button component
 with targeted focus style for non-pointer devices

---
 packages/react/src/button/styles.js           | 80 ++++++++++---------
 .../__tests__/__snapshots__/Menu.test.js.snap | 18 ++---
 .../__snapshots__/Popover.test.js.snap        | 17 ++--
 .../__snapshots__/Tooltip.test.js.snap        | 17 ++--
 4 files changed, 62 insertions(+), 70 deletions(-)

diff --git a/packages/react/src/button/styles.js b/packages/react/src/button/styles.js
index 1c0372d3f6..b28a16da0f 100644
--- a/packages/react/src/button/styles.js
+++ b/packages/react/src/button/styles.js
@@ -28,20 +28,20 @@ const defaultVariantStyle = ({
     light: 'gray:30',
   }[colorMode];
   // Focus
-  const focusBorderColor = {
+  const focusVisibleBorderColor = {
     dark: 'blue:60',
     light: 'blue:60',
   }[colorMode];
-  const focusBoxShadowOuterColor = {
-    dark: theme?.colors?.['blue:60'],
-    light: theme?.colors?.['blue:60'],
+  const focusVisibleBoxShadowOuterColor = {
+    dark: 'blue:60',
+    light: 'blue:60',
   }[colorMode];
-  const focusBoxShadowOuterSpreadRadius = theme?.sizes?.['1q'];
-  const focusBoxShadowInnerColor = {
-    dark: theme?.colors?.['black:emphasis'],
-    light: theme?.colors?.['white:emphasis'],
+  const focusVisibleBoxShadowOuterSpreadRadius = '1q';
+  const focusVisibleBoxShadowInnerColor = {
+    dark: 'black:emphasis',
+    light: 'white:emphasis',
   }[colorMode];
-  const focusBoxShadowInnerSpreadRadius = theme?.sizes?.['2q'];
+  const focusVisibleBoxShadowInnerSpreadRadius = '2q';
   // Disabled
   const disabledBackgroundColor = {
     dark: 'gray:60',
@@ -69,11 +69,12 @@ const defaultVariantStyle = ({
     backgroundColor,
     borderColor: 'transparent',
     color,
-    _focus: {
-      ':not(:active)': {
-        borderColor: focusBorderColor,
-        boxShadow: `inset 0 0 0 ${focusBoxShadowOuterSpreadRadius} ${focusBoxShadowOuterColor}, inset 0 0 0 ${focusBoxShadowInnerSpreadRadius} ${focusBoxShadowInnerColor}`,
-      },
+    _focusVisible: {
+      borderColor: focusVisibleBorderColor,
+      boxShadow: [
+        `inset 0 0 0 ${theme?.sizes?.[focusVisibleBoxShadowOuterSpreadRadius]} ${theme?.colors?.[focusVisibleBoxShadowOuterColor]}`,
+        `inset 0 0 0 ${theme?.sizes?.[focusVisibleBoxShadowInnerSpreadRadius]} ${theme?.colors?.[focusVisibleBoxShadowInnerColor]}`,
+      ].join(', '),
       // Bring overlapping border to front when focused
       zIndex: 1,
     },
@@ -130,16 +131,15 @@ const secondaryVariantStyle = ({
   const activeBorderColor = hoverBorderColor;
   const activeColor = hoverColor;
   // Focus
-  const focusBorderColor = {
+  const focusVisibleBorderColor = {
     dark: 'blue:60',
     light: 'blue:60',
   }[colorMode];
-  const focusBoxShadowColor = {
-    dark: theme?.colors?.['blue:60'],
-    light: theme?.colors?.['blue:60'],
+  const focusVisibleBoxShadowColor = {
+    dark: 'blue:60',
+    light: 'blue:60',
   }[colorMode];
-  const focusBoxShadowSpreadRadius = theme?.sizes?.['1q'];
-  const focusColor = color;
+  const focusVisibleBoxShadowSpreadRadius = '1q';
   // Disabled
   const disabledBorderColor = borderColor;
   const disabledColor = {
@@ -163,16 +163,15 @@ const secondaryVariantStyle = ({
   return {
     borderColor,
     color,
-    _focus: {
-      borderColor: focusBorderColor,
-      boxShadow: `inset 0 0 0 ${focusBoxShadowSpreadRadius} ${focusBoxShadowColor}`,
-      color: focusColor,
+    _focusVisible: {
+      borderColor: focusVisibleBorderColor,
+      boxShadow: `inset 0 0 0 ${theme?.sizes?.[focusVisibleBoxShadowSpreadRadius]} ${theme?.colors?.[focusVisibleBoxShadowColor]}`,
       // Bring overlapping border to front when focused
       zIndex: 1,
     },
     _hover: {
       color: hoverColor,
-      '&:not(:focus)': {
+      '&:not(:focus-visible)': {
         borderColor: hoverBorderColor,
       },
       // Use a higher z-index value to bring overlapping border to front when hovered
@@ -263,16 +262,20 @@ const fillColorVariantStyle = ({
     light: `${colorProp}:70`,
   }[colorMode];
   // Focus
-  const focusBorderColor = {
-    dark: theme?.colors?.['blue:60'],
-    light: theme?.colors?.['blue:60'],
+  const focusVisibleBorderColor = {
+    dark: 'blue:60',
+    light: 'blue:60',
+  }[colorMode];
+  const focusVisibleBoxShadowOuterColor = {
+    dark: 'blue:60',
+    light: 'blue:60',
   }[colorMode];
-  const focusBoxShadowSpreadRadius = theme?.sizes?.['1q'];
-  const boxShadowColor = {
-    dark: theme?.colors?.['black:emphasis'],
-    light: theme?.colors?.['white:emphasis'],
+  const focusVisibleBoxShadowOuterSpreadRadius = '1q';
+  const focusVisibleBoxShadowInnerColor = {
+    dark: 'black:emphasis',
+    light: 'white:emphasis',
   }[colorMode];
-  const boxShadowSpreadRadius = theme?.sizes?.['2q'];
+  const focusVisibleBoxShadowInnerSpreadRadius = '2q';
   // Disabled
   const disabledBackgroundColor = {
     dark: 'gray:60',
@@ -300,11 +303,12 @@ const fillColorVariantStyle = ({
     backgroundColor,
     borderColor: 'transparent',
     color,
-    _focus: {
-      ':not(:active)': {
-        borderColor: focusBorderColor,
-        boxShadow: `inset 0 0 0 ${focusBoxShadowSpreadRadius} ${focusBorderColor}, inset 0 0 0 ${boxShadowSpreadRadius} ${boxShadowColor}`,
-      },
+    _focusVisible: {
+      borderColor: focusVisibleBorderColor,
+      boxShadow: [
+        `inset 0 0 0 ${theme?.sizes?.[focusVisibleBoxShadowOuterSpreadRadius]} ${theme?.colors?.[focusVisibleBoxShadowOuterColor]}`,
+        `inset 0 0 0 ${theme?.sizes?.[focusVisibleBoxShadowInnerSpreadRadius]} ${theme?.colors?.[focusVisibleBoxShadowInnerColor]}`,
+      ].join(', '),
       // Bring overlapping border to front when focused
       zIndex: 1,
     },
diff --git a/packages/react/src/menu/__tests__/__snapshots__/Menu.test.js.snap b/packages/react/src/menu/__tests__/__snapshots__/Menu.test.js.snap
index 689f3b2757..c9477de741 100644
--- a/packages/react/src/menu/__tests__/__snapshots__/Menu.test.js.snap
+++ b/packages/react/src/menu/__tests__/__snapshots__/Menu.test.js.snap
@@ -49,6 +49,12 @@ exports[`Menu should render correctly 1`] = `
   column-gap: var(--tonic-sizes-1x);
 }
 
+.emotion-2:focus-visible {
+  border-color: var(--tonic-colors-blue-60);
+  box-shadow: inset 0 0 0 .0625rem #1e5ede;
+  z-index: 1;
+}
+
 .emotion-2[aria-selected=true],
 .emotion-2[data-selected] {
   background-color: var(--tonic-colors-gray-70);
@@ -56,22 +62,14 @@ exports[`Menu should render correctly 1`] = `
   pointer-events: none;
 }
 
-.emotion-2:focus,
-.emotion-2[data-focus] {
-  border-color: var(--tonic-colors-blue-60);
-  box-shadow: inset 0 0 0 .0625rem #1e5ede;
-  color: var(--tonic-colors-white-primary);
-  z-index: 1;
-}
-
 .emotion-2:hover,
 .emotion-2[data-hover] {
   color: var(--tonic-colors-blue-40);
   z-index: 2;
 }
 
-.emotion-2:hover:not(:focus),
-.emotion-2[data-hover]:not(:focus) {
+.emotion-2:hover:not(:focus-visible),
+.emotion-2[data-hover]:not(:focus-visible) {
   border-color: var(--tonic-colors-blue-50);
 }
 
diff --git a/packages/react/src/popover/__tests__/__snapshots__/Popover.test.js.snap b/packages/react/src/popover/__tests__/__snapshots__/Popover.test.js.snap
index e3935baf0c..08bea810e3 100644
--- a/packages/react/src/popover/__tests__/__snapshots__/Popover.test.js.snap
+++ b/packages/react/src/popover/__tests__/__snapshots__/Popover.test.js.snap
@@ -40,6 +40,12 @@ exports[`Popover should render correctly 1`] = `
   border-color: var(--tonic-colors-transparent);
 }
 
+.emotion-0:focus-visible {
+  border-color: var(--tonic-colors-blue-60);
+  box-shadow: inset 0 0 0 .0625rem #1e5ede,inset 0 0 0 .125rem rgba(0, 0, 0, 1.0);
+  z-index: 1;
+}
+
 .emotion-0[aria-selected=true],
 .emotion-0[data-selected] {
   background-color: var(--tonic-colors-gray-70);
@@ -47,17 +53,6 @@ exports[`Popover should render correctly 1`] = `
   pointer-events: none;
 }
 
-.emotion-0:focus,
-.emotion-0[data-focus] {
-  z-index: 1;
-}
-
-.emotion-0:focus:not(:active),
-.emotion-0[data-focus]:not(:active) {
-  border-color: var(--tonic-colors-blue-60);
-  box-shadow: inset 0 0 0 .0625rem #1e5ede,inset 0 0 0 .125rem rgba(0, 0, 0, 1.0);
-}
-
 .emotion-0:hover,
 .emotion-0[data-hover] {
   background-color: var(--tonic-colors-gray-50);
diff --git a/packages/react/src/tooltip/__tests__/__snapshots__/Tooltip.test.js.snap b/packages/react/src/tooltip/__tests__/__snapshots__/Tooltip.test.js.snap
index b61dca3ded..34d622c972 100644
--- a/packages/react/src/tooltip/__tests__/__snapshots__/Tooltip.test.js.snap
+++ b/packages/react/src/tooltip/__tests__/__snapshots__/Tooltip.test.js.snap
@@ -40,6 +40,12 @@ exports[`Tooltip should render correctly 1`] = `
   border-color: var(--tonic-colors-transparent);
 }
 
+.emotion-0:focus-visible {
+  border-color: var(--tonic-colors-blue-60);
+  box-shadow: inset 0 0 0 .0625rem #1e5ede,inset 0 0 0 .125rem rgba(0, 0, 0, 1.0);
+  z-index: 1;
+}
+
 .emotion-0[aria-selected=true],
 .emotion-0[data-selected] {
   background-color: var(--tonic-colors-gray-70);
@@ -47,17 +53,6 @@ exports[`Tooltip should render correctly 1`] = `
   pointer-events: none;
 }
 
-.emotion-0:focus,
-.emotion-0[data-focus] {
-  z-index: 1;
-}
-
-.emotion-0:focus:not(:active),
-.emotion-0[data-focus]:not(:active) {
-  border-color: var(--tonic-colors-blue-60);
-  box-shadow: inset 0 0 0 .0625rem #1e5ede,inset 0 0 0 .125rem rgba(0, 0, 0, 1.0);
-}
-
 .emotion-0:hover,
 .emotion-0[data-hover] {
   background-color: var(--tonic-colors-gray-50);