Skip to content

Commit eb3f47e

Browse files
committed
fix(ui-dialog,ui-a11y-utils): fix focus getting stuck in some cases if something is removed from the middle of the focus stack
The issue occured in the following conditions: 1.A non-keyboard focusable element is added to the top of the stack,e.g. an Overlay with just text 2.The keyboard focusable element is removed from the stack just below it. In this caset the focus region stayesd the now removed element getting the focus stuck Fixes INSTUI-4685
1 parent 327fe69 commit eb3f47e

File tree

5 files changed

+98
-14
lines changed

5 files changed

+98
-14
lines changed

cypress/component/Focusable.cy.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2222
* SOFTWARE.
2323
*/
24-
import React from 'react'
24+
2525
import { Focusable } from '@instructure/ui'
2626

2727
import '../support/component'

cypress/component/Tray.cy.tsx

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
* SOFTWARE.
2323
*/
2424

25-
import { Tray } from '@instructure/ui'
25+
import { Button, Overlay, Text, Tray, View } from '@instructure/ui'
26+
import React from 'react'
2627

2728
import '../support/component'
2829
import 'cypress-real-events'
@@ -66,4 +67,79 @@ describe('<Tray />', () => {
6667
cy.get('@tray').realPress('Escape')
6768
cy.wrap(onDismiss).should('have.been.called')
6869
})
70+
71+
it.only('should handle focus properly in complex cases', async () => {
72+
const Example = () => {
73+
const [showTray, setShowTray] = React.useState(false)
74+
const [showOverlay, setShowOverlay] = React.useState(false)
75+
76+
const handleTrayButtonClick = () => {
77+
setShowOverlay(true)
78+
// hide the Tray after the Overlay is rendered.
79+
// This means it's not on the top of the FocusRegion stack
80+
setTimeout(() => {
81+
setShowTray(false)
82+
}, 30)
83+
}
84+
85+
return (
86+
<View textAlign="center">
87+
<View as="div" maxWidth="600px" margin="0 auto">
88+
<Text as="p" size="large">
89+
Click the button below to open a tray. Then click the button
90+
inside the tray to trigger an overlay that will automatically
91+
close after a short time.
92+
</Text>
93+
<Button
94+
color="primary"
95+
size="large"
96+
id="open_tray_button"
97+
onClick={() => setShowTray(true)}
98+
>
99+
Open Tray
100+
</Button>
101+
<Button id="test1_button">test 1</Button>
102+
<Button id="test2_button">test 2</Button>
103+
</View>
104+
105+
<Tray label="Sample Tray" open={showTray} placement="end">
106+
<Text as="p">
107+
This is the Tray. Click the button below to show an overlay and
108+
automatically close this tray after a short time.
109+
</Text>
110+
<Button
111+
color="success"
112+
id="close_tray_button"
113+
onClick={handleTrayButtonClick}
114+
>
115+
Close after a short time
116+
</Button>
117+
<Button onClick={() => setShowTray(false)}>Cancel</Button>
118+
<Button>test</Button>
119+
</Tray>
120+
121+
<Overlay open={showOverlay} transition="fade" label="Loading overlay">
122+
<Text size="large" color="success">
123+
This is the overlay.
124+
</Text>
125+
</Overlay>
126+
</View>
127+
)
128+
}
129+
130+
cy.mount(<Example />)
131+
132+
cy.get('#open_tray_button').click()
133+
// Wait 500ms so the Tray CSS animation can finish
134+
cy.wait(500)
135+
// Click the close_tray_button. This should run the state changes in its handler
136+
cy.get('#close_tray_button').click()
137+
// Wait 500ms so the Tray CSS animation can finish
138+
cy.wait(500)
139+
cy.focused().should('have.attr', 'id', 'open_tray_button')
140+
cy.realPress('Tab')
141+
cy.focused().should('have.attr', 'id', 'test1_button')
142+
cy.realPress('Tab')
143+
cy.focused().should('have.attr', 'id', 'test2_button')
144+
})
69145
})

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,15 @@ class FocusRegion {
6767
shouldCloseOnEscape: true,
6868
isTooltip: false
6969
}
70+
this._id = uid()
71+
// eslint-disable-next-line no-param-reassign
72+
options.regionId = this._id
7073
this._contextElement = element
7174
this._screenReaderFocusRegion = new ScreenReaderFocusRegion(
7275
element,
7376
options
7477
)
7578
this._keyboardFocusRegion = new KeyboardFocusRegion(element, options)
76-
this._id = uid()
7779
}
7880

7981
updateElement(element: Element | Node, options?: FocusRegionOptions) {
@@ -211,17 +213,15 @@ class FocusRegion {
211213
}
212214

213215
deactivate({ keyboard = true }: { keyboard?: boolean } = {}) {
214-
if (this._active) {
215-
this._listeners.forEach((listener) => {
216-
listener.remove()
217-
})
218-
this._listeners = []
219-
if (keyboard) {
220-
this._keyboardFocusRegion.deactivate()
221-
}
222-
this._screenReaderFocusRegion.deactivate()
223-
this._active = false
216+
this._listeners.forEach((listener) => {
217+
listener.remove()
218+
})
219+
this._listeners = []
220+
if (keyboard) {
221+
this._keyboardFocusRegion.deactivate()
224222
}
223+
this._screenReaderFocusRegion.deactivate()
224+
this._active = false
225225
}
226226

227227
focus() {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,8 @@ export type FocusRegionOptions = {
8686
* Whether or not the element is a Tooltip
8787
*/
8888
isTooltip?: boolean
89+
/**
90+
* The ID of the `FocusRegion` this belongs to. Used only for debugging.
91+
*/
92+
regionId?: string
8993
}

packages/ui-dialog/src/Dialog/props.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,11 @@ const propTypes: PropValidators<PropKeys> = {
136136
/**
137137
* Whether or not the element is a Tooltip
138138
*/
139-
isTooltip: PropTypes.bool
139+
isTooltip: PropTypes.bool,
140+
/**
141+
* The ID of the `FocusRegion` this belongs to. Used only for debugging.
142+
*/
143+
regionId: PropTypes.string
140144
}
141145

142146
const allowedProps: AllowedPropKeys = [

0 commit comments

Comments
 (0)