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

[WIP] Initial implementation of Modal on Android #5320

Closed
wants to merge 6 commits into from
Closed

[WIP] Initial implementation of Modal on Android #5320

wants to merge 6 commits into from

Conversation

satya164
Copy link
Contributor

The <Modal /> component has the same API as on iOS - http://facebook.github.io/react-native/docs/modal.html#content

The animated prop is not implemented yet.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @mkonicek, @ajwhite and @bestander to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 14, 2016
@@ -0,0 +1,72 @@
package com.facebook.react.views.modalhost;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add a license header please

@satya164
Copy link
Contributor Author

@mkonicek Done :)

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@mkonicek
Copy link
Contributor

Wow that was fast! I assume I'll have to update some internal Buck files in order to merge this, let's see :)

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/741802762587821/int_phab to review.

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@satya164
Copy link
Contributor Author

Import failed? @mkonicek

@mkonicek
Copy link
Contributor

Yup need to update BUCK files.

@satya164
Copy link
Contributor Author

Resolved conflicts.

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@Ehesp
Copy link
Contributor

Ehesp commented Jan 15, 2016

Nice work :)

@mkonicek
Copy link
Contributor

Cool, will update the Buck files and merge this. I'm doing an internal hackathon today with Shawn Wilsher from the Buck team trying to get OSS RN to build with Buck. Will move the release to Monday.

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/741802762587821/int_phab to review.

@mkonicek
Copy link
Contributor

Does the Modal work well for you? I've added ModalExample to UIExplorerList.android.js, here's a screen recording: https://youtu.be/nRe_Clx4Y-k

@satya164
Copy link
Contributor Author

@mkonicek I had only tried closing the modal with back button. Will investigate what's happening. Thanks.

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@satya164
Copy link
Contributor Author

So, I've been trying to fix the <Modal /> the whole night. The issue is that the view inside the modal doesn't receive touch. I think this is because the createViewInstance method returns a ReactModalHostView which is initialized and configured to receive touches and stuff, but it's not really used anywhere. A different ReactViewGroup instance is created which is added to the dialog, and it's not configured to respond to touches.

I cannot add the instance of the class to the dialog either, because that instance is appended to a parent view on it's creation

@satya164
Copy link
Contributor Author

cc @AaaChiuuu

@mkonicek
Copy link
Contributor

Thanks for looking into it and sorry about that, I assumed it worked out of the box!

@satya164
Copy link
Contributor Author

@mkonicek Me too. Let's get it fixed. It's gonna save me a bunch of really ugly code :D

@satya164 satya164 changed the title Initial implementation of Modal on Android [WIP] Initial implementation of Modal on Android Jan 17, 2016
@mpretty-cyro
Copy link

@satya164 @mkonicek has there been any movement on this? I'm about to start adding in some platform checks to avoid attempting to display UI modally but would love to avoid having to do that if this PR is going to drop some time soon.

@satya164
Copy link
Contributor Author

@mpretty-homepass Nope. There are some technical difficulties we need to handle before this can be merged. It might take some time.

@mpretty-cyro
Copy link

@satya164 Ok I'll set Android up to push all modals for the time being, thanks for the update.

@satya164
Copy link
Contributor Author

<Modal /> was open sourced in db7a154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants