From 2e427a1e8397b562744c5152516feeec1c8e960a Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Thu, 11 Apr 2024 20:51:56 -0400
Subject: [PATCH] fix: Menus with accessory in-front of label cannot be
 navigated with keyboard letters fix: Flash of outline state when focusing
 menu item with keyboard

---
 app/components/ContextMenu/MenuItem.tsx | 7 +++----
 app/components/ContextMenu/Template.tsx | 4 ++--
 app/components/ContextMenu/index.tsx    | 6 ++++++
 shared/styles/globals.ts                | 2 ++
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/app/components/ContextMenu/MenuItem.tsx b/app/components/ContextMenu/MenuItem.tsx
index 9fbf7c2bb..a535fd137 100644
--- a/app/components/ContextMenu/MenuItem.tsx
+++ b/app/components/ContextMenu/MenuItem.tsx
@@ -71,12 +71,11 @@ const MenuItem = (
           ])}
         >
           {selected !== undefined && (
-            <>
+            <MenuIconWrapper aria-hidden>
               {selected ? <CheckmarkIcon /> : <Spacer />}
-              &nbsp;
-            </>
+            </MenuIconWrapper>
           )}
-          {icon && <MenuIconWrapper>{icon}</MenuIconWrapper>}
+          {icon && <MenuIconWrapper aria-hidden>{icon}</MenuIconWrapper>}
           {children}
         </MenuAnchor>
       );
diff --git a/app/components/ContextMenu/Template.tsx b/app/components/ContextMenu/Template.tsx
index 8c79f5b8d..fd874ffaa 100644
--- a/app/components/ContextMenu/Template.tsx
+++ b/app/components/ContextMenu/Template.tsx
@@ -126,7 +126,7 @@ function Template({ items, actions, context, ...menu }: Props) {
           item.type !== "separator" &&
           item.type !== "heading"
         ) {
-          item.icon = item.icon || <MenuIconWrapper />;
+          item.icon = item.icon || <MenuIconWrapper aria-hidden />;
         }
 
         if (item.type === "route") {
@@ -220,7 +220,7 @@ function Title({
 }) {
   return (
     <Flex align="center">
-      {icon && <MenuIconWrapper>{icon}</MenuIconWrapper>}
+      {icon && <MenuIconWrapper aria-hidden>{icon}</MenuIconWrapper>}
       {title}
     </Flex>
   );
diff --git a/app/components/ContextMenu/index.tsx b/app/components/ContextMenu/index.tsx
index 277a420c5..766eb33e2 100644
--- a/app/components/ContextMenu/index.tsx
+++ b/app/components/ContextMenu/index.tsx
@@ -223,6 +223,12 @@ export const Position = styled.div`
   position: absolute;
   z-index: ${depths.menu};
 
+  &.focus-visible {
+    transition-delay: 250ms;
+    transition-property: outline-width;
+    transition-duration: 0;
+  }
+
   /*
    * overrides make mobile-first coding style challenging
    * so we explicitly define mobile breakpoint here
diff --git a/shared/styles/globals.ts b/shared/styles/globals.ts
index 60fbafcfc..7fac03c79 100644
--- a/shared/styles/globals.ts
+++ b/shared/styles/globals.ts
@@ -106,10 +106,12 @@ export default createGlobalStyle<Props>`
 
   .js-focus-visible :focus:not(.focus-visible) {
     outline: none;
+    outline-width: 0;
   }
 
   .js-focus-visible .focus-visible {
     outline-color: ${s("accent")};
     outline-offset: -1px;
+    outline-width: initial;
   }
 `;