fix(overlay): safari :focus-visible inconsistency when using overlay type modal

packages/action-menu/test/index.ts

@@ -41,7 +41,7 @@ import type { Overlay } from '@spectrum-web-components/overlay';
 import { sendKeys, setViewport } from '@web/test-runner-commands';
 import { TemplateResult } from '@spectrum-web-components/base';
 import { isWebKit } from '@spectrum-web-components/shared';
-import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/picker/src/MobileController.js';
+import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/picker/src/InteractionController.js';
 
 ignoreResizeObserverLoopError(before, after);
 

packages/button/src/button.css

@@ -21,6 +21,11 @@ governing permissions and limitations under the License.
     :host([treatment]:not([disabled]):hover) {
         border-color: highlight;
     }
+
+    :host(.remove-focus-ring-safari-hack:focus-visible):after {
+        forced-color-adjust: none;
+        box-shadow: none;
+    }
 }
 
 @keyframes show-progress-circle {
@@ -99,3 +104,39 @@ sp-progress-circle {
 :host([pending]:not([disabled])) #label {
     animation: hide-icons-label 0s var(--pending-delay, 1s) forwards;
 }
+
+:host(.remove-focus-ring-safari-hack:focus-visible):after {
+    margin: calc(
+        -1 * var(--mod-button-focus-indicator-gap, var(--mod-focus-indicator-gap, var(--spectrum-focus-indicator-gap)))
+    );
+    box-shadow: none;
+}
+
+:host(.remove-focus-ring-safari-hack:focus-visible) {
+    box-shadow: none;
+    outline: none;
+}
+
+:host(.remove-focus-ring-safari-hack:focus-visible:not(:hover)) {
+    background-color: var(
+        --highcontrast-button-background-color-default,
+        var(
+            --mod-button-background-color-default,
+            var(--spectrum-button-background-color-default)
+        )
+    );
+    border-color: var(
+        --highcontrast-button-border-color-default,
+        var(
+            --mod-button-border-color-default,
+            var(--spectrum-button-border-color-default)
+        )
+    );
+    color: var(
+        --highcontrast-button-content-color-default,
+        var(
+            --mod-button-content-color-default,
+            var(--spectrum-button-content-color-default)
+        )
+    );
+}

packages/overlay/src/HoverController.ts

@@ -11,13 +11,15 @@ governing permissions and limitations under the License.
 */
 
 import { conditionAttributeWithId } from '@spectrum-web-components/base/src/condition-attribute-with-id.js';
+import { isWebKit } from '@spectrum-web-components/shared';
 import { randomID } from '@spectrum-web-components/shared/src/random-id.js';
-
+import { noop } from './AbstractOverlay.js';
 import {
     InteractionController,
     InteractionTypes,
+    lastInteractionType,
+    SAFARI_FOCUS_RING_CLASS,
 } from './InteractionController.js';
-import { noop } from './AbstractOverlay.js';
 
 const HOVER_DELAY = 300;
 
@@ -32,15 +34,33 @@ export class HoverController extends InteractionController {
 
     pointerentered = false;
 
+    handleKeyup(event: KeyboardEvent): void {
+        if (event.code === 'Tab' || event.code === 'Escape') {
+            this.open = true;
+            this.removeSafariFocusRingClass();
+        }
+    }
+
     handleTargetFocusin(): void {
         if (!this.target.matches(':focus-visible')) {
             return;
         }
+
+        if (
+            isWebKit() &&
+            this.target[lastInteractionType] === InteractionTypes.click
+        ) {
+            this.target.classList.add(SAFARI_FOCUS_RING_CLASS);
+            return;
+        }
+
         this.open = true;
         this.focusedin = true;
+        this.removeSafariFocusRingClass();
     }
 
     handleTargetFocusout(): void {
+        this.removeSafariFocusRingClass();
         this.focusedin = false;
         if (this.pointerentered) return;
         this.open = false;
@@ -138,6 +158,11 @@ export class HoverController extends InteractionController {
         this.abortController?.abort();
         this.abortController = new AbortController();
         const { signal } = this.abortController;
+        this.target.addEventListener(
+            'keyup',
+            (event) => this.handleKeyup(event),
+            { signal }
+        );
         this.target.addEventListener(
             'focusin',
             () => this.handleTargetFocusin(),
@@ -179,4 +204,12 @@ export class HoverController extends InteractionController {
             { signal }
         );
     }
+
+    private removeSafariFocusRingClass(): void {
+        if (
+            isWebKit() &&
+            this.target.classList.contains(SAFARI_FOCUS_RING_CLASS)
+        )
+            this.target.classList.remove(SAFARI_FOCUS_RING_CLASS);
+    }
 }

packages/overlay/src/InteractionController.ts

@@ -14,17 +14,24 @@ import type { ReactiveController } from '@spectrum-web-components/base';
 import { AbstractOverlay } from './AbstractOverlay.js';
 
 export enum InteractionTypes {
-    'click',
-    'hover',
-    'longpress',
+    click = 'click',
+    hover = 'hover',
+    longpress = 'longpress',
 }
 
+export const lastInteractionType = Symbol('lastInteractionType');
+export const SAFARI_FOCUS_RING_CLASS = 'remove-focus-ring-safari-hack';
+
 export type ControllerOptions = {
     overlay?: AbstractOverlay;
     handleOverlayReady?: (overlay: AbstractOverlay) => void;
     isPersistent?: boolean;
 };
 
+type InteractionTarget = HTMLElement & {
+    [lastInteractionType]?: InteractionTypes;
+};
+
 export class InteractionController implements ReactiveController {
     abortController!: AbortController;
 
@@ -50,6 +57,7 @@ export class InteractionController implements ReactiveController {
         if (this.overlay) {
             // If there already is an Overlay, apply the value of `open` directly.
             this.overlay.open = open;
+            this.target[lastInteractionType] = this.type;
             return;
         }
         if (!open) {
@@ -65,6 +73,7 @@ export class InteractionController implements ReactiveController {
                 const { Overlay } = await import('./Overlay.js');
                 this.overlay = new Overlay();
                 this.overlay.open = true;
+                this.target[lastInteractionType] = this.type;
             });
         import('@spectrum-web-components/overlay/sp-overlay.js');
     }
@@ -93,7 +102,7 @@ export class InteractionController implements ReactiveController {
     type!: InteractionTypes;
 
     constructor(
-        public target: HTMLElement,
+        public target: InteractionTarget,
         { overlay, isPersistent, handleOverlayReady }: ControllerOptions
     ) {
         this.isPersistent = !!isPersistent;

packages/overlay/stories/overlay.stories.ts

@@ -8,8 +8,17 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
+import '@spectrum-web-components/action-button/sp-action-button.js';
+import '@spectrum-web-components/action-group/sp-action-group.js';
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
+import '@spectrum-web-components/button/sp-button.js';
+import { DialogWrapper } from '@spectrum-web-components/dialog';
+import '@spectrum-web-components/dialog/sp-dialog-wrapper.js';
+import '@spectrum-web-components/dialog/sp-dialog.js';
+import '@spectrum-web-components/field-label/sp-field-label.js';
+import '@spectrum-web-components/icons-workflow/icons/sp-icon-magnify.js';
+import '@spectrum-web-components/icons-workflow/icons/sp-icon-open-in.js';
 import {
     openOverlay,
     Overlay,
@@ -19,40 +28,31 @@ import {
     TriggerInteractions,
     VirtualTrigger,
 } from '@spectrum-web-components/overlay';
-import '@spectrum-web-components/action-button/sp-action-button.js';
-import '@spectrum-web-components/action-group/sp-action-group.js';
-import '@spectrum-web-components/button/sp-button.js';
-import '@spectrum-web-components/dialog/sp-dialog.js';
-import '@spectrum-web-components/dialog/sp-dialog-wrapper.js';
-import { DialogWrapper } from '@spectrum-web-components/dialog';
-import '@spectrum-web-components/field-label/sp-field-label.js';
-import '@spectrum-web-components/icons-workflow/icons/sp-icon-magnify.js';
-import '@spectrum-web-components/icons-workflow/icons/sp-icon-open-in.js';
 import '@spectrum-web-components/overlay/overlay-trigger.js';
 
+import '@spectrum-web-components/accordion/sp-accordion-item.js';
+import '@spectrum-web-components/accordion/sp-accordion.js';
+import '@spectrum-web-components/button-group/sp-button-group.js';
+import '@spectrum-web-components/menu/sp-menu-divider.js';
+import '@spectrum-web-components/menu/sp-menu-group.js';
+import '@spectrum-web-components/menu/sp-menu-item.js';
+import '@spectrum-web-components/menu/sp-menu.js';
+import '@spectrum-web-components/overlay/sp-overlay.js';
 import { Picker } from '@spectrum-web-components/picker';
 import '@spectrum-web-components/picker/sp-picker.js';
-import '@spectrum-web-components/overlay/sp-overlay.js';
-import '@spectrum-web-components/menu/sp-menu.js';
-import '@spectrum-web-components/menu/sp-menu-item.js';
-import '@spectrum-web-components/menu/sp-menu-group.js';
-import '@spectrum-web-components/menu/sp-menu-divider.js';
 import '@spectrum-web-components/popover/sp-popover.js';
-import '@spectrum-web-components/slider/sp-slider.js';
-import '@spectrum-web-components/radio/sp-radio.js';
 import '@spectrum-web-components/radio/sp-radio-group.js';
-import '@spectrum-web-components/tooltip/sp-tooltip.js';
+import '@spectrum-web-components/radio/sp-radio.js';
+import '@spectrum-web-components/slider/sp-slider.js';
 import '@spectrum-web-components/theme/sp-theme.js';
 import '@spectrum-web-components/theme/src/themes.js';
-import '@spectrum-web-components/accordion/sp-accordion.js';
-import '@spectrum-web-components/accordion/sp-accordion-item.js';
-import '@spectrum-web-components/button-group/sp-button-group.js';
+import '@spectrum-web-components/tooltip/sp-tooltip.js';
 import '../../../projects/story-decorator/src/types.js';
 
-import './overlay-story-components.js';
-import { render } from 'lit-html';
-import { Popover } from '@spectrum-web-components/popover';
 import { Button } from '@spectrum-web-components/button';
+import { Popover } from '@spectrum-web-components/popover';
+import { render } from 'lit-html';
+import './overlay-story-components.js';
 import { PopoverContent } from './overlay-story-components.js';
 
 const storyStyles = html`
@@ -296,12 +296,32 @@ export const accordion = (): TemplateResult => {
 accordion.swc_vrt = {
     skip: true,
 };
-
 accordion.parameters = {
     // Disables Chromatic's snapshotting on a global level
     chromatic: { disableSnapshot: true },
 };
 
+export const clickAndHoverTarget = (): TemplateResult => {
+    return html`
+        <overlay-trigger type="modal">
+            <sp-button variant="primary" slot="trigger">Button</sp-button>
+            <sp-popover slot="click-content" placement="bottom" tip>
+                Popover content
+            </sp-popover>
+            <sp-tooltip slot="hover-content" placement="right">
+                Tooltip content
+            </sp-tooltip>
+        </overlay-trigger>
+    `;
+};
+clickAndHoverTarget.swc_vrt = {
+    skip: true,
+};
+clickAndHoverTarget.parameters = {
+    // Disables Chromatic's snapshotting on a global level
+    chromatic: { disableSnapshot: true },
+};
+
 export const clickAndHoverTargets = (): TemplateResult => {
     return html`
         <div>

packages/overlay/test/overlay.test.ts

@@ -34,6 +34,7 @@ import {
 } from '@open-wc/testing';
 import { sendKeys } from '@web/test-runner-commands';
 import {
+    clickAndHoverTarget,
     definedOverlayElement,
     virtualElement,
 } from '../stories/overlay.stories';
@@ -50,6 +51,9 @@ import {
     isOnTopLayer,
 } from '../../../test/testing-helpers.js';
 import { Menu } from '@spectrum-web-components/menu';
+import { Button } from '@spectrum-web-components/button';
+import { isWebKit } from '@spectrum-web-components/shared';
+import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/overlay/src/InteractionController.js';
 
 async function styledFixture<T extends Element>(
     story: TemplateResult
@@ -786,6 +790,74 @@ describe('Overlay - type="modal"', () => {
         await close;
         expect(el.open).to.be.null;
     });
+
+    describe('maintains consistency of focus ring in different browsers', () => {
+        it('should not open hover overlay right after closing the click overlay using the mouse', async () => {
+            const overlayTrigger = await fixture<OverlayTrigger>(
+                clickAndHoverTarget()
+            );
+
+            await elementUpdated(overlayTrigger);
+            expect(overlayTrigger.open).to.be.undefined;
+
+            const trigger = overlayTrigger.querySelector(
+                'sp-button[slot="trigger"]'
+            ) as Button;
+
+            const opened = oneEvent(trigger, 'sp-opened');
+            trigger.click();
+            await opened;
+
+            expect(overlayTrigger.open).to.equal('click');
+
+            const closed = oneEvent(trigger, 'sp-closed');
+            sendMouse({
+                steps: [
+                    {
+                        type: 'click',
+                        position: [1, 1],
+                    },
+                ],
+            });
+            await closed;
+
+            expect(overlayTrigger.open).to.be.undefined;
+            expect(document.activeElement === trigger, 'trigger focused').to.be
+                .true;
+            if (isWebKit())
+                expect(trigger.classList.contains(SAFARI_FOCUS_RING_CLASS)).to
+                    .be.true;
+        });
+
+        it('should not open hover overlay right after closing the click overlay using the keyboard', async () => {
+            const overlayTrigger = await fixture<OverlayTrigger>(
+                clickAndHoverTarget()
+            );
+
+            const trigger = overlayTrigger.querySelector(
+                'sp-button[slot="trigger"]'
+            ) as Button;
+
+            const opened = oneEvent(trigger, 'sp-opened');
+            trigger.click();
+            await opened;
+
+            expect(overlayTrigger.open).to.equal('click');
+
+            const closed = oneEvent(trigger, 'sp-closed');
+            sendKeys({
+                press: 'Escape',
+            });
+            await closed;
+
+            expect(overlayTrigger.open).to.be.undefined;
+            expect(document.activeElement === trigger, 'trigger focused').to.be
+                .true;
+            if (isWebKit())
+                expect(trigger.classList.contains(SAFARI_FOCUS_RING_CLASS)).to
+                    .be.true;
+        });
+    });
 });
 describe('Overlay - timing', () => {
     it('manages multiple modals in a row without preventing them from closing', async () => {

packages/picker/src/InteractionController.ts

@@ -22,6 +22,7 @@ export enum InteractionTypes {
     'desktop',
     'mobile',
 }
+export const SAFARI_FOCUS_RING_CLASS = 'remove-focus-ring-safari-hack';
 
 export class InteractionController implements ReactiveController {
     abortController!: AbortController;