Allow valid 0-value in renderUI of Progress widget #1173
Conversation
AFaust
force-pushed
the
Progress-nonTruthyValidProgress
branch
from
3a62e52
to
d6cf57a
Compare
|
I've checked out your branch locally and I've getting unit test failures. However, the unit test is quite old and may need some updating as it's not the best... I'll test for real on Share (using the download as zip action where this is used). If it works, then I'll update the unit test. |
|
Sadly, this doesn't appear to be working on Share either.... when downloading as a zip you do not see the progress bar update. I can't merge this as is - it looks like the unit test has done it's job here. I think (based on IRC comments) that you've had issues getting the the unit tests up and running, but it would definitely be useful. I'll have a dig into the issue to see if I can understand what the problem might be. |
| // 0 is a legal numeric value (e.g. initial "start" progress) | ||
| // can't use simple truthy vs non-truthy check | ||
| if (isNaN(total) || total < 0 || isNaN(done) || done < 0 || | ||
| isNaN(filesAdded) || filesAddded < 0 || isNaN(totalFiles) || totalFiles < 0) |
There was a problem hiding this comment.
I think the issue here is a simple typo... filesAddded needs to be filesAdded (you've got an extra "d" in there!)
|
I had planned to try one final time to get the whole node.js ... working while I am on this vacation. May as well be tomorrow then. |
|
If you can fix your typo I should be able to merge this - the unit tests will pass and I've verified with the download as zip action on Share as well. When I opened up the file in in Sublime, JSHint highlighted the error immediately. I'm not sure if you've tried Sublime, but it works really well for JS changes. Also, the setup instructions should get everything running for you... if they don't work for you then it would be good to know. We have had them verified in the past by Windows users. |
|
Thanks for making that update - I'll merge |
Currently the Progress widget is only displaying/tracking any progress if at least one "item" (may not be used for file-based actions) has been "done". The simple truthy-check within a condition of renderUI eliminates 0 as a valid value despite it being perfectly valid for an initial progress update at the start of a long running operation.
This PR allows 0-values in renderUI by adapting the check to consider all non-negative integers as valid and explicitly checking for NaN.