Show text message "Loading File attachments..." in the files placeholder while the attachments are loading
Review Request #8642 — Created Jan. 20, 2017 and submitted
When load a review request with some file attachments, the "Files" list appears blank until the attachments finally load in, it looks broken.
After change, a text message "Loading File attachments..." will show in the files placeholder while the attachments are loading.
I have made some testing as following:
1. Created a new review request (File attachment request only), then added a file, after clicking "UPLOAD" button and before a file is loaded, the text message is showing in the files placeholder. Once the file is uploaded, the text message is replaced by the file thumbnail.
2. For an existing request (File attachment request only), when I added a new file, same thing happened as above.
| Description | From | Last Updated |
|---|---|---|
|
Typo in "Testing Done": "exsitng" |
 david
|
|
|
Please put the bug number into the bugs field, and update your summary/description to discuss the change itself. Please see … |
david
|
|
|
Summary shouldn't list the bug number or "Easyfix". It should just describe the change. |
 chipx86
|
|
|
Ideally, we'd have a spinner next to the loading text. You can place the following before the text <span class="fa … |
chipx86
|
|
|
Can you add a screenshot of how this looks with the placeholder there? |
chipx86
|
|
|
There are too many newlines here. |
david
|
|
|
If you change the ID in the HTML file, make sure to update it here. We also prefer single quotes … |
david
|
|
|
A few things on this line: Can we use a <div> instead of <span style="display:block">. Can we change the id … |
david
|
|
|
redefinition of unused 'TestCase' from line 55 |
 reviewbot
|
|
|
We won't need the style="display:block" anymore, since <div>'s have display:block by default. |
 mike_conley
|
|
|
We should add aria-hidden="true" to the spinner <span> so screen readers don't say anything aloud. You can also put the … |
david
|
|
|
There is an extra space between {% and trans. |
 brennie
|
-
Can you test the case of opening a review request with no file attachments, and then adding a new one via drag-and-drop?
-
-
Please put the bug number into the bugs field, and update your summary/description to discuss the change itself. Please see https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/ for details on what we like to see for those.
-
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 1) There are too many newlines here.
-
reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 1) If you change the ID in the HTML file, make sure to update it here.
We also prefer single quotes in JS strings.
-
reviewboard/templates/reviews/review_request_box.html (Diff revision 1) A few things on this line:
- Can we use a
<div>instead of<span style="display:block">. - Can we change the
idto bereview-request-files-placeholder? - "Loading file attachments..." should be wrapped in
{% trans "..." %}
- Can we use a
AN
Description: |
|
|||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||
Bugs: |
|
|||||||||||||||
Commit: |
|
|||||||||||||||
Diff: |
Revision 2 (+770 -1) |
-
Tool: Pyflakes Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js -
-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/stages/builtin_stages.py reviewboard/reviews/evolutions/add_stages.py reviewboard/reviews/tests/test_review_request.py reviewboard/reviews/tests/test_review_request_stage_controller.py reviewboard/datagrids/columns.py reviewboard/reviews/__init__.py reviewboard/reviews/evolutions/__init__.py reviewboard/reviews/stages/__init__.py reviewboard/reviews/stages/registry.py reviewboard/extensions/hooks.py reviewboard/reviews/models/review_request_draft.py reviewboard/reviews/stages/controller.py reviewboard/reviews/models/review_request.py reviewboard/reviews/builtin_fields.py reviewboard/datagrids/grids.py reviewboard/reviews/stages/base.py reviewboard/reviews/tests/test_review_request_draft.py reviewboard/reviews/models/base_review_request_details.py reviewboard/extensions/tests.py Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js
AN
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+3) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js
-
This looks great, anni! Just one more thing and then I think you're good to go.
-
reviewboard/templates/reviews/review_request_box.html (Diff revision 3) We won't need the style="display:block" anymore, since <div>'s have display:block by default.
AN
Summary: |
|
||||
|---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 4 (+5) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js
-
-
reviewboard/templates/reviews/review_request_box.html (Diff revision 4) We should add
aria-hidden="true"to the spinner<span>so screen readers don't say anything aloud.You can also put the
{% trans ... %}on the next line.Indentation within HTML files should be 1 space.
AN
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 5 (+6) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js
-
-
reviewboard/templates/reviews/review_request_box.html (Diff revision 5) There is an extra space between
{%andtrans.
AN
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 6 (+6) |
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_request_box.html reviewboard/static/rb/js/views/reviewRequestEditorView.js