Fix #95 -- Emit upload progress signal

[schwartzmx]

Very open to changes / concerns with this route. It seemed there was something similar that was part of the original javascript in this middleware that would update progress, this one is a tiny bit different in that it is completely optional, and will only update a specific progress bar if it's id is s3UploadFileProgress (something we could document in the README). It will sum all ongoing file upload loaded and total event attributes for xhr.upload.onprogress listener and calculate a percentage of completion of the upload.

One thing with it being optional though is that if you did want a progress to be displayed, you'd need some HTML along the lines of this (which is using some very basic bootstrap classes/styling):

        <div id="s3UploadFileProgressBar" class="progress" style="height: 20px;">
          <div id="s3UploadFileProgress" class="progress-bar progress-bar-striped" role="progressbar" style="width:0%;" aria-valuenow="0" aria-valuemin="0" aria-valuemax="100"></div>
        </div>

(this is the one displayed below)

Would close #95 if we can find something that works.

Demo (using a small file for the sake of .gif length):

Do note: I have very little experience with javascript and am probably not following best practices here, but this is what I got to work and I think it works pretty well so far. This was a learning experience for me and I am open to changing any of this.

Looking for some feedback whenever you have some free time @codingjoe.

Thanks,
Phil

[codecov]

Codecov Report

Merging #96 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #96   +/-   ##
=======================================
  Coverage   98.68%   98.68%           
=======================================
  Files           4        4           
  Lines          76       76           
=======================================
  Hits           75       75           
  Misses          1        1

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53c5554...6b51dac. Read the comment docs.

[codingjoe]

Hi @schwartzmx,
This looks like a good start. Thank you!

There are some minor points that I would want to improve. Firstly I would like to keep this package very simple and extendible. I don't know if someone actually wants to display a progress bar or something else. Maybe this package should only emit a signal with the percentage in it, and leave the rest to the developers using it. (We'd add good documentation of course, with an example on how to consume the signal and create a progress bar.)

I think currently that actually does not work correctly for multiple file inputs or inputs with the multiple attribute set.

One last thing, I usually follow StandardJS. I don't know why it's currently not running on travis maybe you can check locally, that your JS style matches standardjs.

Keep up the good work! I really like that feature.

[schwartzmx]

I pushed some changes addressing most your current PR comments and changing a small bit of the functionality to keep it functionally working.

However, after re-reading what you said, I don't think we still want to go this route.

Maybe this package should only emit a signal with the percentage in it, and leave the rest to the developers using it. (We'd add good documentation of course, with an example on how to consume the signal and create a progress bar.)

I actually had one of my JS buddies from work take a look at what I've done and he actually made this same suggestion. He said it would make sense to have the developer (on your own client-side JS scripts, outside of the s3file.js script) define their own callback function that could be used to receive upload percentage change events and then have that function directly alter the styling instead of injecting this into the middleware JS.

Is this something along the lines you were thinking of?

For example in calculateProgressAndSetStyle, it would actually test for a function to be defined (this would be something defined on the developer's side JS) and it will call it passing it the fileUpload object, which they can then process the results on their end and set any styling they wish.

in s3file.js

 function calculateProgressAndSetStyle(formUpload) {
    const percentage = calculateFileUploadProgress(formUpload.fileProgress);
    const shouldEmit = formUpload.currPercentComplete !== percentage;
    formUpload.currPercentComplete = percentage;
    if (shouldEmit && typeof(someConstantDocumentedFunctionName) === typeof(Function)) {
        try { someConstantDocumentedFunctionName(formUpload); }
        catch (err) { console.log(err); }
    };
 }

in uploadpage.js (developer code)

  function someConstantDocumentedFunctionName(formUpload) {
    const progressBars = formUpload.form.querySelectorAll('#s3UploadFileProgress');
    if (progressBars && progressBars.length > 0) {
      progressBars.forEach(pb => {
        if (pb) {
          var pctTxt = formUpload.currPercentComplete.toString() + '%';
          // set the text value and progress style
          pb.innerHTML = pctTxt;
          pb.style.width = pctTxt;
        }
      });
    }
  }

Let me know your thoughts, thanks for the review!

[codingjoe]

Is this something along the lines you were thinking of?

@schwartzmx yes, precisely. Except using signals not try catch ;)

[schwartzmx]

@codingjoe pushed an updated commit with an attempt at implementing a signaler / emitter. This works but again I'm new to JS so I'm not 100% on if this is the correct way to implement this.

On the user side JS, you can implement a subscriber to the s3 emitter object,

  function uploadProgressBars(formObj) {
    const progressBars = formObj.form.querySelectorAll('#s3UploadFileProgress')
    if (progressBars && progressBars.length > 0) {
      progressBars.forEach(pb => {
        var pctTxt = formObj.percentComplete.toString() + '%'
        // set the text value and progress style
        pb.innerHTML = pctTxt
        pb.style.width = pctTxt
      })
    }
  }

  window.onload=function() {
    // subscribe to s3 file upload emitter
    s3FileUploadProgressEmitter.subscribe('s3-upload:upload-percentage-change', uploadProgressBars)
  }

given that the form upload has an id of s3-upload

        <form id="s3-upload" method="post" class="form" enctype="multipart/form-data" action="{% url 'idtr-ui-create-job' %}" onsubmit='toggleCreateButton(false); '>
          {% bootstrap_form form_job %}
          {% bootstrap_form form_upload %}
          {% buttons %}
            <a href="{% url 'idtr-ui-jobs' %}" class="btn btn-link" role="button">Cancel</a>
            <button id='createButton' type="submit" class="btn btn-primary" onclick="toggleProcessing(true, false);">Create</button>
          {% endbuttons %}
          <div id="s3UploadFileProgressBar" class="progress" style="height: 20px;">
            <div id="s3UploadFileProgress" class="progress-bar progress-bar-striped" role="progressbar" style="width:0%;" aria-valuenow="0" aria-valuemin="0" aria-valuemax="100"></div>
          </div>
        </form>

The formObj passed to the subscriber contains the form target, the percentComplete and the fileUploadProgress map for inputs.

I chose to use the form's id to uniquely identify forms and their associated upload progress, so you will see that being passed around between functions now and then looked up on the emitter when needed.

Looking for some more feedback when you have a chance.

Thanks

[codingjoe]

@schwartzmx love the effort you are putting into this. I would highly suggest tho, to take a look at JavaScript's Event framework, here is a good resource on how to implement custom events:
https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Creating_and_triggering_events#The_old-fashioned_way

[schwartzmx]

@codingjoe so you are suggesting I create a document event? And then on the user side they create a listener using some element on their side to listen for the specific event?

I was also looking at https://medium.com/@zandaqo/eventtarget-the-future-of-javascript-event-systems-205ae32f5e6b for extending EventTarget

I could extend that on the current class, and on the user side they can add a listener on the instantiated class (similar to what my latest commit allows)
specifically looking at this example

// This would be in s3file.js
class MyTarget extends EventTarget {}
const target = new MyTarget();


// this would be in user.js
target.addEventListener(
    'move',
    (event) => console.log(`moved ${event.detail.steps} steps`)
);



// this would be in s3file.js to emit percentage change
target.dispatchEvent(
    new CustomEvent('move', { detail: { steps: 2 }  })
);
//=> moved 2 steps

What do you suggest? Scrap the last commit and just use document event? Also what about having many events depending on the form?

Thanks

[codingjoe]

@schwartzmx I believe the event should be per input, not per form. That way you could display multiple progress bars for a single form. That might be handy. It also makes more sense, since the initialization of the upload helper is per input, not per form.