-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Header related updates. New useSafeAreaInsetsStyle hook. Documentation updates.
#2307
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
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.
Nice work! The changes look good to me!
I'd vote to make a hooks dir and move the following from utils there:
- useHeader.tsx
- useSafeAreaInsetsStyle.ts
- isMounted.ts and rename to useIsMounted.ts
and maybe add an index.ts for exports?
Low gafo but utils I think of storage, date functions etc and feel like hooks is more direct where to find them.
Thoughts?
@frankcalise I have a low-gafo too. I prefer to have the fewest amount of folders possible. With the |
| await episodeStore.fetchEpisodes() | ||
| setIsLoading(false) | ||
| })() | ||
| }, [episodeStore]) |
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.
Not relevant to this PR, but this makes me think, wouldn't it be nice to have a useLoadingEffect custom hook?
Something like...
const loading = useLoadingEffect(async () => {
await episodeStore.fetchEpisodes()
}, [episodeStore])It then handles the isLoading state under the hood, maybe.
Just TOL, not for this PR.
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 lot of querying libraries already do that. mst-query, react-query, urql, etc.. Since we don't use that, it might be cool to have a generic hook for any promises (something like this: https://github.com/bsonntag/react-use-promise#usage).
Also, isn't react coming out with use() hook that will have something simiarl?
jamonholmgren
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.
Looks great. One comment about the useHeader example.
| useHeader({ | ||
| headerShown: true, | ||
| header: () => <Header title="Hello" />, | ||
| }) |
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.
| useHeader({ | |
| headerShown: true, | |
| header: () => <Header title="Hello" />, | |
| }) | |
| useHeader({ | |
| title: "Hello", | |
| }) |
The API of this useHeader is you just pass in the props, I believe.
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.
You're right, my mistake.. copy pasting stuff is hard.
| @@ -0,0 +1,34 @@ | |||
| # Collection of helpful utilities | |||
|
|
|||
| Here you can find a library of utilities that are used often within your application. This includes hooks, helper functions, and various tools. | |||
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.
I miss the humor we used to inject in these things.
| Here you can find a library of utilities that are used often within your application. This includes hooks, helper functions, and various tools. | |
| Here you can find a library of utilities that are used often within your application next to the stapler, tape, scissors and migraine meds. This includes hooks, helper functions, and that thing from that wedding. |
(Don't accept this...lol)
|
🎉 This PR is included in version 8.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Please verify the following:
yarn testjest tests pass with new tests, if relevantREADME.mdhas been updated with your changes, if relevantDescribe your PR
useSafeAreaInsetPaddinghook into a newuseSafeAreaInsetsStyleshared hook.useSafeAreaInsetsStylehook instead of<SafeAreaView/>.useHeaderdocumentation.useSafeAreaInsetsStyledocumentation.Headerdocumentation with "Usage" examples.Screendocumentation.Closes #2300. Thanks to @ryskin for the ideas.
Proofs
useSafeAreaInsetsStyleDocumentationuseHeaderdocumentationHeaderusage documentation