-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Offline Mode: Integrate "Media Uploads" with a sync engine for drafts #23103
Offline Mode: Integrate "Media Uploads" with a sync engine for drafts #23103
Conversation
worker.setLongerDelay() | ||
} | ||
|
||
let delay = worker.nextRetryDelay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's safer to retry (but with a longer delay) than to never retry again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also helps if you have a draft post where sync is failing due to the media uploads and you go to "Context Menu / Publish / Media Uploads" and remove the upload for there – it doesn't trigger re-sync (now that I wrote it maybe it also should?)
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
Hi @kean 👋 , I'm bumping this PR's milestone to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit unexpected for the warning icon to still be there after I take an action to cancel the upload. What do you think of showing a spinner?
1541b73
to
4dba16a
Compare
I'm going to move the target back to 24.9 – it's not needed for the current release, and there are some enhancements still needed. |
998a240
to
d70876e
Compare
I agree, and I think it's not the only scenario where the spinner is missing. I changes the
|
There needed to be a way to somehow inform the user why the sync fails in case of a terminal error with media uploads.
To test:
var maxUploadSize: NSNumber?
)Screen.Recording.2024-04-26.at.11.18.36.AM.mov
Regression Notes
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: