Dataup 209 fix error msg cutoff #1885

eamahanna · 2020-10-23T04:05:13Z

Description of PR purpose/changes

This PR alters the file row UI in the dropzone area. On success and error, the file row has been updated to reflect this figma mock: https://www.figma.com/file/zBQcHiLV4FlIkD3Y1ogNcy/concept-sketches2?node-id=13%3A153

Jira Ticket / Issue

https://kbase-jira.atlassian.net/browse/DATAUP-209

Added the Jira Ticket to the title of the PR e.g. (DATAUP-69 Adds a PR template)

Testing Instructions

Details for how to test the PR:

To test this PR, run the narrative locally and navigate to the staging area. Drop a file that is larger than the max file size (20). This will cause the file to error and you will be able to see the changes.

To view the successful file upload, you should see it flash briefly upon success. If you would like to view it for a longer period, open devtools and put a break point on line 108 in file fileUploadWidget.js.

A question I am currently asking Mallory: If the successful files are cleared immediately, do we need the brief flash of the success styling? Does it make sense to leave those files in the dropzone or should we just remove it immediately?

Dev Checklist:

My code follows the guidelines at https://sites.google.com/truss.works/kbasetruss/development
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
I have run Black and Flake8 on changed Python Code manually or with git precommit (and the travis build passes)

Updating Version and Release Notes (if applicable)

Version has been bumped for each release
Release notes have been updated for each release (and during the merge of feature branches)

coveralls · 2020-10-23T04:33:48Z

Coverage increased (+0.05%) to 17.027% when pulling 5076004 on DATAUP-209-fix-error-msg-cutoff into beef10e on truss.

eamahanna · 2020-10-23T15:19:32Z

kbase-extension/static/kbase/css/kbaseNarrative.css

@@ -17,7 +17,7 @@
  margin: 2em 4.5em;
 }

-.text-button {
+.btn__text {


Changed the button name to match new BEM standard.

eamahanna · 2020-10-23T15:21:47Z

kbase-extension/static/kbase/js/widgets/narrative_core/upload/fileUploadWidget.js

-                        // If there is a button already in the area, it has to be removed,
-                        // and appened to the new document when additional errored files are added.
-                        this.deleteClearAllButton();
-                        $dropzoneElem.append(this.makeClearAllButton());


Deleted this and moved it to the 'added' file event to fix a bug with the clear all button.

eamahanna · 2020-10-23T15:23:39Z

kbase-extension/static/kbase/js/widgets/narrative_core/upload/fileUploadWidget.js

                })
                .on('success', (file, serverResponse) => {
+                    var $successElem = $(file.previewElement);
+                    $successElem.find('#upload_progress_and_cancel').hide();
+                    $successElem.find('#dz_file_row_1').css({"display": "flex", "align-items": "center"});


Had to use display: flex to get the items in the row vertically aligned. I'm wasn't able to do this with any bootstrap trickery.

kbase-extension/static/kbase/js/widgets/narrative_core/upload/fileUploadWidget.js

kbase-extension/static/kbase/templates/data_staging/dropped_file.html

ialarmedalien · 2020-10-23T16:23:28Z

kbase-extension/static/kbase/templates/data_staging/dropped_file.html

+        <div id="upload_progress_and_cancel" style="display: inline;">
+            <div class="col-md-3 text-center">
+                <div class="progress progress-striped active dz-file__progress__bar" role="progressbar" aria-valuemin="0" aria-valuemax="100" aria-valuenow="0">
+                    <div class="progress-bar progress-bar-success" style="width:0%;" data-dz-uploadprogress></div>


no unit for 0

still needs the unit removing :)

is there a reason this styling is inline vs css?

ialarmedalien · 2020-10-23T16:28:40Z

Can you add a bit more space around the icons? They look a little cramped as-is.

leia-sefkin

AJ already caught everything I would have asked to be fixed - otherwise things are looking good.

Small comment/tip for testing: but I've also been getting around testing with a 20GB file by setting the maxFileSize var in the fileUploadWidget manually to 2

kbase-extension/static/kbase/css/kbaseNarrative.css

leia-sefkin

A few minor comments but looks good to me otherwise

leia-sefkin · 2020-10-27T16:42:58Z

kbase-extension/static/kbase/js/widgets/narrative_core/upload/fileUploadWidget.js

@@ -168,6 +172,15 @@ define([
            $('#clear-all-btn').remove();
        },

+        removeProgressBar: function($dropzoneElem) {
+            if (this.dropzone.getQueuedFiles().length === 0 &&


minor but it should be enough to say .length without the ===0 check

leia-sefkin · 2020-10-27T16:43:34Z

kbase-extension/static/kbase/templates/data_staging/dropped_file.html

+        <div id="upload_progress_and_cancel" style="display: inline;">
+            <div class="col-md-3 text-center">
+                <div class="progress progress-striped active dz-file__progress__bar" role="progressbar" aria-valuemin="0" aria-valuemax="100" aria-valuenow="0">
+                    <div class="progress-bar progress-bar-success" style="width:0%;" data-dz-uploadprogress></div>


is there a reason this styling is inline vs css?

ialarmedalien · 2020-10-27T17:00:32Z

kbase-extension/static/kbase/js/widgets/narrative_core/upload/fileUploadWidget.js

+                    $successElem.find('#dz_file_row_1').css({'display': 'flex', 'align-items': 'center'});
+                    $successElem.find('#success_icon').css('display', 'flex');
+                    $successElem.find('#success_message').css('display', 'inline');


Will there only ever be one file loading at a time -- i.e. will the IDs always be unique?

Can we change these so that rather than adding/removing inline styles, we are adding/removing a class instead? That way, all the css info can go in the stylesheet.

e.g.

 <div class="dz-file__status__success dz-file__status__success__hidden"> <span>Completed</span> <i class="fa fa-2x fa-check-circle dz-file__status__icon" aria-hidden="true"></i> </div> <div class="dz-file__status__error dz-file__status__error__hidden"> <span>ZOMG ERROR!</span> <i class="fa fa-2x fa-check-circle dz-file__status__icon" aria-hidden="true"></i> </div>

.dz-file__status__success { display: flex; ... } .dz-file__status__success__hidden { display: none; }

$successElem.find('#success_icon').removeClass('dz-file__status__success__hidden')

So this code only changes the file template (that is shown in dropzone) for a the single file. The default template, which creates each file's html won't be changed by the .on('errorr'... changes. I don't know if that made sense. If it didn't, let me know and I can explain on a quick zoom call. I can work on making the styling changes in classes instead of jquery stuff :)

sonarcloud · 2020-10-28T00:58:52Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
1 Code Smell

No Coverage information
0.0% Duplication

eamahanna added 4 commits October 21, 2020 15:55

Add functionality

f15c77b

styling changes

7d77fc1

fix alignment

741fa53

fix typo

e1baef8

eamahanna changed the base branch from develop to truss October 23, 2020 04:05

eamahanna added 3 commits October 23, 2020 05:57

Fix success clear

a703fef

Add test

d2eec86

quick error fix

1b250a0

eamahanna marked this pull request as ready for review October 23, 2020 15:15

eamahanna commented Oct 23, 2020

View reviewed changes

eamahanna added 2 commits October 23, 2020 07:41

fix quotes

4afdc29

Fix tests

e3e0076

eamahanna requested review from briehl, leia-sefkin, bio-boris and ialarmedalien October 23, 2020 15:45

ialarmedalien reviewed Oct 23, 2020

View reviewed changes

kbase-extension/static/kbase/js/widgets/narrative_core/upload/fileUploadWidget.js Outdated Show resolved Hide resolved

ialarmedalien reviewed Oct 23, 2020

View reviewed changes

kbase-extension/static/kbase/templates/data_staging/dropped_file.html Outdated Show resolved Hide resolved

ialarmedalien reviewed Oct 23, 2020

View reviewed changes

kbase-extension/static/kbase/templates/data_staging/dropped_file.html Outdated Show resolved Hide resolved

ialarmedalien reviewed Oct 23, 2020

View reviewed changes

leia-sefkin reviewed Oct 23, 2020

View reviewed changes

review edits

0cc1aba

eamahanna requested review from ialarmedalien and leia-sefkin October 26, 2020 21:20

ialarmedalien reviewed Oct 27, 2020

View reviewed changes

kbase-extension/static/kbase/css/kbaseNarrative.css Outdated Show resolved Hide resolved

ialarmedalien reviewed Oct 27, 2020

View reviewed changes

kbase-extension/static/kbase/css/kbaseNarrative.css Outdated Show resolved Hide resolved

ialarmedalien reviewed Oct 27, 2020

View reviewed changes

kbase-extension/static/kbase/css/kbaseNarrative.css Outdated Show resolved Hide resolved

leia-sefkin approved these changes Oct 27, 2020

View reviewed changes

ialarmedalien reviewed Oct 27, 2020

View reviewed changes

eamahanna added 2 commits October 27, 2020 16:43

Review edits

ec4b462

review edit

5076004

eamahanna merged commit ab028c9 into truss Oct 28, 2020

eamahanna deleted the DATAUP-209-fix-error-msg-cutoff branch October 28, 2020 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dataup 209 fix error msg cutoff #1885

Dataup 209 fix error msg cutoff #1885

eamahanna commented Oct 23, 2020 •

edited

Loading

coveralls commented Oct 23, 2020 •

edited

Loading

eamahanna Oct 23, 2020

eamahanna Oct 23, 2020

eamahanna Oct 23, 2020

ialarmedalien Oct 23, 2020

ialarmedalien Oct 26, 2020

leia-sefkin Oct 27, 2020

ialarmedalien commented Oct 23, 2020

leia-sefkin left a comment

leia-sefkin left a comment

leia-sefkin Oct 27, 2020

leia-sefkin Oct 27, 2020

ialarmedalien Oct 27, 2020 •

edited

Loading

eamahanna Oct 27, 2020

sonarcloud bot commented Oct 28, 2020

Dataup 209 fix error msg cutoff #1885

Dataup 209 fix error msg cutoff #1885

Conversation

eamahanna commented Oct 23, 2020 • edited Loading

Description of PR purpose/changes

Jira Ticket / Issue

Testing Instructions

Dev Checklist:

Updating Version and Release Notes (if applicable)

coveralls commented Oct 23, 2020 • edited Loading

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

ialarmedalien commented Oct 23, 2020

leia-sefkin left a comment

Choose a reason for hiding this comment

leia-sefkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ialarmedalien Oct 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 28, 2020

eamahanna commented Oct 23, 2020 •

edited

Loading

coveralls commented Oct 23, 2020 •

edited

Loading

ialarmedalien Oct 27, 2020 •

edited

Loading