Skip to content

Commit

Permalink
fix(430): ESC intermittently not working in Safari
Browse files Browse the repository at this point in the history
There's a weird issue, probably related to Safari's `:focus-visible`
behavior or their `<dialog>` implementation, where the `<dialog>`
element's button doesn't receive focus when it's opened. It could also
be a race condition because of the animation.

Regardless, let's try listening for `Escape` at the `document` level
when we zoom an image. We need to make sure we provide the `useCapture`
option and get to the event first so we can intercept it and don't
trigger the default `<dialog>` `Escape` behavior so we can animate
zooming out.
  • Loading branch information
rpearce committed Jul 29, 2023
1 parent 5f44d06 commit ee5a4f5
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions source/Controlled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import React, {
Component,
ElementType,
ImgHTMLAttributes,
KeyboardEvent,
MouseEvent,
ReactElement,
ReactNode,
Expand Down Expand Up @@ -153,7 +152,6 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
const {
handleDialogCancel,
handleDialogClick,
handleDialogKeyDown,
handleUnzoom,
handleZoom,
imgEl,
Expand Down Expand Up @@ -304,7 +302,7 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
</button>
</WrapElement>}
{hasImage && elDialogContainer != null && createPortal(
<dialog /* eslint-disable-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-redundant-roles */
<dialog /* eslint-disable-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-redundant-roles */
aria-labelledby={idModalImg}
aria-modal="true"
className={classDialog}
Expand All @@ -313,7 +311,6 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
onClick={handleDialogClick}
onClose={handleUnzoom /* eslint-disable-line react/no-unknown-property */}
onCancel={handleDialogCancel}
onKeyDown={handleDialogKeyDown}
ref={refDialog}
role="dialog"
>
Expand Down Expand Up @@ -352,6 +349,7 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
window.removeEventListener('touchend', this.handleTouchMove)
window.removeEventListener('touchcancel', this.handleTouchCancel)
window.removeEventListener('resize', this.handleResize)
document.removeEventListener('keydown', this.handleKeyDown, true)
}

// ===========================================================================
Expand Down Expand Up @@ -503,7 +501,7 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
/**
* Intercept default dialog.close() and use ours so we can animate
*/
handleDialogKeyDown = (e: KeyboardEvent<HTMLDialogElement>) => {
handleKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape' || e.keyCode === 27) {
e.preventDefault()
e.stopPropagation()
Expand Down Expand Up @@ -585,6 +583,7 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
window.addEventListener('touchstart', this.handleTouchStart, { passive: true })
window.addEventListener('touchend', this.handleTouchMove, { passive: true })
window.addEventListener('touchcancel', this.handleTouchCancel, { passive: true })
document.addEventListener('keydown', this.handleKeyDown, true)

this.refModalImg.current?.addEventListener?.('transitionend', this.handleZoomEnd, { once: true })
}
Expand Down Expand Up @@ -612,6 +611,7 @@ class ControlledBase extends Component<ControlledPropsWithDefaults, ControlledSt
window.removeEventListener('touchstart', this.handleTouchStart)
window.removeEventListener('touchend', this.handleTouchMove)
window.removeEventListener('touchcancel', this.handleTouchCancel)
document.removeEventListener('keydown', this.handleKeyDown, true)

this.refModalImg.current?.addEventListener?.('transitionend', this.handleUnzoomEnd, { once: true })
}
Expand Down

0 comments on commit ee5a4f5

Please sign in to comment.