1096798 - prototype modal show_bug view

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: User Interface, defect)

Description

[:glob ✱]

prototype a modal show_bug view:

rough plan:
- modal: read-only / edit
- group fields by function
- "read-only"
    - hide fields which are not set
    - ensure show_bug.cgi loads current values only
    - allow adding a comment, comment tagging, needinfo, and CC'ing (with simple on/off toggle)
- edit
    - show all fields, still in their groups
    - retain simple self-cc toggle as well as full list editing
- comments
    - show_bug.cgi could load only the most recent N bugs (5?)
    - lazy load remaining comments with js
- bugzfeed integration

Comment 1

[:glob ✱]

Attachment: 1096798_1.patch

as per our discussions, please review for functional correctness only.
we can iterate over design decisions at a later date with feedback from the community.

- adds a user preference “Use experimental user interface”, default off
- can be enabled by passing format=modal to show_bug, or disabled with format=__default__
- as this user interface only works with the mozilla skin; enabling it will automatically change your skin.  support for other skins (classic, dusk) is not planned.

this patch includes two small images as binary, use |git apply| to deploy.

this ui does not yet have feature parity with show_bug, notable features missing include (but are not limited to):
- cannot edit cc list (cannot remove or add other people)
- comment preview
- comment tagging (existing tags are shown, cannot add/delete tags)
- cc activity is not visible
- bulk comment collapsing/expanding (all, by tag, tbpl push bot)
- alternative ordering of comments (eg. newest-first)
- bmo show_bug extensions (eg moz review, orange factor, bounty tracking, crash signature rendering)

looking forward this should give us a much more modern and consistent base to build other user interface improvements upon, such as role/team specific UIs, bugzfeed integration, lightbox for images, etc.

once we’re closer to a deployable patch, i’ll start filing bugs for this work.

Comment 2

[:glob ✱]

Attachment: 1096798_2.patch

- use the correct show_bug format after bug creation
- 'assignee' should take precedence over 'report' when reporting a user's role
- add missing ucfirst filter (absent from older tt versions)
- fix duplicate id="cc-list" divs
- retain query-string format when clicking 'cancel'
- extend product & component info toggle to the whole name instead of just the latch
- fix display of cc list for bugs with an empty cc list
- adding an invalid field shouldn't show debugging (eg. missing custom field)
- add url rewriting (process_bug.cgi --> show_bug.cgi)
- use template->context->define_filter and define_vmethod instead of editing template's internals

Comment 3

[:glob ✱]

Attachment: 1096798_3.patch

- fix inability to update custom textareas
- fix display of custom field_types on post-update bug display
- treat all @...bugs addresses as unassigned (eg. [e-mail])
- only float the edit button of custom textareas right if a value is set

Comment 4

[:glob ✱]

Attachment: 1096798_4.patch

- fix the display of tracking flags that don't have a corresponding status flag (eg. tracking-e10s)
- use jquery-ui tooltips for bug references
- fix jquery-ui tooltip memory leak

Comment 5

[:glob ✱]

Attachment: 1096798_5.patch

following lots of excellent feedback from :bwinton:
- add 'copy summary' to copy the bug id and summary to clipboard (for hg commmit messages, etc)
- change 'expand all' to toggle modules instead of only expanding
- move comment actions to top-right of div
- replace votes with whiteboard in details header summary, and collapse the details module by default
- fix whiteboard xss
- fix styling of quoted text in comments
- defocus spam comments by removing background colour
- lots of minor UI alignment tweaks
- minor changes to the mozilla skin (faster transitions, footer tweaks)

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 8579136 [details] [diff] [review]
1096798_4.patch

Review of attachment 8579136 [details] [diff] [review]:
-----------------------------------------------------------------

Running commentary. I am going through JS right now but wanted to get these to you sooner so you can debunk/agree with anything I have so far.

Comments for now:

1. Nit: Private comments are not as obvious as before. Just the word Private. Kind of liked the old color change of the comment background
2. Do we need gravatars for smaller identities such asssignee, attachment creators, etc.? can't hardly see them. Seems only useful for comments IMO.
3. Nit: Add a <label for=""> for the word "Private" for each comment.
4. Space after "of" when marking as duplicate.
5. No comment tags UI
6. No hook for comment actions such as "[edit]", etc.
7. I may be going crazy, but I do not see a way to add someone (other than myself) to the CC list.
8. When making an attachment, it takes me back to the old show_bug page and not the modal one. If I reload I get the modal back.
9. When clicking on a patch name, I should get the raw patch output. We still have a "details" link for the edit attachment page. Either get rid of "details" if your plan was to make this the new behavior or change the attachment name back to showing the raw content.
10. Nit: Tracking flag values in the subtitle of the module are as: (firefox28<affected>, b2g18<affected>, b2g-v1.4<affected>). Maybe either do (firefox28:affected, b2g18:affected, b2g-v1.4:affected) or add a space such as (firefox28 <affected>, b2g18 <affected>, b2g-v1.4 <affected>) Looks cryptic when that are pushed together that way IMO.
11. Nit: Change "Dependency tree / graph" to "Dependency tree or graph".
12. Missing "Ignore bug mail"
Overall I really this this is almost ready and really like it.

Comments for future:

1. Need "(take)" shortcuts for assignee and qa contact.
2. Would love to see a floating "Back to Top" link that always stays in the bottom right of the 'viewport' similar to other sites.
3. Maybe add another "Edit" button in a second location. Currently if I want to mark a comment private and I am at the bottom, I have to go back to the top, hit Edit, and then go back down to check the privacy checkbox.
4. Nit: Make the status a different color in the top module to differentiate it from the summary.

::: extensions/BugModal/template/en/default/bug_modal/module.html.tmpl
@@ +17,5 @@
> +
> +<div class="module
> +    [%~ " edit-hide" IF hide_on_edit %]
> +    [%~ " edit-show" IF hide_on_view && !hide_on_edit %]"
> +    [% IF hide_on_view +%] style="display:none"[% END %]

nit: should be a class.

Comment 7

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #6)
> 1. Nit: Private comments are not as obvious as before. Just the word
> Private. Kind of liked the old color change of the comment background
> 2. Do we need gravatars for smaller identities such asssignee, attachment
> creators, etc.? can't hardly see them. Seems only useful for comments IMO.

as per our conversations and comment 1, please review functional elements only, not design decisions at this point.

> 5. No comment tags UI
> 6. No hook for comment actions such as "[edit]", etc.
> 7. I may be going crazy, but I do not see a way to add someone (other than
> myself) to the CC list.

comment 1 has a list of missing features which includes all of these.

> 8. When making an attachment, it takes me back to the old show_bug page and
> not the modal one. If I reload I get the modal back.
> 9. When clicking on a patch name, I should get the raw patch output. We
> still have a "details" link for the edit attachment page. Either get rid of
> "details" if your plan was to make this the new behavior or change the
> attachment name back to showing the raw content.

will fix these bugs.

> > +<div class="module
> > +    [%~ " edit-hide" IF hide_on_edit %]
> > +    [%~ " edit-show" IF hide_on_view && !hide_on_edit %]"
> > +    [% IF hide_on_view +%] style="display:none"[% END %]
> 
> nit: should be a class.

jquery's hide/show/toggle works with inline styles, not classes.  we don't gain anything by using a display:none class, and it makes the js more cumbersome.

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 8579821 [details] [diff] [review]
1096798_5.patch

Review of attachment 8579821 [details] [diff] [review]:
-----------------------------------------------------------------

Nits: Move web/bug_modal.js to web/js/bug_modal.js (same for web/style).

Overall very very nice. Like the simplicity that jQuery brings over YUI. JS is much cleaner. Only a few small nits/comments that can be fixed on commit if valid. Other issues are known from discussions and previous comments. r=dkl

::: extensions/BugModal/lib/ActivityStream.pm
@@ +115,5 @@
> +        ? $bug->get_activity()
> +        : Bugzilla::Bug::GetBugActivity($bug->id);
> +
> +    # allow other extensions to alter history
> +    Bugzilla::Hook::process('inline_history_activtiy', { activity => $raw_activity });

s/inline_history_activtiy/inline_history_activity/

::: extensions/BugModal/template/en/default/bug_modal/cc_list.html.tmpl
@@ +10,5 @@
> +  UNLESS cc_list.size;
> +    'None';
> +  END;
> +  FOREACH cc IN cc_list;
> +    INCLUDE bug_modal/user.html.tmpl u=cc nick_only=1 no_menu=1;

no_menu not used

::: extensions/BugModal/web/bug_modal.css
@@ +633,5 @@
> +    max-width: 1024px !important;
> +    min-width: 800px !important;
> +}
> +
> +.vcard {

.vcard defined twice

::: extensions/BugModal/web/bug_modal.js
@@ +199,5 @@
> +    //
> +    // anything after this point is only executed for logged in users
> +    //
> +
> +    if (BUGZILLA.user.id === 0) return;

Contradictory as we do no allow the new form for non-logged-in users according to Extension.pm

@@ +293,5 @@
> +                    $('#top-save-btn').show();
> +                    $('#cancel-btn').show();
> +                    $('#commit-btn').show();
> +                },
> +                function(message) {

message not used?

Comment 9

[:glob ✱]

thanks for the review!

will fix those issues and other points from comment 6 on commit.



To ssh://[e-mail]/webtools/bmo/bugzilla.git
   11bd061..3ac7012  master -> master