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

Implemented tracksViewChanges for Android #2477

Merged

Conversation

danielgindi
Copy link
Contributor

@danielgindi danielgindi commented Sep 8, 2018

Does any other open PR do the same thing?

No

What issue is this PR fixing?

This is related to the following issues:
#1870 #2105 #2048 #2082

How did you test this PR?

In my own project, with a progressing pie animation inside the marker.
I've also made sure that tracking stops when the marker is removed from the map etc.

Some background and explanation

iOS has the tracksViewChanges property which causes re-renders of the markers all the time, and slows down performance of the map greatly.
Still, it is very handy especially given the async nature of React.
As many times the first images cached is of a blank component, it is very useful to use tracksViewChanges=true and then set it to false when componentDidUpdate is hit.

Another issue is loading images. Assuming the image loading issue will be fixed one day - we will still want tracksViewChanges and disabled that when onLoadEnd hits.
We could even start with it as a false value, and then turn it on and off when the loading is done.

Now some people use several hacks for the image issue on Android, like actual changes to Fesco code and usage of SVG images inside the Marker. But in those cases it is still slightly unstable and sometimes blank - but just because of the asyncronisity of react. This will also solve that.

I've set a default fps of 25, which seemed reasonable to me. But based on user feedback this could be easily adjusted.

This will help avoid unstable rendering of markers,
 and even support some level of animation. (like in iOS)
@rborn
Copy link
Collaborator

rborn commented Sep 9, 2018

@alvelig LGTM but this is android , could you 🐽

@alvelig
Copy link
Contributor

alvelig commented Sep 9, 2018

LGTM

@alvelig alvelig merged commit e3cc67a into react-native-maps:master Sep 9, 2018
Stalder added a commit to 91team/react-native-maps that referenced this pull request Feb 5, 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

3 participants