Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload #9019

noi5e · 2021-01-16T20:29:23Z

Fixes #9018 (see this issue page for a more detailed write-up & game plan)

This fixes the original bug AND I made a few extra changes to image upload CSS:

reset .progress-bar's CSS width to 0 after a fileupload, so it can be reused for a second upload.
hover class was not appearing (the div wasn't darkening) when user hovered an image over the .dropzone.
changed EDIT comment form's progress bar to be stripy, and animated.

div darkens on hover, then resets on dragout
upload bar resets to 0

(This issue is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)

gitpod-io · 2021-01-16T20:29:26Z

codecov · 2021-01-16T20:36:10Z

Codecov Report

❗ No coverage uploaded for pull request base (main@68c9022). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #9019   +/-   ##
=======================================
  Coverage        ?   82.03%           
=======================================
  Files           ?      100           
  Lines           ?     5933           
  Branches        ?        0           
=======================================
  Hits            ?     4867           
  Misses          ?     1066           
  Partials        ?        0

noi5e · 2021-01-17T05:26:02Z

app/assets/javascripts/dragdrop.js

@@ -17,44 +17,36 @@ jQuery(function() {
    );
  }

-  $('.dropzone').on('dragover',function(e) {
-    e.preventDefault();


I stuck these listeners in the $('.dropzone').each further down. Having them inside that function lets me use $(this) to attach an eventListener... Which reduces the number of times that the same eventListener is attached to the same element.

Amazing 👍

noi5e · 2021-01-17T05:27:04Z

app/assets/javascripts/dragdrop.js

+      e.preventDefault();
+      $(e.currentTarget).addClass('hover');
+    });
+


Changed dragover and dragout to dragenter and dragleave so they don't fire every time the user makes a movement with the image on the surface of the div.

wow good thinking 🎉

noi5e · 2021-01-17T05:29:00Z

app/assets/stylesheets/editor.css

    font-size: inherit;
    color: inherit;
    white-space: pre-wrap;
    background-color: transparent;
    border-radius: 0;
  }
-}



I believe this was an extra close curly-brace that may have been affecting the CSS! Also fixed the commenting before this block.

noi5e · 2021-01-17T05:36:49Z

app/assets/stylesheets/question.css

@@ -118,7 +118,6 @@ table th a:hover {
  visibility: hidden;
 }

-.question-show textarea,
 .question-show pre {
  margin: 0;


I removed this CSS reference to fix a styling issue in Edit Comment forms. I believe this was done originally to fix #1304, which I think was for the publiclab.org/questions route, so unrelated to comments on questions.

This is what the Edit Comment looked like before this change (no border color, no padding):

This is what it looks like after this fix:

noi5e · 2021-01-17T05:38:00Z

app/views/comments/_edit.html.erb

-        jQuery(document).ready(function() {
-          $E.initialize();
-        });
-
        function setInit(id) {


I removed this $E.initialize call, I think it's unnecessary because it's already done in so many other places on a page with comments.

noi5e · 2021-01-17T06:07:15Z

@jywarren Okay, ready to merge!

Sagarpreet

This is awesome 🎉
I think a better approach to find the divs or anything inside a comment editor would be the direct use of id's, like instead of trying to find the closest DOM element inside form like:
e.target.closest('div.comment-form-wrapper');
It would be better to access it directly like:
comment-form-wrapper-2131 where 2131 is some unique id.
Advantage being we are sure that even if their are multiple forms, we are always manipulating the correct DOM elements.
Saying that, the current approach is also fine for now 😄

Sagarpreet · 2021-01-17T07:42:40Z

app/assets/javascripts/dragdrop.js

@@ -17,44 +17,36 @@ jQuery(function() {
    );
  }

-  $('.dropzone').on('dragover',function(e) {
-    e.preventDefault();


Amazing 👍

Sagarpreet · 2021-01-17T07:43:46Z

app/assets/javascripts/dragdrop.js

+      e.preventDefault();
+      $(e.currentTarget).addClass('hover');
+    });
+


wow good thinking 🎉

Sagarpreet · 2021-01-17T07:49:04Z

app/assets/javascripts/dragdrop.js

@@ -115,15 +102,27 @@ jQuery(function() {
        },

        // see callbacks at https://github.com/blueimp/jQuery-File-Upload/wiki/Options
-        fileuploadfail: function(e,data) {
+        // fileuploadfail: function(e,data) {


Should we show a toast/noty with "something went wrong while uploading, kindly report an issue if this persists"?
@jywarren thoughts?

Or atleast printing in console the e error msg for quick debugging?

That's a good idea, can do it in a follow-up PR

Sagarpreet · 2021-01-17T07:51:04Z

app/assets/stylesheets/editor.css

    font-size: inherit;
    color: inherit;
    white-space: pre-wrap;
    background-color: transparent;
    border-radius: 0;
  }
-}



codeclimate · 2021-01-17T17:47:05Z

Code Climate has analyzed commit 162fce6 and detected 0 issues on this pull request.

View more on Code Climate.

noi5e · 2021-01-17T18:15:00Z

@Sagarpreet Thanks for the detailed review! I just resolved the merge conflict, and tests are passing, so this one's ready to merge again. @jywarren @cesswairimu

I totally agree about referring to comment forms with a unique ID rather than div.comment-form-wrapper. I put it on my to-do list. I'm starting to refactor a lot of the code this week, so I hope to make some progress on implementing that.

jywarren

This looks amazing, i appreciate all the small and careful tweaks, fixes, and documentation. it was easy to read and review and @Sagarpreet's review is also super helpful, thanks!

Approving and merging!!!

…ubliclab#9019) * add tests for image upload progress bars publiclab#9018 * fix image upload hover & progress bar issues publiclab#9018 * change ID references to class references publiclab#9018 * fix style for edit comment forms on questions publiclab#9018 * add classes to image upload progress bars publiclab#9018 * add classes; remove non-unique IDs publiclab#9018 * tweak image upload progress bar tests publiclab#9018 * forgot to remove screenshot publiclab#9018

add tests for image upload progress bars publiclab#9018

35b00a4

noi5e added bug the issue is regarding one of our programs which faces problems when a certain task is executed JavaScript outreachy labels Jan 16, 2021

noi5e added this to the [Outreachy] Comment Editor Winter 2020 milestone Jan 16, 2021

noi5e added 6 commits January 16, 2021 19:52

fix image upload hover & progress bar issues publiclab#9018

7c0958f

change ID references to class references publiclab#9018

63c9156

fix style for edit comment forms on questions publiclab#9018

42473fe

add classes to image upload progress bars publiclab#9018

b1cac82

add classes; remove non-unique IDs publiclab#9018

ef1a336

tweak image upload progress bar tests publiclab#9018

4905029

noi5e requested a review from a team as a code owner January 17, 2021 03:56

forgot to remove screenshot publiclab#9018

859962f

noi5e commented Jan 17, 2021

View reviewed changes

noi5e added the readytomerge label Jan 17, 2021

noi5e requested review from cesswairimu, jywarren and sagarpreet-chadha January 17, 2021 06:02

noi5e mentioned this pull request Jan 17, 2021

OLD CLOSED Comment Editor Overhaul Issue #8775

Closed

Sagarpreet approved these changes Jan 17, 2021

View reviewed changes

Merge branch 'main' into bug/edit-comment-progress-bars

162fce6

jywarren approved these changes Jan 18, 2021

View reviewed changes

jywarren merged commit 6d500fe into publiclab:main Jan 18, 2021

noi5e deleted the bug/edit-comment-progress-bars branch January 18, 2021 19:56

noi5e mentioned this pull request Jan 23, 2021

Comment Editor Overhaul Project [Outreachy] #9069

Open

noi5e changed the title ~~Bug: Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload~~ Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload Jan 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload #9019

Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload #9019

noi5e commented Jan 16, 2021 •

edited

gitpod-io bot commented Jan 16, 2021 •

edited

codecov bot commented Jan 16, 2021 •

edited

noi5e Jan 17, 2021

Sagarpreet Jan 17, 2021

noi5e Jan 17, 2021

Sagarpreet Jan 17, 2021

noi5e Jan 17, 2021

Sagarpreet Jan 17, 2021

noi5e Jan 17, 2021

noi5e Jan 17, 2021

noi5e commented Jan 17, 2021

Sagarpreet left a comment

Sagarpreet Jan 17, 2021

Sagarpreet Jan 17, 2021

Sagarpreet Jan 17, 2021

Sagarpreet Jan 17, 2021

noi5e Jan 17, 2021

Sagarpreet Jan 17, 2021

codeclimate bot commented Jan 17, 2021

noi5e commented Jan 17, 2021

jywarren left a comment

Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload #9019

Show Image Upload Progress Bars in Edit Comment CLICK-to-Upload #9019

Conversation

noi5e commented Jan 16, 2021 • edited

gitpod-io bot commented Jan 16, 2021 • edited

codecov bot commented Jan 16, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noi5e commented Jan 17, 2021

Sagarpreet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot commented Jan 17, 2021

noi5e commented Jan 17, 2021

jywarren left a comment

Choose a reason for hiding this comment

noi5e commented Jan 16, 2021 •

edited

gitpod-io bot commented Jan 16, 2021 •

edited

codecov bot commented Jan 16, 2021 •

edited