fix(FocusManager): Replaces implementation with focus-trap-react #28
Conversation
JoaoTMDias
force-pushed
the
fix-issue-27-cant-re-assign-focus-to-another-focus-trap
branch
from
7fad3d7
to
45adc66
Compare
size-limit report 📦
|
| props: Omit<FocusManagerProps, "children">, | ||
| ref?: never, | ||
| ): UseFocusManagerAsProps; | ||
| export function useFocusManager<GenericElement extends HTMLElement | SVGElement>( |
There was a problem hiding this comment.
Was this a mistake? Exporting 3 functions with the name useFocusManager on the same file.
There was a problem hiding this comment.
Nope, it is a typescript feature called function overloading.
Basically, you can define multiple function signatures so that, depending on the input, the expected output is different.
For example, if you pass only the props, the hook behaves as if inside a component, otherwise it will behave like a hook that provides the FocusTrap functionality.
JoaoTMDias
force-pushed
the
fix-issue-27-cant-re-assign-focus-to-another-focus-trap
branch
from
45adc66
to
d6e08f9
Compare
In order to fix the bug reported on issue 2, this commit does a re-implementation of the FocusManager, by replacing the current functionality, with the functionality provided by the "focus-trap" package. In order to keep the same API as before, it makes them still available as before, but changes the default values for autoFocus, restoreFocus and contain props. It also exposes the "focus-trap" API on the "options". BREAKING CHANGE: autoFocus, restoreFocus and contain are now set to true by default Closes #27
JoaoTMDias
force-pushed
the
fix-issue-27-cant-re-assign-focus-to-another-focus-trap
branch
from
d6e08f9
to
ddf275f
Compare
|
I can’t approve here, but from my side, everything seems good to go! One suggestion for improvement would be to include a small video or screenshot of the demo. While this may not be necessary for this particular case, I believe it can always help people understand the context a bit better. This is just a personal preference, but I think it would be beneficial for future PRs. Nonetheless, great job!! |
# [3.0.0](v2.0.1...v3.0.0) (2024-07-15) ### Bug Fixes * **FocusManager:** Replaces implementation with focus-trap-react ([#28](#28)) ([19bc86c](19bc86c)), closes [#27](#27) ### BREAKING CHANGES * **FocusManager:** autoFocus, restoreFocus and contain are now set to true by default * **FocusManager:** the `useFocusManager` was reimplemented. Since we no longer use a React context state management solution to travel between elements, the hook was re-implemented as an optional way to facilitate the creation of a `FocusTrap` instance, but without using the provided element by the package.
|
🎉 This PR is included in version 3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Because
In order to fix the bug reported on issue 2
This Commit
Does a re-implementation of the
FocusManager, by replacing the current functionality, with the functionality provided by the "focus-trap-react" package.In order to keep the same API as before, it makes them still available as before, but changes the default values for
autoFocus,restoreFocusandcontainprops. It also exposes the "focus-trap" configuration API on theoptionsprop.As such, the component itself also gains a whole set of new functionalities, made possible by the
focus-trappackage.BREAKING CHANGE: autoFocus, restoreFocus and contain are now set to true by default
BREAKING CHANGE: the
useFocusManagerwas reimplemented. Since we no longer use a React context state management solution to travel between elements, the hook was re-implemented as an optional way to facilitate the creation of aFocusTrapinstance, but without using the provided element by the package.Closes #27