-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Hookify TextField, Toast, and TopBar examples #2135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
LauraAubin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch more cases where we can simplify toggles and avoid creating two functions over time
src/components/Toast/README.md
Outdated
| function ToastExample() { | ||
| const [active, setActive] = useState(false); | ||
|
|
||
| const toggleActive = useCallback(() => setActive(!active), [active]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const toggleActive = useCallback(() => setActive(!active), [active]); | |
| const toggleActive = useCallback(() => setActive((state) => !state), []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding, how does this not modify all of the values in state if we only want to change active?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, I assumed set* didn't accept functions or callbacks. Guess I was wrong about the former 😄 I'll comb through my other examples and update them where it's needed.
For my understanding, how does this not modify all of the values in state if we only want to change active?
When using hooks your able to have a single state opposed to state objects in class-based components. If you do decide to use an object to hold state using useState, you won't be able to partially update state as you can in class base components.
const [booleanValue, setBooleanValue] = useState(false)
const toggleBooleanValue = useCallback(() => setActive(booleanValue => !booleanValue), []);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crosslinking ya to the other PR where I mentioned this, which has some explanations and links to docs #2126 (comment)
9f1fcfc to
e519bc8
Compare
e519bc8 to
b3ccf74
Compare
WHY are these changes introduced?
Convert examples to hooks
WHAT is this pull request doing?
Converting TextField, Toast, and TopBar class-based examples to hooks
STILL TO DO
I'm making quite a few pr's converting examples to hooks so I anticipate lots of
unreleased.mdconflicts, so I'll add the entry before I merge.How to 🎩
Make sure the behavior of the example mirrors the style guide
🎩 checklist
README.mdwith documentation changes