-
-
This is some older code, but these day we use:
this.listenTo(fileAttachments, 'destroy', function() { ... });instead of using
on(). This helps to avoid some circular reference issues by allowingthisto intelligently disconnect any signal handlers when it wants to clean things up.Can you make that change for both of these
fileAttachments.onstatements, and re-test with those?
Fix Files header still appearing after all attachments are removed.
Review Request #8417 — Created Sept. 19, 2016 and submitted
The review page still showed the "Files" header when all attachments were
removed and the page was not reloaded. This was because the visibility of
the header was determined by the template once on page load, but the
attachments removed themselves dynamically from the page.We now listen to "attachment remove" events, and dynamically hide the header
as necessary. Note that it wasn't necessary to check if the header should be
re-shown if an attachment was added because it either
a) triggers a full-page reload (Update > Add File) or
b) Automatically reshows the Files section (drag-and-drop).
- Created a new review request and added (multiple) files to it.
- Deleted some of the files and confirmed deleting still works as before.
- When there was only one attachment left, removed it and confirmed the
"Files" header was also removed. - Uploaded another file attachment (now that there is no files/header)
using both drag-and-drop and Update > Add File and confirmed Files header
(and the file) reappears. - Repeated testing on a published review request.
| Description | From | Last Updated |
|---|---|---|
|
Oops, realized this wasn't updated for 2.5. Once that's confirmed to work and the branch info on here is correct, … |
 chipx86
|
|
|
This is some older code, but these day we use: this.listenTo(fileAttachments, 'destroy', function() { ... }); instead of using on(). … |
chipx86
|
|
|
#file-list-contianer will be underneath the review request editor view's element, so you can use this.$('#file-list-container') to narrow the scope to … |
 brennie
|
|
|
I believe you can just use this._$attachmentsContainer instead of re-querying. |
chipx86
|
|
|
jQuery's .hide() is the same as .css('display', 'none') |
 david
|
FI
- Description:
-
~ The review page still showed the "Files" header when all attachments were removed and the page was not reloaded. This was because the visibility of the header was determined by the template once on page load, but the attachments removed themselves dynamically from the page.
~ The review page still showed the "Files" header when all attachments were
+ removed and the page was not reloaded. This was because the visibility of + the header was determined by the template once on page load, but the + attachments removed themselves dynamically from the page. ~ We now listen to "attachment remove" events, and dynamically hide the header as necessary. Note that it wasn't necessary to check if the header should be re-shown if an attachment was added because that triggers a full-page reload and thus causes the template to re-render the header with the correct visibility.
~ We now listen to "attachment remove" events, and dynamically hide the header
+ as necessary. Note that it wasn't necessary to check if the header should be + re-shown if an attachment was added because it either + a) triggers a full-page reload (Update > Add File) or + b) Automatically reshows the Files section (drag-and-drop). - Testing Done:
-
- Created a new review request and added (multiple) files to it.
- Deleted some of the files and confirmed deleting still works as before.
~ - When there was only one attachment left, removed it and confirmed the "Files" header was also removed.
~ - Repeated testing on a published review request.
~ - When there was only one attachment left, removed it and confirmed the
"Files" header was also removed.
~ - Uploaded another file attachments (now that there is no files/header)
using both drag-and-drop and Update > Add File and confirmed Files header
(and the file) reappears.
+ - Repeated testing on a published review request.
- Commit:
-
79270a1799fae42de7e372545bac9bed03e5638c9a447bf1cd8bbb627f8aa559d421ebf16d38a8d8
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewRequestEditorView.js
FI
- Testing Done:
-
- Created a new review request and added (multiple) files to it.
- Deleted some of the files and confirmed deleting still works as before.
- When there was only one attachment left, removed it and confirmed the
"Files" header was also removed.
~ - Uploaded another file attachments (now that there is no files/header)
using both drag-and-drop and Update > Add File and confirmed Files header
(and the file) reappears.
~ - Uploaded another file attachment (now that there is no files/header)
using both drag-and-drop and Update > Add File and confirmed Files header
(and the file) reappears.
- Repeated testing on a published review request.
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewRequestEditorView.js
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewRequestEditorView.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewRequestEditorView.js