-
Notifications
You must be signed in to change notification settings - Fork 367
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
refactor: [M3-7433] - Query Key Factory for Images #10302
refactor: [M3-7433] - Query Key Factory for Images #10302
Conversation
Coverage Report: ✅ |
export const isInProgressEvent = (event: Event) => { | ||
const { percent_complete } = event; | ||
if (percent_complete === null || isLongPendingEvent(event)) { |
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.
We were not considering image_upload
an in progress event, which caused us to not poll for it. We probably did this in the past to poll less when an upload was happening but we must this event so that an image upload can transition from Processing
to Ready
without the user needing to refresh the page.
The change basically removes the special logic for image_upload
to improve UX
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 wasn't able to replicate the |
In prod, I've noticed I can replicate this with OBJ as well as Upload Image. Screen.Recording.2024-03-22.at.9.46.42.AM.mov |
Screen.Recording.2024-03-22.at.1.18.53.PM.movScreen.Recording.2024-03-22.at.1.20.04.PM.mov@mjac0bs @abailly-akamai I'm confused |
@bnussman-akamai Seems to be just Chrome. Just tested Safari and Firefox and had no issues. 😅 Not sure what's going on... but I can create a ticket. Edit: wait, you were in chrome above, not safari, weren't you? Edit, edit: OH. Chrome was slightly out of date. cc @abailly-akamai - if you're not on Screen.Recording.2024-03-22.at.10.32.20.AM.mov |
@mjac0bs maybe we can create a ticket to take a look at this indeed. In any case it shouldn't block this ticket. |
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.
✅ All image create/upload/edit/delete actions from Images landing, table sorting, events, and Linode Create > Image flow seemed to be working as expected with the new query keys.
As far as the Browse Files button: possibly one of the packages we use for the upload component is incompatible with outdated Chrome. I don't know if this is a persistent issue with out-of-date Chrome and therefore if this is really worth investigating, but I created M3-7931 so it's in the backlog if the issue arises again or we want to look into if improvements are possible.
Description 📝
processing
toready
🐛How to test 🧪
As an Author I have considered 🤔