- 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 |
- Change Summary:
-
Made formatting changes, removed bug 4434 as this doesn't entirely address that issue.
- Bugs:
-
- Commit:
e006825ebddd78a36e2ede8e34eb18a7bdbd8be013845564a5ba62ec85a2788ef9218f37e012f6f2
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js
- Description:
-
~ When pressing ESC or Cancel in the diff viewer comment dialog it would close and the comment text would be deleted.
~ 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.
~ Now it will prompt the user if there is text that cancelling will
+ delete their data. - Testing Done:
-
~ Tested closing comment box while empty using ESC and cancel, closing comment box with text using ESC and cancel and posting comment. Tests done in Chrome 53 and Firefox 48 on Ubuntu 16.04
~ 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.
- Branch:
-
masterrelease-2.5.x
- Commit:
-
13845564a5ba62ec85a2788ef9218f37e012f6f2f3b1da968db46149f1738bca5b85ae993ab79bab
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/commentDialogView.js
- Summary:
-
Add confirmation when closing diff comment dialog with text present.[WIP] Add confirmation when closing diff comment dialog with text present.
- Testing Done:
-
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.
+ + Working on Jasmine tests now
- Change Summary:
-
Added Jasmine unit tests
- Summary:
-
[WIP] Add confirmation when closing diff comment dialog with text present.Add confirmation when closing diff comment dialog with text present.
- Testing Done:
-
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.
~ Working on Jasmine tests now
~ Jasmine tests all pass, added 2 unit tests for this change.
- Commit:
-
f3b1da968db46149f1738bca5b85ae993ab79bab795e8029dcd782a1a9de56bf92920268655f3f94
-
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
- Change Summary:
-
Added 2 tests for cancelling close
- Commit:
-
795e8029dcd782a1a9de56bf92920268655f3f94d3d8005779d7b41eb59eed3a36407b3482faef44