Add MapView annotation callback when it gets / lost the focus #5167
Conversation
|
By analyzing the blame information on this pull request, we identified @nicklockwood, @ultralame and @tadeuzagallo to be potential reviewers. |
facebook-github-bot
added
GH Review: review-needed
CLA Signed
|
Might be better to add onFocus/onBlur events to the annotation object itself? |
|
Sounds good, will re-implement it. |
|
@jerolimov updated the pull request. |
|
I have pushed an annotation-based It is implement similar to |
christoph-jerolimov
changed the title
christoph-jerolimov
changed the title
| alert('Annotation gets focus'); | ||
| }, | ||
| onBlur: () => { | ||
| alert('Annotation lost focus'); |
|
@jerolimov updated the pull request. |
| @@ -17,6 +17,7 @@ | |||
|
|
|||
| var React = require('react-native'); | |||
| var { | |||
| Alert, | |||
There was a problem hiding this comment.
property Alert Property not found in Object.create
|
Just rebased on the current master.. |
| @@ -309,7 +310,7 @@ exports.examples = [ | |||
| rightCalloutView: ( | |||
| <TouchableOpacity | |||
| onPress={() => { | |||
| alert('You Are Here'); | |||
| Alert.alert('You Are Here'); | |||
There was a problem hiding this comment.
Why make this change?
There was a problem hiding this comment.
Isn't it recommended to use the Alert module instead of the polyfil?
There was a problem hiding this comment.
Not that I'm aware of.
There was a problem hiding this comment.
Ok, your decision. Revert or keep? 😏
There was a problem hiding this comment.
I'll probably revert it when I merge this. But for future reference, it's better to avoid making unrelated changes in PRs anyway, even if they are uncontroversial, as it makes it harder to review.
If we do ever want to move away from using alert(), we can just do it once as a codemod across the whole project.
|
This looks fine, I think. |
|
@facebook-github-bot shipit |
|
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1634599690135008/int_phab to review. |
|
@jerolimov updated the pull request. |
|
@jerolimov updated the pull request. |
|
@nicklockwood Sorry there was a mistakenly change on the RCTWebView in the last commit. I pushed a new version which remove this change again. Sorry for the trouble and thanks for your community work / merging my PR anyway. 👍 |
| return <AnnotationExample style={styles.map} annotation={{ | ||
| title: 'More Info...', | ||
| onFocus: () => { | ||
| alert('Annotation gets focus'); |
ghost
closed this
in
Summary: For my project it was required to receive a notification when the MapView annotation was deselected. So I renamed `onAnnotationPress` to `onAnnotationSelected` and added a new method `onAnnotationDeselected`, this names was "inspired" by the underlaying iOS API. The old API was still called and marked as deprecated. But maybe you have an idea for a better naming (onAnnotationFocus/-Blur?) -- or should a deselected call the press method again without an annotation (undefined)? Closes facebook#5167 Reviewed By: svcscm Differential Revision: D2869695 Pulled By: nicklockwood fb-gh-sync-id: 91795ac3f1e4533b250af8901534d8870729d9db
For my project it was required to receive a notification when the MapView annotation was deselected.
So I renamed
onAnnotationPresstoonAnnotationSelectedand added a new methodonAnnotationDeselected, this names was "inspired" by the underlaying iOS API. The old API was still called and marked as deprecated.But maybe you have an idea for a better naming (onAnnotationFocus/-Blur?) -- or should a deselected call the press method again without an annotation (undefined)?