Convert a few simple components to use hooks #2065
Conversation
BPScott
force-pushed
the
some-easy-hookifys
branch
from
3322a50 to
a6442c2
Compare
BPScott
force-pushed
the
some-easy-hookifys
branch
2 times, most recently
from
a3d727a to
5696e34
Compare
src/utilities/use-toggle.ts
Outdated
| return [state, toggle] as [typeof state, typeof toggle]; | ||
| } | ||
|
|
||
| export function useForcableToggle(initialState: boolean) { |
There was a problem hiding this comment.
I think this is a bit of a stretch, I would just do this manually in the file itself and I don't think it comes up often enough to warrant a custom hook. Idk, feels a bit weird, if others think it's fine I'm fine with it though
There was a problem hiding this comment.
I've saw cases where we want to force set a boolean toggle to false (e.g. when you click a modal's close button / leave focus in popovers.
I think this crops up often enough to make a general purpose thing.
| @@ -0,0 +1,24 @@ | |||
| import {useState, useCallback} from 'react'; | |||
|
|
|||
| export function useToggle(initialState: boolean) { | |||
There was a problem hiding this comment.
I can see why you did this, I just wonder it's worth it. It's simple but it requires knowledge about the hook and it's less declarative and just handling the toggle when needed. We aren't saving on much here imo.
There was a problem hiding this comment.
The same can be said for pretty much all of our custom hooks :)
IMO The value from these small utilities comes from encapsulation, that small amount of complexity saved adds up over the course of several components and its repeated presence means we'll become familiar with it over time for when we have a boolean toggle, which is pretty often.
There was a problem hiding this comment.
I think for a really common usecase like a toggle variable it's fine. I agree that every hook adds to the cognitive load needed to understand the codebase, and we should avoid them
Remove withAppProvider, and use hooks for the following components: ActionMenu/RollupAction Autocomplete Card EmptySearchResult Form SkeletonPage TopBar
BPScott
force-pushed
the
some-easy-hookifys
branch
from
e1e0a93 to
bdc3307
Compare
* Convert a few simple components to use hooks Remove withAppProvider, and use hooks for the following components: ActionMenu/RollupAction Autocomplete Card EmptySearchResult Form SkeletonPage TopBar * PR Feedback
3 participants
This PR is best viewed if you disable whitespace changes, as the hookification saves a level of indentation, which makes it look like i changed EVERYTHING
WHY are these changes introduced?
Starting down the path of removing withAppProvider usage into the new world of hooks.
Part of #1995
WHAT is this pull request doing?
Remove withAppProvider, and use hooks for the following components:
ActionMenu/RollupAction
Autocomplete
Card
EmptySearchResult
Form
SkeletonPage
TopBar
How to 🎩
<EmptySearchResult title="Title" description="Description" withIllustration />renders in playground