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

Implement shouldUpdate to avoid unnecessary component remounts #3

Merged
merged 2 commits into from
May 15, 2019

Conversation

abdyer
Copy link

@abdyer abdyer commented May 15, 2019

I based this on the implementation for ImageSpec in the Litho repo. It's debatable which props should be included in the check, but hopefully this helps others avoid the image flicker I issue I ran into.

@pavlospt pavlospt requested a review from charbgr May 15, 2019 11:26
@pavlospt
Copy link
Collaborator

@abdyer The conversation on this issue with Andy Street involved the isPureRender=true as well. Isn't that needed anymore?

@abdyer
Copy link
Author

abdyer commented May 15, 2019

@pavlospt Thanks for the reminder. I had it in my working code, but we're not using this library directly so I forgot to include it here. Added.

@pavlospt
Copy link
Collaborator

@abdyer Great!

@charbgr
Copy link
Owner

charbgr commented May 15, 2019

Thanks @abdyer ! Looks good to me! I will merge it but I will not upload an artifact due to some bintray issues I face. I will try to solve them on the weekend! Maybe it will require to change the package name to solve the issue(maybe: com.github.charbgr:litho-picasso2:1.0)

I am also thinking of bumping the litho version and upload then the artifact.
What do you think @pavlospt @abdyer?

@charbgr charbgr merged commit 7589808 into charbgr:master May 15, 2019
@pavlospt
Copy link
Collaborator

@charbgr yeap bumping the version as well would be nice!

@charbgr
Copy link
Owner

charbgr commented May 27, 2019

Friendly ping.. It is up under com.github.charbgr:litho-picassox:1.0 and with AndroidX support! :) @pavlospt @abdyer

Thanks @abdyer for the contribution!

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.

3 participants