-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix mockup preview doesnt work when batch uploading #119
Fix mockup preview doesnt work when batch uploading #119
Conversation
The fix is to use a queue
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.
Thanks @pkong-ds, I found a small bug when testing, please check check
public/scripts/upload.js
Outdated
// observe fileListViewModel: imageUploads[].length | ||
// side effect: generate preview mockup | ||
mobx.reaction( | ||
() => viewModel.fileList.imageUploads.length, | ||
(newLen) => { | ||
if (viewModel.fileList.imageUploads.length !== newLen) { | ||
console.error("unexpected mobx error, image upload length not matched"); | ||
return; | ||
} | ||
const newImage = viewModel.fileList.imageUploads[newLen - 1]; | ||
viewModel.generatePreviewMockup(newImage); | ||
}, | ||
); | ||
|
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 will trigger generate preview for last image when removing uploaded images from, that's why I add viewModel.generatePreviewMockup(newImage);
in add function, maybe add check if newImage.previewURL != null
?
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.
Lemme check it with
if newLen > oldLen
maybe add check if newImage.previewURL != null?
I think this is workaround. This side effect should only be triggered by target observable.
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.
Updated, thanks yayun!
Preview
This is a batch job uploading 21 files.
However, there is another problem - the performance. It took ard 7 mins to generate all 21 previews
Preview - Upload
Screen.Recording.2024-08-27.at.7.28.16.PM.mp4
Preview - Preview
Screen.Recording.2024-08-27.at.7.31.18.PM.mp4