Doodlebug Fast follow tickets #836
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
Commits
|
|
Hi @arugupta1992 , since the PR has changes in common file transition-screen.js which is used for transition screen across all workflows, I am adding label platform so that a quick sanity can be done across all workflows. cc @sigadamvenkata @asonnalagi |
| } | ||
|
|
||
| clearProgressBarHandler() { | ||
| if (this.progressBarTimeoutId != null) { |
There was a problem hiding this comment.
Nit: consider using !== null here. != null works correctly in this case but strict equality is more explicit and avoids any potential eqeqeq lint rule failures.
| if (this.progressBarTimeoutId != null) { | |
| if (this.progressBarTimeoutId !== null) { |
| this.progressBarTimeoutId = null; | ||
| if (generation !== this.progressBarGeneration) return; | ||
| const v = initialize ? 0 : parseInt(progressBar.getAttribute('value'), 10); | ||
| if (v === 100) return; |
There was a problem hiding this comment.
Nit: the if (v === 100) return inside the setTimeout callback appears to be dead code post this update. if v were 100, updateProgressBar would have already called clearProgressBarHandler(), causing the generation check above it to exit first. Please check once and see if above understanding is correct and if we need to remove it.
MWPW-195943
MWPW-194444
MWPW-193717
MWPW-189974
Fix Explanantion for MWPW-189974
progressBarHandler used recursive setTimeout chains with no cleanup. After cancel, timers kept running and could overwrite the bar on a second upload or after updateProgressBar(100).
Fix (in transition-screen.js)
Test URLs: