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

Improve upload on background #10981

Merged
merged 11 commits into from
Feb 18, 2019
Merged

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Feb 8, 2019

Related GB-Mobile PR: wordpress-mobile/gutenberg-mobile#576

This PR improves the use of thumbnails for image uploads to use the local cache images for Media. This is especially important when we have error states on background uploads.

To test:

  • Create a post in GB
  • Add an image to it
  • Publish while the upload is going on
  • Go back to post and see if the image shows up correctly
  • Repeat but simulate an error while the upload is going on.

@SergioEstevao SergioEstevao added Media Gutenberg Editing and display of Gutenberg blocks. labels Feb 8, 2019
@SergioEstevao SergioEstevao added this to the 11.8 milestone Feb 8, 2019
@SergioEstevao SergioEstevao changed the title Issue/gb improve upload on background Improve upload on background Feb 8, 2019
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Hey @SergioEstevao - With these changes, upload sync feels more robust, and I appreciate the higher thumb quality. 😁

I played quite a lot with it and it mostly behaves super good.
After a lot of making the upload fail, killing the app while updating and retrying many times, I managed to put the upload on a weird state, where the cell shows upload progress, but the image doesn't.

Here is a video showing this:
https://cloudup.com/csFj1y5noGl

But again, this happen after trying really hard to break it, and in general it works good!

Let me know if this issue is related to this PR or if you prefer to check it out on another PR to ✅ this one.

@SergioEstevao
Copy link
Contributor Author

Hey @SergioEstevao - With these changes, upload sync feels more robust, and I appreciate the higher thumb quality. 😁

I played quite a lot with it and it mostly behaves super good.
After a lot of making the upload fail, killing the app while updating and retrying many times, I managed to put the upload on a weird state, where the cell shows upload progress, but the image doesn't.

Here is a video showing this:
https://cloudup.com/csFj1y5noGl

But again, this happen after trying really hard to break it, and in general it works good!

Let me know if this issue is related to this PR or if you prefer to check it out on another PR to ✅ this one.

I don't think it's related to this PR, it's more about Media states. It looks that sometimes the Media objects can get saved in incorrect states that makes think it's uploaded but it isn't.

@etoledom etoledom modified the milestones: 11.8, 11.9 Feb 11, 2019
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

I don't think it's related to this PR

Cool to know that! :)

Let's wait until the WPiOS code freeze before merging this PR. It should be today.
I changed the milestone to 11.9.
cc @loremattei

@loremattei
Copy link
Contributor

Thanks @etoledom! 10.8 code freeze is complete now, so you can merge this when you want.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 18, 2019

Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page.

Generated by 🚫 dangerJS

@SergioEstevao SergioEstevao merged commit 2d9a1d9 into develop Feb 18, 2019
@SergioEstevao SergioEstevao deleted the issue/gb_improve_upload_on_background branch February 18, 2019 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Editing and display of Gutenberg blocks. Media
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants