cosmetic improvements for ticket changelog

[Christian Boos]

I'd like to propose a few cosmetic improvements for the ticket view, mainly for the #changelog:

• leaner threaded mode
• have the inline buttons (reply, edit, delete) appear on hover (cf. r13333)
• highlight the author if it matches the user
• better identify the property changes and emphasize the new values
• add a show/hide control for the comments themselves (i.e. only showing the changes)

[Christian Boos]

See changes in cboos.git@t11835-ticket-changelog-facelift (below in chronological order):

• [0d118179/cboos.git] (#11835): CSS improvements for the #changelog in the ticket view
• [3fb5e47f/cboos.git] (#11835): Only show the inline buttons for a ticket change when hovering on it.
• [92ff7e82/cboos.git] (#11835): in threaded comments mode, don't show the in reply to and follow-up links
• [0658a6d2/cboos.git] (#11835): jQuery clean-up in threaded_comments.js.
• [d3fd9474/cboos.git] (#11835): show property changes on top of a small "yellow panel"
• [45c904a5/cboos.git] (#11835): add trac-author-use style when author matches the user
• [aba50dc3/cboos.git] (#11835): emphasizes the new value in a property change
• [ec2e0b7e/cboos.git] (#11835): add Show change comments checkbox in ticket view.

[Ryan J Ollos]

I definitely like:

• Hiding the editor buttons until hover.
• Property changes emphasis.
• The addition of some styling for spans returned by Chrome.authorinfo.

I'll need to consider for a while:

• The small yellow panel for property changes.
• Hiding change comments (see another comment below).

Some small things to consider:

• Removing some border radius from #ticketchange .changes

  border-bottom-right-radius: 0.4em;
  

  
• In the threaded view (above) it would be nice if the li.child border swept smoothly into the ul.changes border creating a 1px wide border on the overlap rather than combining to form a 2px wide border.
• The trac-ticket-buttons should push the comment:N in the h3 to the left, but only when hovering over the comment above it?
• I like the border around trac-author-user, but it always feels odd to me when two borders intersect. The border from trac-author-user looks to be hidden under the h3's border.
• Maybe the class name trac-author-self would be more clear for devs than trac-author-user? I'm unsure.
• The view when Show change comments is checked view feels odd to me initially (one refinement for later would be to make this unchecked by default). Rather than hiding changing comments, I might be wanting to collapse them all, and then expand only the one or ones that I'm interested in.

[Christian Boos]

Thanks for the comments! I'll try to integrate your feedback in the next iteration of the branch.

A few precisions:

• for the hovering, I tried to avoid moving content (using visibility: hidden instead of display: none), which works well in the full view, but leads to strange effects you see above when you hide the comments; so I'll reconsider or find another positioning
• I also thought about trac-author-self, but if this is appeals to the dev, it could maybe a bit obscure for the admin; anyway, it should be clear that by "user" we mean the currently logged in user (e.g. #150)
• for the Show change comments it's just a bug, the comments should be of course shown by default (Show comments would be clear enough); collapsing/expanding them would be nice, yes

[Christian Boos]

Ok, one step further, same branch cboos.git@t11835-ticket-changelog-facelift (below in chronological order):

• [973a42b0/cboos.git] (#11835): fix Show change comments checkbox
• [a95ba619/cboos.git] (#11835): improve the visual appearance of the property changes.
• [0adf23c3/cboos.git] (#11835): try floating left the property change boxes.
• [012fec24/cboos.git] (#11835): make sure the boxed author info doesn't touch bottom borders.
• [c6297dbc/cboos.git] (#11835): make sure that comment buttons are always reachable.

[Christian Boos]

Next iteration, same branch cboos.git@t11835-ticket-changelog-facelift (below in chronological order):

• [09483e25/cboos.git] bugfix checked → :checked
• [90d5716b/cboos.git] (#11835): the direction of the vertical arrows now corresponds to the changelog order
• [6eb45534/cboos.git] (#11835): move small yellow boxes on the right
• [7a827e08/cboos.git] (#11835): a few changes on global style
• [3a036145/cboos.git] (#11835): the change controls are always visible if neither the comments nor the property changes are shown

[Ryan J Ollos]

It is looking good. Some minor comments:

• Having the Reply / Edit buttons appear below the .trac-change-panel feels odd, but I'm not sure how else to deal with that. If left there, we should probably have slightly more spacing between the panel border and the button borders.
• Should the Reply button in the Description region autohide as well?
• The main drawback I see to the .trac-change-panel is that images will get pushed down the page when they overlap the panel, but I like the styling of the panel. The position of the image probably won't be an issue for most cases.
• Unrelated to your changes, but the users in the ticket property changes for the CC field aren't wrapped in a span with one of the trac-author classes (I recall thinking about this while working on #11145).
• One small issue when editing comments is that the wikitoolbar no longer sits above the textarea:

I'll do some more testing soon.

[Christian Boos]

Replying to rjollos:

It is looking good. Some minor comments:

• Having the Reply / Edit buttons appear below the .trac-change-panel feels odd, but I'm not sure how else to deal with that. If left there, we should probably have slightly more spacing between the panel border and the button borders.

Well, I went with some back and forth, above, below… The problem of putting the controls above is that the property changes box may feel too detached from the change title to which it belongs to, i.e. it might look closer to the next change title. I tried to mitigate this in the last changeset, please see how it works for you.

• Should the Reply button in the Description region autohide as well?

I don't think so. The main reason for hiding the buttons for the comments is their duplication. For the ticket box, the controls appear once, at different places and don't give this feeling of redundancy, I think. There are more reasons: with the related #10948 changes, I'll have the Modify link replaced by the "Edit" button, and this one should not be hidden. Having some hidden and some others not in the ticket box will be confusing.

• The main drawback I see to the .trac-change-panel is that images will get pushed down the page when they overlap the panel, but I like the styling of the panel. The position of the image probably won't be an issue for most cases.

Well, OK, but I also think it's not a big deal. You'll always have the possibility to move some text before the image if it really looks too ugly with the image first.

• Unrelated to your changes, but the users in the ticket property changes for the CC field aren't wrapped in a span with one of the trac-author classes (I recall thinking about this while working on #11145).

Noted.

One small issue when editing comments is that the wikitoolbar no longer sits above the textarea: […]

Easy to fix with a clear: right.

See [d25959ab/cboos.git].

[Ryan J Ollos]

Retargeting tickets that I think won't be completed for 1.1.5, but please move back if I'm incorrect.

[Ryan J Ollos]

I've rebased these changes against the current trunk. I'll post the changes after rebasing again when #7339 is committed.

[Ryan J Ollos]

Changes rebased in log:rjollos.git:t11835-ticket-changelog-facelift.1. Additional review and testing is needed.

[Ryan J Ollos]

Changes rebased in log:rjollos.git:t11835-ticket-changelog-facelift.2. Any thoughts on the changes?

[Christian Boos]

Yes, thanks a lot for reviving this! (Sorry for not having found the time to do it myself…)

[Ryan J Ollos]

The changes look good to me. I'll be doing more testing and will wait at least a few days for feedback before pushing.

[Ryan J Ollos]

Committed to trunk in [14393:14394]. I always seem to make an error pushing from Git to SVN! Also, changes broke tests, which I thought I had run. I will fix the issues shortly.

[Ryan J Ollos]

Tests fixed in [14395].

[Christian Boos]

DONE <pre> blocks in comments would benefit from a clear:right, see e.g. ticket:10562#comment:7

[Jun Omae]

Reporter and owner links are rendered with username, not user's full name. Before [14393], the full names was used. That is intentional?

[Ryan J Ollos]

That must be a regression. Probably the changes weren't rebased correctly. I'll fix it.

[Ryan J Ollos]

Regression fixed in [14456].

How about the following change to address comment:22?

trac/htdocs/css/ticket.css

@@ -144 +144 @@
  border-radius: .8em;
 }
 #changelog ul.children h3 { margin-right: .4em }
+#changelog pre.wiki, #changelog div.code { clear: right }
 
 .in-reply-to:before { content: "↑" }
 .follow-up:before { content: "↓" }

[Ryan J Ollos]

I committed the comment:25 CSS rule in [14465]. t.e.o has been updated to r14465.

[Jun Omae]

The order of form buttons in ticket comment is reversed after those changes.

1.0-stable / ticket comment

Reply, Edit, Delete

1.0-stable / ticket description

Reply, Delete

1.2dev / ticket comment

Delete, Edit, Reply (should be Reply, Edit, Delete)

1.2dev / ticket description

Reply, Delete

[Ryan J Ollos]

Yet another issue can be seen with attachment comments below comment:3:ticket:10323.

[Christian Boos]

Replying to Ryan J Ollos:

Yet another issue can be seen with attachment comments below comment:3:ticket:10323.

Ok, kicking out the max-width seems to fix it. IIRC, it was there to avoid such issues but since it doesn't work in case of overflow…

[Ryan J Ollos]

The CSS rule in [14465] doesn't address the issue for comment previews. For that, we'd need a rule like:

#ticketchange pre.wiki, #ticketchange div.code { clear: right }

… or maybe there is a more elegant solution.

[Ryan J Ollos]

comment:27, comment:28 and comment:30 will be addressed in #12347, along with comment:9:ticket:12120.

[Ryan J Ollos]

If Show property changes is unselected and linking to the change (for example comment:5:ticket:11901), there's no anchor on the page to scroll to. Maybe we could Show property changes in this case. Do you think this case is worth addressing?

[Christian Boos]

Replying to Ryan J Ollos:

Do you think this case is worth addressing?

Yes, but I think it's not serious enough to be worth the trouble of some merge conflict, so this could wait for 1.3.1 and I'll do it directly on the Jinja2 branch (now if you feel fixing it before, I won't mind ;-) ).

[Ryan J Ollos]

Fixing in a future version is fine with me. I actually made the note about this many months ago and just never got around to posting a comment, so I haven't felt it to be too urgent an issue.

[Jun Omae]

Revert button in property changes box works after auto-preview. The behavior feels broken revert button and annoys the user.

[Christian Boos]

Sorry, what?

[Jun Omae]

$(this).closest("li").remove(); doesn't work.

trac/ticket/templates/ticket.html

@@ -55 +55 @@
               input = $("#propertyform input[name=cc_update]").val([]);
             }
             input.change();
-            $(this).closest("li").remove();
+            // Remove a row to revert field
+            if ($(this).closest("tbody").children("tr").length === 1)
+              $(this).closest(".trac-change-panel").remove();
+            else
+              $(this).closest("tr").remove();
             return false;
           });
         }

[Christian Boos]

Ah sorry, I tested here, where it still works.

[Jun Omae]

$(this).closest("li").remove(); doesn't work. […]

Applied in [14763].