Skip to content

Improve Drawer #48

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 6, 2021
Merged

Improve Drawer #48

merged 1 commit into from
Dec 6, 2021

Conversation

reck753
Copy link
Contributor

@reck753 reck753 commented Nov 4, 2021

Hi!

I have changed few things:

  • add missing options
  • enable creating screen with children (ScreenWithCallback)
  • small improvements in existing options

Type reference:

NOTE: The only option I didn't add is header, but I left it commented out. I wasn't sure if I could do it properly even though I had a reference in Stack.

NOTE: These changes introduce a breaking change since I changed the type of drawerLabel to be able to accept string along the render fn.

@reck753
Improve Drawer:
b9f8e53 
- add missing options
- enable creating screen with children (`ScreenWithCallback`)
- small improvements in existing options
Freddy03h
Freddy03h approved these changes Nov 5, 2021
View reviewed changes
Copy link
Member

@Freddy03h Freddy03h left a comment

Choose a reason for hiding this comment

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

LGTM! I'm not using Drawer so I can't test this PR on a real project but if you done those improvements for your project it's because it was needed so it's enough for me :)

@Freddy03h Freddy03h requested a review from MoOx November 5, 2021 09:39
@reck753
Copy link
Contributor Author

reck753 commented Nov 5, 2021

LGTM! I'm not using Drawer so I can't test this PR on a real project but if you done those improvements for your project it's because it was needed so it's enough for me :)

Hi Freddy!
I needed most of these improvements, but I have tested all of them regardless.

MoOx
MoOx approved these changes Dec 6, 2021
View reviewed changes
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.

Not tested, but I will trust you on this.
Sorry for the horrible delay.
Will publish on npm asap.

@MoOx MoOx merged commit b539ea4 into rescript-react-native:main Dec 6, 2021
@MoOx
Copy link
Member

MoOx commented Dec 6, 2021

6.0.1 is out on npm!

@reck753
Copy link
Contributor Author

reck753 commented Dec 6, 2021

Not tested, but I will trust you on this. Sorry for the horrible delay. Will publish on npm asap.

Hi @MoOx !
No worries about the delay. Thanks for merging 😊

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

@Freddy03h Freddy03h Freddy03h 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.
3 participants
@reck753 @MoOx @Freddy03h