Rework archive/mute UI.
Review Request #7270 — Created April 30, 2015 and submitted
The UI for archive and mute within a review request was a little bit intrusive,
and given how complicated this page already is, was too cluttered. In
particular, the addition of new banners at the top of the page (which
inadvertantly hid existing banners like the draft banner or closed banner) were
just far too much UI for this.I've replaced the "Hide" menu and the banners with a new menu on the left-hand
side of the review request box, just next to the star icon. This uses a new
"inbox" icon in the same style as the star icon, which highlights when the
review request is archived or muted.As part of this, archiving/muting/unarchiving/unmuting no longer reloads the
page. Instead, the code in the ReviewRequestEditorView will update the
visibility of the relevant menu items after the state is saved to the server.
Once we have a better icon, I intend to also have it change the state of the
icon.
- Archived, unarchived, muted, and unmuted review requests.
- Checked that the state was always tracked correctly.
- Ran unit tests.
- Ran js-tests.
| Description | From | Last Updated |
|---|---|---|
|
How does this look as a darker brown? Something equivalent to setting the opacity to 80%, maybe. Might help visually … |
chipx86
|
|
|
Can we have this happen when the 'visibility' property changes, instead of hard-coding each time we set it? |
chipx86
|
|
|
This gets repeated a lot. I think it's probably worth putting this in a utility function. It can take the … |
chipx86
|
|
|
The ? should be indented 2 more spaces. Can you put parens around the whole thing? |
chipx86
|
Review request changed
- Description:
-
The UI for archive and mute within a review request was a little bit intrusive,
and given how complicated this page already is, was too cluttered. In particular, the addition of new banners at the top of the page (which inadvertantly hid existing banners like the draft banner or closed banner) were just far too much UI for this. I've replaced the "Hide" menu and the banners with a new menu on the left-hand
side of the review request box, just next to the star icon. For the moment, this uses FA's "archive" icon, but that's pretty clunky, so a future change will introduce a new, cleaner icon that matches better with the star. As part of this, archiving/muting/unarchiving/unmuting no longer reloads the
page. Instead, the code in the ReviewRequestEditorView will update the visibility of the relevant menu items after the state is saved to the server. Once we have a better icon, I intend to also have it change the state of the icon. + + The change to
icons.svghas been excluded from this review request, because+ you don't need to see all that XML. - Commit:
-
5e4ac3059dc1ff357765ea31fa1c507a542f9892e48a4f67bb538ad940beb527a235c1b404325b51
- Diff:
-
Revision 2 (+144 -273)
- Removed Files:
- Added Files:
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/ui/base.html reviewboard/templates/reviews/reviewable_page_data.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/templates/diffviewer/view_diff.html reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/reviewRequestModel.js reviewboard/static/rb/css/assets/icons.less reviewboard/templates/reviews/review_header.html reviewboard/templates/reviews/review_request_actions_secondary.html reviewboard/static/rb/images/icons.png reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/images/icons@2x.png reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/ui/base.html reviewboard/templates/reviews/reviewable_page_data.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/templates/diffviewer/view_diff.html reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/reviewRequestModel.js reviewboard/static/rb/css/assets/icons.less reviewboard/templates/reviews/review_header.html reviewboard/templates/reviews/review_request_actions_secondary.html reviewboard/static/rb/images/icons.png reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/images/icons@2x.png reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
-
Your description says a new icon will be made later, but that seems out of date now :)
-
Can we have this happen when the 'visibility' property changes, instead of hard-coding each time we set it?
-
This gets repeated a lot. I think it's probably worth putting this in a utility function. It can take the desired value, the collection, and an add/remove flag.. Or maybe just take the function itself, instead of collection + flag.
-
Review request changed
- Description:
-
The UI for archive and mute within a review request was a little bit intrusive,
and given how complicated this page already is, was too cluttered. In particular, the addition of new banners at the top of the page (which inadvertantly hid existing banners like the draft banner or closed banner) were just far too much UI for this. I've replaced the "Hide" menu and the banners with a new menu on the left-hand
~ side of the review request box, just next to the star icon. For the moment, ~ this uses FA's "archive" icon, but that's pretty clunky, so a future change ~ will introduce a new, cleaner icon that matches better with the star. ~ side of the review request box, just next to the star icon. This uses a new ~ "inbox" icon in the same style as the star icon, which highlights when the ~ review request is archived or muted. As part of this, archiving/muting/unarchiving/unmuting no longer reloads the
page. Instead, the code in the ReviewRequestEditorView will update the visibility of the relevant menu items after the state is saved to the server. Once we have a better icon, I intend to also have it change the state of the icon. - - The change to
icons.svghas been excluded from this review request, because- you don't need to see all that XML. - Diff:
-
Revision 3 (+129 -272)
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/ui/base.html reviewboard/templates/reviews/reviewable_page_data.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/templates/diffviewer/view_diff.html reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/reviewRequestModel.js reviewboard/static/rb/css/assets/icons.less reviewboard/templates/reviews/review_header.html reviewboard/templates/reviews/review_request_actions_secondary.html reviewboard/static/rb/images/icons.png reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/images/icons@2x.png reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/ui/base.html reviewboard/templates/reviews/reviewable_page_data.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/templates/diffviewer/view_diff.html reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/reviewRequestModel.js reviewboard/static/rb/css/assets/icons.less reviewboard/templates/reviews/review_header.html reviewboard/templates/reviews/review_request_actions_secondary.html reviewboard/static/rb/images/icons.png reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/images/icons@2x.png reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js
Tool: Pyflakes Ignored Files: reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/ui/base.html reviewboard/templates/reviews/reviewable_page_data.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/templates/diffviewer/view_diff.html reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/reviewRequestModel.js reviewboard/templates/reviews/review_header.html reviewboard/templates/reviews/review_request_actions_secondary.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js Tool: PEP8 Style Checker Ignored Files: reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/ui/base.html reviewboard/templates/reviews/reviewable_page_data.js reviewboard/static/rb/js/views/reviewRequestEditorView.js reviewboard/templates/diffviewer/view_diff.html reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/reviewRequestModel.js reviewboard/templates/reviews/review_header.html reviewboard/templates/reviews/review_request_actions_secondary.html reviewboard/static/rb/js/models/userSessionModel.js reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js