Implement preview mockup selector #97
Conversation
|
@pkong-ds I just add |
| @@ -29,6 +29,8 @@ async def upload_single_image(origin_image, file_name): | |||
|
|
|||
|
|
|||
| async def upload_file(): | |||
| from js import imageUpload | |||
|
|
|||
There was a problem hiding this comment.
Why need to move import here?
There was a problem hiding this comment.
@pkong-ds added comment in script
Since we will update
imageUploadwhen calling this function,
need to re-import it to force update to new value
or we will always generate first uploaded image
There was a problem hiding this comment.
Since we will update imageUpload when calling this function,
😱 which line update imageUpload? I view a bit, couldn't see
public/scripts/upload.js
Outdated
| setTimeout(() => { | ||
| alert("Generation failed, please reload and try again later"); | ||
| }, 100); |
There was a problem hiding this comment.
Why need setTimeout? If wanna manipulate event loop orders, can add comment to specify which event this wanna be behind of?
There was a problem hiding this comment.
Can use async/await for this?
There was a problem hiding this comment.
@YayunHuang seems still using setTimeout
public/scripts/upload.js
Outdated
| setTimeout(() => { | ||
| itemNode.classList.remove("file-list-item--done"); | ||
| itemNode.classList.add("file-list-item--loading"); | ||
| }, 10); |
There was a problem hiding this comment.
Timebox 30 min to remove progress bar, if not enough, open another issue for this
There was a problem hiding this comment.
seems did not remove progress bar. let's open issue to remove those unused code?
YayunHuang
force-pushed
the
mup-138-preview-mockup-selector
branch
from
2beac95 to
9099b4b
Compare
|
@pkong-ds all updated, please review again, thanks |
| // Avoiding read same image file | ||
| setTimeout(() => { | ||
| this._imageUploads.push(imageUpload); | ||
| window.viewModel.generatePreviewMockup(imageUpload); | ||
| }, i * 10); |
There was a problem hiding this comment.
why same image file will be read if without setTimeout?
|
@YayunHuang 4 unresolved comments, but I tested a bit seems feature works fine. Lemme merge first, see if you wanna fix those later |
Ref: MUP-138, MUP-150
Screen.Recording.2024-08-20.at.5.11.07.PM.mov
Screen.Recording.2024-08-20.at.5.18.27.PM.mov