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

fix(FocusTrapZone): do not propagate any keyboard events #1180

Merged
merged 3 commits into from
Apr 5, 2019
Merged

fix(FocusTrapZone): do not propagate any keyboard events #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d7e506
fix(FocusTrapZone): do not propagate any keyboard events
Apr 5, 2019
36ee4a2
update changelogs
Apr 5, 2019
3bde189
Merge branch 'master' into fix/focus-trap-zone-do-not-propagate-keybo…
sophieH29 Apr 5, 2019
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Fix click triggering logic of `Space` and `Enter` keys for `MenuItem` @kuzhelov ([#1175](https://github.com/stardust-ui/react/pull/1175))
- Truncate `content` and `header` of `ListItem` when used from `DropdownSelectedItem` @silviuavram ([#1161](https://github.com/stardust-ui/react/pull/1161))
- Fix `rotate` prop on `Icon` not working in `rtl` @mnajdova ([#1179](https://github.com/stardust-ui/react/pull/1179))
- `FocusTrapZone` - Do not propagate any keyboard events @sophieH29 ([#1180](https://github.com/stardust-ui/react/pull/1180))
Copy link
Contributor

Choose a reason for hiding this comment

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

could you, please, provide some notes about resolution strategy in this PR's description - I mean, specifically

  • what was the cause of the issue
  • why this was considered as a fix

This description will help a lot to better understand why provided changes are considered to be a fix for the issue, as well as to retrospect this case if we will experience similar problem in future. Thank you!


<!--------------------------------[ v0.26.0 ]------------------------------- -->
## [v0.26.0](https://github.com/stardust-ui/react/tree/v0.26.0) (2019-04-03)
Expand Down
7 changes: 0 additions & 7 deletions packages/react/src/components/Popup/Popup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,6 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
const focusTrapProps = {
...(typeof accessibility.focusTrap === 'boolean' ? {} : accessibility.focusTrap),
...popupWrapperAttributes,
onKeyDown: (e: React.KeyboardEvent) => {
// No need to propagate keydown events outside Popup
// when focus trap behavior is used
// allow only keyboard actions to execute
_.invoke(accessibility.keyHandlers.popup, 'onKeyDown', e)
e.stopPropagation()
},
} as FocusTrapZoneProps

const autoFocusProps = {
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ This is a list of changes made to the Stardust copy of FocusTrapZone in comparis
### fixes
- Do not focus trigger on outside click @sophieH29 ([#627](https://github.com/stardust-ui/react/pull/627))
- Do not hide aria-live regions from accessibility tree @sophieH29 ([#917](https://github.com/stardust-ui/react/pull/917))
- Do not propagate any keyboard events @sophieH29 ([#1180](https://github.com/stardust-ui/react/pull/1180))

### features
- Add focus trap zone [#239](https://github.com/stardust-ui/react/pull/239)
- Used Stardust utils instead of Fabric utilities:
Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/lib/accessibility/FocusZone/FocusTrapZone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ export class FocusTrapZone extends React.Component<FocusTrapZoneProps, {}> {
private _onKeyboardHandler = (ev: React.KeyboardEvent<HTMLDivElement>): void => {
this.props.onKeyDown && this.props.onKeyDown(ev)

// do not propogate keyboard events outside focus trap zone
ev.stopPropagation()

if (
ev.isDefaultPrevented() ||
keyboardKey.getCode(ev) !== keyboardKey.Tab ||
Expand All @@ -212,11 +215,9 @@ export class FocusTrapZone extends React.Component<FocusTrapZoneProps, {}> {
if (ev.shiftKey && _firstTabbableChild === ev.target) {
focusAsync(_lastTabbableChild)
ev.preventDefault()
ev.stopPropagation()
} else if (!ev.shiftKey && _lastTabbableChild === ev.target) {
focusAsync(_firstTabbableChild)
ev.preventDefault()
ev.stopPropagation()
}
}

Expand Down