Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

Upgrade focus trap to v8.0.0 #3490

Closed
wants to merge 8 commits into from
Closed

Upgrade focus trap to v8.0.0 #3490

wants to merge 8 commits into from

Conversation

saket2403
Copy link
Contributor

Summary

Upgrade focus trap to v8.0.0

Closes #3388

Deployment Link

https://terra-core-deployed-pr-#.herokuapp.com/

Testing

Additional Details

Thank you for contributing to Terra.
@cerner/terra

@ryanthemanuel ryanthemanuel temporarily deployed to terra-core-focus-trap-vpiqj4o9 October 1, 2021 06:41 Inactive
@ryanthemanuel ryanthemanuel temporarily deployed to terra-core-focus-trap-vpiqj4o9 October 1, 2021 06:42 Inactive
@saket2403 saket2403 self-assigned this Oct 4, 2021
@benbcai
Copy link
Contributor

benbcai commented Oct 4, 2021

Do we know if bumping focus-trap-react from v6 to v8 on terra-dropdown-button and terra-overlay have any negative impact on consumers of these two components? In looking at the focus-trap-react v7 and v8 changelogs, it's hard to know what those breaking changes really impact.

@ryanthemanuel ryanthemanuel temporarily deployed to terra-core-focus-trap-vpiqj4o9 October 6, 2021 05:05 Inactive
@saket2403
Copy link
Contributor Author

saket2403 commented Oct 6, 2021

Do we know if bumping focus-trap-react from v6 to v8 on terra-dropdown-button and terra-overlay have any negative impact on consumers of these two components? In looking at the focus-trap-react v7 and v8 changelogs, it's hard to know what those breaking changes really impact.

There will be no impacts on consumers. The only impact it had has been handled in this PR which was caused by the PropTypes introduced in one of the Major versions. @benbcai

@ryanthemanuel ryanthemanuel temporarily deployed to terra-core-focus-trap-vpiqj4o9 October 8, 2021 13:28 Inactive
@jmsv6d
Copy link
Contributor

jmsv6d commented Oct 8, 2021

It looks like the focus behavior has changed in the terra-dropdown-button from the screenshots. What is going on there. Is that desired?

@benbcai
Copy link
Contributor

benbcai commented Oct 13, 2021

Consumers reported the following issue in terra-popup after bumping focus-trap-react from v6 to v8. Should we evaluate what impact terra-overlay and terra-dropdown-button may have before merging and releasing this change?

focus-trap.js?b846:291 Uncaught Error: Your focus-trap must have at least one container with at least one tabbable node in it at all times
    at updateTabbableNodes (focus-trap.js?b846:291)
    at Object.activate (focus-trap.js?b846:563)
    at FocusTrap.setupFocusTrap (focus-trap-react.js?fe41:190)
    at FocusTrap.componentDidMount (focus-trap-react.js?fe41:202)
    at commitLifeCycles (react-dom.development.js?61bb:19814)
    at commitLayoutEffects (react-dom.development.js?61bb:22803)
    at HTMLUnknownElement.callCallback (react-dom.development.js?61bb:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js?61bb:237)
    at invokeGuardedCallback (react-dom.development.js?61bb:292)
    at commitRootImpl (react-dom.development.js?61bb:22541)
react_devtools_backend.js:2526 The above error occurred in the <FocusTrap> component:
    in FocusTrap (created by PopupContent)
    in PopupContent (created by Popup)
    in Portal (created by Hookshot)
    in Hookshot (created by Popup)
    in Popup (created by Menu)
    in Menu (created by ConditionPopup)

@benbcai benbcai temporarily deployed to terra-core-focus-trap-7xobu0k6 November 9, 2021 19:59 Inactive
@benbcai benbcai temporarily deployed to terra-core-focus-trap-onogg3it November 16, 2021 15:01 Inactive
@stale
Copy link

stale bot commented Jan 15, 2022

This issue has been automatically marked as inactive because it has not had recent activity. It will be closed in seven days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive label Jan 15, 2022
@stale stale bot closed this Jan 22, 2022
@xenoworf xenoworf deleted the focus-trap branch March 10, 2022 21:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[spike] Update to focus-trap-react v8 in terra-core
6 participants