Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Missing options props #35

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

Merged
MoOx merged 12 commits into rescript-react-native:master from Freddy03h:missing-options-props
Aug 11, 2020

Conversation

@Freddy03h
Copy link
Member

@Freddy03h Freddy03h commented Jul 8, 2020

Add and removing props and options regarding react-navigation v5 documentation.

idkjs reacted with rocket emoji
Copy link
Member

@MoOx MoOx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Are you confident with this improvements (= should we cut another release?)
Also could you specify the version of react-navigation you refered to somewhere?
Maybe update this https://github.com/reason-react-native/reason-react-navigation/blob/master/package.json#L5 & add other packages so we people know better the version?

Copy link
Member Author

I tried those changes on my own project this week-end, it work well :)

  • fix tabBar : function that return an element instead of being a component (my previous PR about it was a bit rushed ^^' )
  • add the view MaterialTopTabBar in MaterialTopTabs
  • add and remove many props and options on MaterialTopTabs, BottomTabs and Stack

For the peerDependencies I think we should add all the dependencies? But at least remove /core because it's now recommended to not add it (only add /native). Should I replace it with native only or should I add all other dep ? (stack, tabs, drawer, etc)

Copy link
Member Author

I didn't tried all the new props/options, but I'm pretty confident about the ones I removed from MaterialTopTabs

Copy link
Member

MoOx commented Jul 15, 2020

For the peer deps yes, please do what is recommended, so only what user should have as deps :) And all of them. Worst case scenario is that you get a warning for a dep you don't use, that's acceptable for now

As soon as you do that, I will adjust the version according to this... Since we don't "match" a single dep, the version number will be different that what we do for other packages.

Copy link
Member Author

Updated peerDependencies to this:

 "peerDependencies": {
 "@react-navigation/native": "^5.0.0",
 "@react-navigation/bottom-tabs": "^5.0.0",
 "@react-navigation/stack": "^5.0.0",
 "@react-navigation/material-top-tabs": "^5.0.0",
 "@react-navigation/material-bottom-tabs": "^5.0.0",
 "@react-navigation/drawer": "^5.0.0",

Regarding all packages changelog, it almost bugfixes, and features are almost typescript definitions and listeners things. There is a somes features linked to a specific minor version of a packaged but we don't supported them yet. So I think it's better to keep the 5.0.0 for the minimum version.

Copy link
Member

MoOx commented Jul 19, 2020

Your PR is still in Draft, do you have other things to push or is it ready as it is?

Freddy03h reacted with thumbs up emoji

@Freddy03h Freddy03h marked this pull request as ready for review July 19, 2020 14:06
Copy link
Member Author

Nothing else to push for now ^^

@MoOx MoOx merged commit 49d5c87 into rescript-react-native:master Aug 11, 2020
Copy link
Member Author

Thank you!

idkjs reacted with rocket emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MoOx MoOx MoOx approved these changes

@idkjs idkjs idkjs approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /