Skip to content

Commit 390b294

Browse files
committed
fix(ui-drawer-layout,ui-a11y-utils): fix DrawerLayout not closing on ESC keypress
The issue was that DrawerLayout updates the FocusRegion, changing the value of shouldCloseOnEscape, but FocusRegion is not adding/removing event listerers when its options change. Also add unit tests for FocusRegion (AI generated) INSTUI-4731
1 parent 1b8946f commit 390b294

File tree

3 files changed

+361
-38
lines changed

3 files changed

+361
-38
lines changed

packages/ui-a11y-utils/src/FocusRegion.ts

Lines changed: 83 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ class FocusRegion {
5656
private readonly _screenReaderFocusRegion: ScreenReaderFocusRegion
5757
private readonly _keyboardFocusRegion: KeyboardFocusRegion
5858
private readonly _id: string
59-
private _listeners: ReturnType<typeof addEventListener>[] = []
59+
private _mouseDownListener: ReturnType<typeof addEventListener> | undefined
60+
private _clickListener: ReturnType<typeof addEventListener> | undefined
61+
private _mouseUpListener: ReturnType<typeof addEventListener> | undefined
62+
private _keyUpListener: ReturnType<typeof addEventListener> | undefined
6063
private _active = false
6164
private _documentClickTarget: Node | null = null
6265
private _contextContainsTarget = false
@@ -82,6 +85,10 @@ class FocusRegion {
8285
this._contextElement = element
8386
if (options) {
8487
this._options = options
88+
this.addOrRemoveListeners(
89+
options.shouldCloseOnDocumentClick,
90+
options.shouldCloseOnEscape
91+
)
8592
}
8693
if (this._keyboardFocusRegion) {
8794
this._keyboardFocusRegion.updateElement(element)
@@ -170,53 +177,92 @@ class FocusRegion {
170177
return (findTabbable(this._contextElement) || []).length > 0
171178
}
172179

173-
activate() {
174-
if (!this._active) {
175-
const doc = ownerDocument(this._contextElement)!
176-
177-
this._keyboardFocusRegion.activate()
178-
this._screenReaderFocusRegion.activate()
179-
180-
if (this._options.shouldCloseOnDocumentClick) {
181-
this._listeners.push(
182-
addEventListener(doc, 'mousedown', this.captureDocumentMousedown)
180+
/**
181+
* Adds or removes mouse/keyboard listeners based on the input parameters
182+
* @param shouldCloseOnDocumentClick Should add listeners that close the region if there is an outside click?
183+
* @param shouldCloseOnEscape Should the region be closed if ESC is pressed?
184+
*/
185+
addOrRemoveListeners(
186+
shouldCloseOnDocumentClick?: boolean,
187+
shouldCloseOnEscape?: boolean
188+
) {
189+
const doc = ownerDocument(this._contextElement)!
190+
if (shouldCloseOnDocumentClick) {
191+
if (!this._mouseDownListener) {
192+
this._mouseDownListener = addEventListener(
193+
doc,
194+
'mousedown',
195+
this.captureDocumentMousedown
183196
)
184-
185-
this._listeners.push(
186-
addEventListener(doc, 'click', this.handleDocumentClick)
197+
}
198+
if (!this._clickListener) {
199+
this._clickListener = addEventListener(
200+
doc,
201+
'click',
202+
this.handleDocumentClick
187203
)
188-
189-
Array.from(doc.getElementsByTagName('iframe')).forEach((el) => {
190-
// listen for mouseup events on any iframes in the document
191-
const frameDoc = el.contentDocument
192-
if (frameDoc) {
193-
this._listeners.push(
194-
addEventListener(frameDoc, 'mouseup', (event) => {
195-
this.handleFrameClick(event as React.MouseEvent, el)
196-
})
197-
)
198-
}
199-
})
200204
}
201-
//This will ensure that the Tooltip's Escape event listener is executed first
202-
//because listeners in the capturing phase are called before other event listeners (like that of the Modal's Escape listener)
203-
//so a Modal with a Tooltip will not get closed when closing the Tooltip with Escape
204-
const useCapture = this._options.isTooltip
205-
if (this._options.shouldCloseOnEscape) {
206-
this._listeners.push(
207-
addEventListener(doc, 'keyup', this.handleKeyUp, useCapture)
205+
Array.from(doc.getElementsByTagName('iframe')).forEach((el) => {
206+
// listen for mouseup events on any iframes in the document
207+
const frameDoc = el.contentDocument
208+
if (frameDoc && !this._mouseUpListener) {
209+
this._mouseUpListener = addEventListener(
210+
frameDoc,
211+
'mouseup',
212+
(event) => {
213+
this.handleFrameClick(event as React.MouseEvent, el)
214+
}
215+
)
216+
}
217+
})
218+
} else {
219+
this._mouseDownListener?.remove()
220+
this._mouseDownListener = undefined
221+
this._clickListener?.remove()
222+
this._clickListener = undefined
223+
this._mouseUpListener?.remove()
224+
this._mouseUpListener = undefined
225+
}
226+
if (shouldCloseOnEscape) {
227+
if (!this._keyUpListener) {
228+
//This will ensure that the Tooltip's Escape event listener is executed first
229+
//because listeners in the capturing phase are called before other event listeners (like that of the Modal's Escape listener)
230+
//so a Modal with a Tooltip will not get closed when closing the Tooltip with Escape
231+
const useCapture = this._options.isTooltip
232+
this._keyUpListener = addEventListener(
233+
doc,
234+
'keyup',
235+
this.handleKeyUp,
236+
useCapture
208237
)
209238
}
239+
} else {
240+
this._keyUpListener?.remove()
241+
this._keyUpListener = undefined
242+
}
243+
}
210244

245+
activate() {
246+
if (!this._active) {
247+
this._keyboardFocusRegion.activate()
248+
this._screenReaderFocusRegion.activate()
249+
this.addOrRemoveListeners(
250+
this._options.shouldCloseOnDocumentClick,
251+
this._options.shouldCloseOnEscape
252+
)
211253
this._active = true
212254
}
213255
}
214256

215257
deactivate({ keyboard = true }: { keyboard?: boolean } = {}) {
216-
this._listeners.forEach((listener) => {
217-
listener.remove()
218-
})
219-
this._listeners = []
258+
this._mouseDownListener?.remove()
259+
this._mouseDownListener = undefined
260+
this._clickListener?.remove()
261+
this._clickListener = undefined
262+
this._mouseUpListener?.remove()
263+
this._mouseUpListener = undefined
264+
this._keyUpListener?.remove()
265+
this._keyUpListener = undefined
220266
if (keyboard) {
221267
this._keyboardFocusRegion.deactivate()
222268
}

0 commit comments

Comments
 (0)