Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Popup|Dialog): ESC should close opened Popup/Dialog if focus is lost and moved to body #1807

Merged
merged 16 commits into from
Aug 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Fix bidirectional `FocusZone` to land focus correctly on DOWN key press after series of UP arrow keys @sophieH29 ([#1794](https://github.com/stardust-ui/react/pull/1794))
- Fix `hand` icon in Teams theme @lucivpav ([#1782](https://github.com/stardust-ui/react/pull/1782))
- Use `target` from `Provider` in `ReactDOM.createPortal()` calls @layershifter ([#1810](https://github.com/stardust-ui/react/pull/1810))
- ESC key should close the last opened `Popup` or `Dialog` if body has focus @sophieH29 ([#1807](https://github.com/stardust-ui/react/pull/1807))

### Features
- Add `overwrite` prop to `Provider` @layershifter ([#1780](https://github.com/stardust-ui/react/pull/1780))
Expand Down
27 changes: 27 additions & 0 deletions e2e/tests/dialogInDialog-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,31 @@ describe('Dialog in Dialog', () => {
expect(await e2e.exists(outerHeader)).toBe(false)
expect(await e2e.exists(innerHeader)).toBe(false)
})

it('A click on content and pressing ESC button should close the last opened dialog', async () => {
await e2e.clickOn(outerTrigger) // opens dialog
expect(await e2e.exists(outerHeader)).toBe(true)

await e2e.clickOn(innerTrigger) // opens nested dialog
expect(await e2e.exists(innerHeader)).toBe(true)

await e2e.clickOn(innerHeader)

// check that focus moved to body after clicking on Dialog content
expect(await e2e.isFocused('body')).toBe(true)

// press ESC and check if nested dialog is closed and focus is on nested trigger
await e2e.pressKey('Escape')
expect(await e2e.exists(innerHeader)).toBe(false)
expect(await e2e.isFocused(innerTrigger)).toBe(true)

// click on dialog content to move focus to body
await e2e.clickOn(outerHeader)
expect(await e2e.isFocused('body')).toBe(true)

// press ESC again and check if the last dialog is closed and focus is on trigger
await e2e.pressKey('Escape')
expect(await e2e.exists(outerHeader)).toBe(false)
expect(await e2e.isFocused(outerTrigger)).toBe(true)
})
})
15 changes: 9 additions & 6 deletions e2e/tests/dialogInPopup-example.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,21 @@ export const selectors = {
dialogHeader: Dialog.slotClassNames.header,
dialogOverlay: Dialog.slotClassNames.overlay,
dialogTrigger: 'dialog-trigger',
popupContent: Popup.Content.className,
popupContent: 'popup-content',
popupTrigger: 'popup-trigger',
}

const DialogInPopupExample = () => (
<Popup
content={
<Dialog
cancelButton={{ content: 'Close', id: selectors.dialogCancel }}
header="A dialog"
trigger={<Button id={selectors.dialogTrigger} content="Open a dialog" />}
/>
<>
<div id={selectors.popupContent}>Popup content</div>
<Dialog
cancelButton={{ content: 'Close', id: selectors.dialogCancel }}
header="A dialog"
trigger={<Button id={selectors.dialogTrigger} content="Open a dialog" />}
/>
</>
}
trigger={<Button id={selectors.popupTrigger} content="Open a popup" />}
/>
Expand Down
29 changes: 28 additions & 1 deletion e2e/tests/dialogInPopup-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const dialogCancel = `#${selectors.dialogCancel}`
const dialogHeader = `.${selectors.dialogHeader}`
const dialogOverlay = `.${selectors.dialogOverlay}`
const dialogTrigger = `#${selectors.dialogTrigger}`
const popupContent = `.${selectors.popupContent}`
const popupContent = `#${selectors.popupContent}`
const popupTrigger = `#${selectors.popupTrigger}`

// https://github.com/stardust-ui/react/issues/1674
Expand Down Expand Up @@ -48,4 +48,31 @@ describe('Dialog in Popup', () => {
expect(await e2e.exists(popupContent)).toBe(true)
expect(await e2e.exists(dialogHeader)).toBe(false)
})

it('A click on content and pressing ESC button should close the first opened dialog', async () => {
await e2e.clickOn(popupTrigger) // opens popup
await e2e.clickOn(dialogTrigger) // opens dialog

expect(await e2e.exists(popupContent)).toBe(true)
expect(await e2e.exists(dialogHeader)).toBe(true)

await e2e.clickOn(dialogHeader)

// check that focus moved to body after clicking on Dialog content
expect(await e2e.isFocused('body')).toBe(true)

// press ESC and check if nested popup is closed and focus is on nested trigger
await e2e.pressKey('Escape')
expect(await e2e.exists(dialogHeader)).toBe(false)
expect(await e2e.isFocused(dialogTrigger)).toBe(true)

// click on popup content to move focus to body
await e2e.clickOn(popupContent)
expect(await e2e.isFocused('body')).toBe(true)

// press ESC again and check if the last popup is closed and focus is on trigger
await e2e.pressKey('Escape')
expect(await e2e.exists(popupContent)).toBe(false)
expect(await e2e.isFocused(popupTrigger)).toBe(true)
})
})
26 changes: 26 additions & 0 deletions e2e/tests/popupInPopup-example.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import * as React from 'react'
import { Button, Popup } from '@stardust-ui/react'

export const selectors = {
popupContentId: 'popup-content-id',
popupTriggerId: 'popup-trigger-id',
popupContentNestedId: 'popup-content-nested-id',
popupTriggerNestedId: 'popup-trigger-nested-id',
}

const PopupInPopupExample = () => (
<Popup
content={
<>
<div id={selectors.popupContentId}>content</div>
<Popup
content={<div id={selectors.popupContentNestedId}>nested content</div>}
trigger={<Button id={selectors.popupTriggerNestedId} content="Open a nested popup" />}
/>
</>
}
trigger={<Button id={selectors.popupTriggerId} content="Open a popup" />}
/>
)

export default PopupInPopupExample
39 changes: 39 additions & 0 deletions e2e/tests/popupInPopup-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { selectors } from './popupInPopup-example'

const popupTrigger = `#${selectors.popupTriggerId}`
const popupContent = `#${selectors.popupContentId}`
const popupTriggerNested = `#${selectors.popupTriggerNestedId}`
const popupContentNested = `#${selectors.popupContentNestedId}`

describe('Popup in Popup', () => {
beforeEach(async () => {
await e2e.gotoTestCase(__filename, popupTrigger)
})

it('A click on content and pressing ESC button should close the last opened popup', async () => {
await e2e.clickOn(popupTrigger) // opens popup
expect(await e2e.exists(popupContent)).toBe(true)

await e2e.clickOn(popupTriggerNested) // opens nested popup
expect(await e2e.exists(popupContentNested)).toBe(true)

await e2e.clickOn(popupContentNested)

// check that focus moved to body after clicking on Popup content
expect(await e2e.isFocused('body')).toBe(true)

// press ESC and check if nested popup is closed and focus is on nested trigger
await e2e.pressKey('Escape')
expect(await e2e.exists(popupContentNested)).toBe(false)
expect(await e2e.isFocused(popupTriggerNested)).toBe(true)

// click on popup content to move focus to body
await e2e.clickOn(popupContent)
expect(await e2e.isFocused('body')).toBe(true)

// press ESC again and check if the last popup is closed and focus is on trigger
await e2e.pressKey('Escape')
expect(await e2e.exists(popupContent)).toBe(false)
expect(await e2e.isFocused(popupTrigger)).toBe(true)
})
})
20 changes: 20 additions & 0 deletions packages/react/src/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as customPropTypes from '@stardust-ui/react-proptypes'
import * as _ from 'lodash'
import * as PropTypes from 'prop-types'
import * as React from 'react'
import * as keyboardKey from 'keyboard-key'

import {
UIComponentProps,
Expand Down Expand Up @@ -203,6 +204,19 @@ class Dialog extends AutoControlledComponent<WithAsProp<DialogProps>, DialogStat
}
}

handleDocumentKeydown = (getRefs: Function) => (e: KeyboardEvent) => {
// if focus was lost from Dialog, for e.g. when click on Dialog's content
// and ESC is pressed, the opened Dialog should get closed and the trigger should get focus
const lastOverlayRef = getRefs().pop()
const isLastOpenedDialog: boolean =
lastOverlayRef && lastOverlayRef.current === this.overlayRef.current

if (keyboardKey.getCode(e) === keyboardKey.Escape && isLastOpenedDialog) {
this.handleDialogCancel(e)
_.invoke(this.triggerRef, 'current.focus')
}
}

renderComponent({ accessibility, classes, ElementType, styles, unhandledProps }) {
const {
actions,
Expand Down Expand Up @@ -312,6 +326,12 @@ class Dialog extends AutoControlledComponent<WithAsProp<DialogProps>, DialogStat
type="click"
capture
/>
<EventListener
listener={this.handleDocumentKeydown(getRefs)}
targetRef={targetRef}
type="keydown"
capture
/>
</>
)}
</Unstable_NestingAuto>
Expand Down
11 changes: 11 additions & 0 deletions packages/react/src/components/Popup/Popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,17 @@ export default class Popup extends AutoControlledComponent<PopupProps, PopupStat
if (isMatchingKey && this.isOutsidePopupElementAndOutsideTriggerElement(getRefs(), e)) {
this.trySetOpen(false, e)
}

// if focus was lost from Popup and moved to body, for e.g. when click on popup content
// and ESC is pressed, the last opened Popup should get closed and the trigger should get focus
const lastContentRef = getRefs().pop()
const isLastOpenedPopup: boolean =
lastContentRef && lastContentRef.current === this.popupDomElement
const bodyHasFocus: boolean = document.activeElement === document.body

if (keyCode === keyboardKey.Escape && bodyHasFocus && isLastOpenedPopup) {
this.close(e, () => _.invoke(this.triggerFocusableDomElement, 'focus'))
}
}

isOutsidePopupElementAndOutsideTriggerElement(refs: NodeRef[], e) {
Expand Down