Bugs: |
|
|---|
Add confirmation when closing diff comment dialog with text present.
Review Request #8410 — Created Sept. 18, 2016 and submitted
When pressing ESC or Cancel in the diff viewer comment dialog it would
close and the comment text would be deleted.Now it will prompt the user if there is text that cancelling will
delete their data.
Tested closing comment box while empty using ESC and cancel,
closing comment box with text using ESC and cancel,
and posting a comment.Tests done in Chrome 53 and Firefox 48 on Ubuntu 16.04.
Jasmine tests all pass, added 2 unit tests for this change.
| Description | From | Last Updated |
|---|---|---|
|
Can you rebase this change onto release-2.5.x? |
 brennie
|
|
|
Can you wrap your description and testing done at 72 characters? |
brennie
|
|
|
Is it also possible to add a test that checks that if the confirm returns false, that we don't close … |
 mike_conley
|
|
|
Blank line between these (should insert ones between variable declarations and code, and also surrounding blocks like if, for, etc.). |
 chipx86
|
|
|
We use 4 space indentation. This looks to be using two. |
chipx86
|
|
|
This needs to be localized by wrappign the string in gettext(...). Also, we use single quotes instead of double quotes … |
chipx86
|
|
|
Blank line between these. |
chipx86
|
-
-
reviewboard/static/rb/js/views/commentDialogView.js (Diff revision 1) Blank line between these (should insert ones between variable declarations and code, and also surrounding blocks like
if,for, etc.). -
reviewboard/static/rb/js/views/commentDialogView.js (Diff revision 1) We use 4 space indentation. This looks to be using two.
-
reviewboard/static/rb/js/views/commentDialogView.js (Diff revision 1) This needs to be localized by wrappign the string in
gettext(...).Also, we use single quotes instead of double quotes whenever possible for strings.
-
ST
Change Summary:
Made formatting changes, removed bug 4434 as this doesn't entirely address that issue.
Bugs: |
|
||||
|---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 2 (+10 -2) |
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js
ST
Description: |
|
|||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||
Branch: |
|
|||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||
Diff: |
Revision 3 (+10 -2) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js
ST
Summary: |
|
|||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
ST
Change Summary:
Added Jasmine unit tests
Summary: |
|
||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||
Diff: |
Revision 4 (+32 -2) |
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/tests/commentDialogViewTests.js reviewboard/static/rb/js/views/commentDialogView.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/tests/commentDialogViewTests.js reviewboard/static/rb/js/views/commentDialogView.js
ST
Change Summary:
Added 2 tests for cancelling close
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 5 (+50 -2) |