Page MenuHomePhabricator

[web][fixed] Added FocusTrap for modals

Authored by kuba on Wed, May 24, 9:55 AM.
Referenced Files
F570482: D7960.diff
Sat, Jun 3, 6:28 AM
Unknown Object (File)
Thu, May 25, 4:43 AM
Unknown Object (File)
Wed, May 24, 12:03 PM



Diff which was reverted:
More info on why it was reverted:

Added fallbackFocus option, which sets focus to div when there are no focusable elements inside the FocusTrap.

Old description:
We want to prevent users from focusing outside the modals. Added focus-trap-react library, to do that, and covered the modals that we use.

Test Plan

Checked if in edit mode and in other modals user can't focus outside the modal using the tab.
Checked if the modals without focusable elements (i.g. in the investor's tab on the landing page) also work.


Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

kuba edited the test plan for this revision. (Show Details)
kuba added inline comments.
63 ↗(On Diff #27004)

It fixes the issue: Your focus-trap must have at least one container with at least one tabbable node in it at all times raised in ENG-3928, when user clicked on the investor without buttons.

Solution source:

kuba retitled this revision from [web] Added FocusTrap for modals to [web][fixed] Added FocusTrap for modals.Wed, May 24, 10:07 AM
kuba edited the test plan for this revision. (Show Details)
ashoat added inline comments.
63 ↗(On Diff #27004)

All object params should be memoized in function components

kuba requested review of this revision.Wed, May 24, 10:14 AM
ashoat requested changes to this revision.Wed, May 24, 11:14 AM

Requesting changes for comment above

This revision now requires changes to proceed.Wed, May 24, 11:14 AM
kuba marked an inline comment as done.
kuba edited the summary of this revision. (Show Details)

Memoize param

This revision is now accepted and ready to land.Thu, May 25, 3:58 AM