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

fix(Popup): Focus the last focused element outside Popup on ESC #861

Merged
merged 6 commits into from
Feb 8, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add `black` and `white` options for the `color` prop of the `Label` component @mnajdova ([#855](https://github.com/stardust-ui/react/pull/855))
- Add `Flex` component @kuzhelov ([#802](https://github.com/stardust-ui/react/pull/802))

### Fixes
- Focus the last focused element which triggered `Popup` on ESC @sophieH29 ([#861](https://github.com/stardust-ui/react/pull/861))

<!--------------------------------[ v0.20.0 ]------------------------------- -->
## [v0.20.0](https://github.com/stardust-ui/react/tree/v0.20.0) (2019-02-06)
[Compare changes](https://github.com/stardust-ui/react/compare/v0.19.2...v0.20.0)
Expand Down
18 changes: 17 additions & 1 deletion packages/react/src/components/Popup/Popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,15 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
private outsideClickSubscription = EventStack.noSubscription

private triggerDomElement = null
// focusable element which has triggered Popup, can be either triggerDomElement or the element inside it
private triggerFocusableDomElement = null
private popupDomElement = null

private closeTimeoutId

protected actionHandlers: AccessibilityActionHandlers = {
closeAndFocusTrigger: e => {
this.close(e, () => _.invoke(this.triggerDomElement, 'focus'))
this.close(e, () => _.invoke(this.triggerFocusableDomElement, 'focus'))
e.stopPropagation()
},
close: e => this.close(e),
Expand Down Expand Up @@ -474,6 +476,10 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
}

private trySetOpen(newValue: boolean, eventArgs: any) {
// when new state 'open' === 'true', save the last focused element
if (newValue) {
this.updateTriggerFocusableDomElement()
}
this.trySetState({ open: newValue })
_.invoke(this.props, 'onOpenChange', eventArgs, { ...this.props, ...{ open: newValue } })
}
Expand All @@ -497,4 +503,14 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
onClose && onClose()
}
}

/**
* Save DOM element which had focus before Popup opens.
* Can be either trigger DOM element itself or the element inside it.
*/
private updateTriggerFocusableDomElement() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to introduce a new method for a single use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense, don't want to mess up tryOpen method and still remain better reading

this.triggerFocusableDomElement = this.triggerDomElement.contains(document.activeElement)
? document.activeElement
: this.triggerDomElement
}
}