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

Update AlertIOS.js #5789

Closed
wants to merge 1 commit into from
Closed

Update AlertIOS.js #5789

wants to merge 1 commit into from

Conversation

39otrebla
Copy link

When using AlertIOS.prompt() the developer can't choose the input type, thus being forced to use 'plain-text'. This simple change allows to set input type ('secure-text' or 'login-password'), keeping as default 'plain-text'.

When using AlertIOS.prompt() the developer can't choose the input type, thus being forced to use 'plain-text'. This simple change allows to set input type ('secure-text' or 'login-password'), keeping as default 'plain-text'.
@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @corbt, @ericvicenti and @christopherdro to be potential reviewers.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 6, 2016
@janicduplessis
Copy link
Contributor

Actually is this already implemented? AlertIOS was changed 16 days ago. Can you take a look at this commit ba4101d

@39otrebla
Copy link
Author

Not sure of how recently it has been implemented, but for sure it is not implemented until v0.19.0, which is the latest version available for installation through npm: https://github.com/facebook/react-native/blob/v0.19.0/Libraries/Utilities/AlertIOS.js

@janicduplessis
Copy link
Contributor

You are right it is not in v0.19.0 but it will be in v0.20.0

@janicduplessis
Copy link
Contributor

Oh, you are trying to merge your changes in the facebook:0.17-stable branch instead of master, this is why the changes don't show in your diff.

@corbt
Copy link
Contributor

corbt commented Feb 6, 2016

@39otrebla so it looks like you're basing your pull request off the 0.17 branch, instead of master. React Native moves really quickly, which is great because lots of new functionality is included in each version but can make it difficult to keep track of the current state of things!

I updated the AlertIOS API a few weeks ago (#5286) and revamped it to match the change you're proposing, as well as a few other rationalizations of the API. It didn't quite make the cut for 0.19, but the updated version will be available in 0.20 (and the docs already include the new version). 0.20 will be released very soon, possibly today, so look forward to the new API!

@corbt corbt closed this Feb 6, 2016
@corbt
Copy link
Contributor

corbt commented Feb 6, 2016

Actually sorry, it's just the RC being released today, not 0.20 stable, but it still has this change and typically the RC releases have been pretty safe to use, so I recommend you switch to it if you need this functionality.

@39otrebla
Copy link
Author

... The fact is that Official Documentation follows v0.20, so people may be confused to see Input Type as prompt() parameter and then receive an error. And it's there since days ago... But v0.20 has not been released yet, so options are 2: update v0.19 (or 0.17 and 0.18) to be documentation-compliant or release v0.20 . Well, since I can't release v0.20, I just thought it would be good to update branch which is commonly used by developers.

@39otrebla
Copy link
Author

RC1 for production? Nope sorry, so please either merge my commit v0.19 or do your one if you prefer, but developers who use v0.17, v0.18 and v0.19 may need this feature.

@39otrebla 39otrebla deleted the patch-1 branch February 6, 2016 19:45
@39otrebla 39otrebla restored the patch-1 branch February 6, 2016 19:45
@janicduplessis
Copy link
Contributor

@39otrebla The documentation is based on master, this is an issue that is currently being worked on to add versioned doc. I don't think there are any plans to support older stable versions so if you really need the feature I suggest that you use a fork until 0.20.0 is released.

@39otrebla 39otrebla deleted the patch-1 branch February 6, 2016 20:32
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.

4 participants