Skip to content
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

Specify slide out time, TypeScript typings #20

Merged
merged 9 commits into from
Jun 27, 2019
Merged

Specify slide out time, TypeScript typings #20

merged 9 commits into from
Jun 27, 2019

Conversation

danielholmes
Copy link
Contributor

Apologies for the multiple updates in one - I'll be happy to pick them apart individually if you're only interested in some. Updates here include:

  • Ability to pass a slideOutTime to the show method
  • Ability to override any of the styles via an optional style prop
  • TypeScript typings

Let me know your thoughts and if you want any modifications.

@carsonwah
Copy link
Owner

Sorry for the late reply and thanks for the PR!

slideOutTime, linting and typescript are all good. Great job! For the style part, I think there are too many customization props in this way. It will be cleaner and more intuitive if we let user pass their custom components instead of styling props. Let me know what you think.

@danielholmes
Copy link
Contributor Author

@carsonwah Yea that sounds good to me. How granular were you thinking? e.g. optional ContentTitle, ContentText, HeaderTime components. Or just a custom component that goes inside the <TouchableWithoutFeedback ? Or maybe a renderProp style approach

@carsonwah
Copy link
Owner

carsonwah commented Jun 22, 2019

@danielholmes I think we can take the approach from another PR #17. It takes a renderPopupContent function prop.

just a custom component that goes inside the <TouchableWithoutFeedback> <- exactly

@danielholmes danielholmes changed the title Custom styling, specify slide out time, TypeScript typings Specify slide out time, TypeScript typings Jun 23, 2019
@danielholmes
Copy link
Contributor Author

@carsonwah I've removed the custom styling/rendering from this PR - I didn't realise there was another open, so I'll leave that task to #17

@carsonwah carsonwah merged commit fd759a0 into carsonwah:master Jun 27, 2019
@carsonwah
Copy link
Owner

@danielholmes Merged. Thanks so much for the contribution. I will release a new version asap, maybe together with #17.

@carsonwah carsonwah mentioned this pull request Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants