Closed
Bug 1146774
Opened 9 years ago
Closed 9 years ago
treeherder/tbpl comments are not automatically collapsed
Categories
(bugzilla.mozilla.org :: User Interface, defect, P1)
Tracking
()
RESOLVED
FIXED
People
(Reporter: glob, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
8.82 KB,
patch
|
dylan
:
review+
|
Details | Diff | Splinter Review |
tbpl comments are not automatically collapsed, and it isn't possible to hide them from the activity stream. implementation wise we should add a hook to activity_stream() which allows the BMO extension to update relevant comments's collapsed attribute.
Summary: tbpl comments are not automatically collapsed → treeherder/tbpl comments are not automatically collapsed
- optimise comment default-collapsing code - add $comment->collapsed_reason - add list of users which cause comments to be collapsed by default - add 'hide/show treeherder comments' to view menu - fixes some bugs when expanding/collapsing collapsed-by-default comments
Attachment #8617753 -
Flags: review?(dylan)
Comment 2•9 years ago
|
||
Comment on attachment 8617753 [details] [diff] [review] 1146774_1.patch Review of attachment 8617753 [details] [diff] [review]: ----------------------------------------------------------------- I don't see anything objectionable in this, although I am surprised that the FILTER checker doesn't complain about extra_class in activity_stream.html.tmpl.
(In reply to Dylan William Hardison [:dylan] from comment #2) > I don't see anything objectionable in this, although I am surprised that the > FILTER checker doesn't complain about extra_class in > activity_stream.html.tmpl. the filter checker is very simple (regex based) and doesn't care much for the inverted style of that line. ie. if i'd done: > %]<div class="change-set cc-only [% extra_class %]" id="[% change_set.id %]" style="display:none">[% instead of > '<div class="change-set cc-only' _ extra_class _ '" id="' _ change_set.id _ '" style="display:none">'; it would have complained. fwiw for both extra_class and change_set.id we're guaranteed that their contents are safe, so i'd be using "FILTER none" here anyhow.
Comment 4•9 years ago
|
||
Comment on attachment 8617753 [details] [diff] [review] 1146774_1.patch Review of attachment 8617753 [details] [diff] [review]: ----------------------------------------------------------------- r- Code works, but the code below may eventually bite us. Comment or DRY. :-) ::: extensions/BugModal/Extension.pm @@ +257,5 @@ > + > + # find the treeherder bot user for the "view -> hide treeherder comments" menu tiem > + $vars->{treeherder} = > + Bugzilla::User->new({ name => 'tbplbot@gmail.com', cache => 1 }) > + || Bugzilla::User->new({ name => 'treeherder@bots.tld', cache => 1 }); The user list is coded as a constant in one place, and literal values here. Add a comment referencing the constant here, or use first { Bugzilla::User->new({name => $_, cache => 1}) } COLLAPSED_USERS;
Attachment #8617753 -
Flags: review?(dylan) → review-
(In reply to Dylan William Hardison [:dylan] from comment #4) > The user list is coded as a constant in one place, and literal values here. it's in two places because they are two different use-cases. the part you quoted is code specific to dealing with treeherder's comments. COLLAPSED_USERS is more generic, and it's likely we'll add more users to that list. there's no reason for the two lists to be identical (although that's currently the case).
- provide a central lookup for treeherder's user
Attachment #8617753 -
Attachment is obsolete: true
Attachment #8621443 -
Flags: review?(dylan)
Comment 7•9 years ago
|
||
Comment on attachment 8621443 [details] [diff] [review] 1146774_2.patch Review of attachment 8621443 [details] [diff] [review]: ----------------------------------------------------------------- r=dylan
Attachment #8621443 -
Flags: review?(dylan) → review+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git ac09f71..c290f3e master -> master
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
oops.. that menu item should only be visible when there are treeherder comments to hide: To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git 061328f..a48e145 master -> master
Updated•5 years ago
|
Component: User Interface: Modal → User Interface
Updated•5 years ago
|
Assignee: glob → nobody
You need to log in
before you can comment on or make changes to this bug.
Description
•