Edgewall Software
Modify

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#11835 closed enhancement (fixed)

cosmetic improvements for ticket changelog

Reported by: Christian Boos Owned by: Christian Boos
Priority: normal Milestone: 1.2
Component: ticket system Version: 1.1dev
Severity: minor Keywords:
Cc: leho@…
Release Notes:

Restyled the ticket changelog. Added a Show comments preference.

API Changes:

Usernames are wrapped in a span with class trac-author-user.

Description

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)

Attachments (2)

20141117T2020.png (78.0 KB ) - added by Ryan J Ollos 4 years ago.
20141130T0422.png (25.8 KB ) - added by Ryan J Ollos 4 years ago.

Download all attachments as: .zip

Change History (41)

comment:1 Changed 4 years ago by Christian Boos

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

Changed 4 years ago by Ryan J Ollos

Attachment: 20141117T2020.png added

comment:2 Changed 4 years ago by 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.

comment:3 Changed 4 years ago by 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

comment:4 Changed 4 years ago by lkraav <leho@…>

Cc: leho@… added

comment:5 Changed 4 years ago by Christian Boos

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

comment:6 Changed 4 years ago by Christian Boos

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

Changed 4 years ago by Ryan J Ollos

Attachment: 20141130T0422.png added

comment:7 Changed 4 years ago by 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.

comment:8 in reply to:  7 Changed 4 years ago by 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].

Last edited 4 years ago by Christian Boos (previous) (diff)

comment:9 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.31.1.4

comment:10 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.41.1.5

comment:11 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.51.1.6

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

comment:12 Changed 3 years ago by Ryan J Ollos

Milestone: 1.1.6next-dev-1.1.x

comment:13 Changed 3 years ago by Ryan J Ollos

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

comment:14 Changed 3 years ago by Ryan J Ollos

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

comment:15 Changed 3 years ago by Ryan J Ollos

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

comment:16 Changed 3 years ago by Christian Boos

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

comment:17 Changed 3 years ago by Ryan J Ollos

Milestone: next-dev-1.1.x1.2

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

comment:18 Changed 3 years ago by Ryan J Ollos

Owner: changed from Christian Boos to Ryan J Ollos
Status: newassigned

comment:19 Changed 3 years ago by 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.

comment:20 Changed 3 years ago by Ryan J Ollos

API Changes: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Tests fixed in [14395].

comment:21 Changed 3 years ago by Ryan J Ollos

Owner: changed from Ryan J Ollos to Christian Boos

comment:22 Changed 3 years ago by Christian Boos

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

Last edited 3 years ago by Ryan J Ollos (previous) (diff)

comment:23 Changed 3 years ago by 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?

comment:24 Changed 3 years ago by Ryan J Ollos

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

comment:25 Changed 3 years ago by Ryan J Ollos

Regression fixed in [14456].

How about the following change to address comment:22?

  • trac/htdocs/css/ticket.css

    diff --git a/trac/htdocs/css/ticket.css b/trac/htdocs/css/ticket.css
    index ce4fbd2..68fa823 100644
    a b ul.children > li.child {  
    144144 border-radius: .8em;
    145145}
    146146#changelog ul.children h3 { margin-right: .4em }
     147#changelog pre.wiki, #changelog div.code { clear: right }
    147148
    148149.in-reply-to:before { content: "↑" }
    149150.follow-up:before { content: "↓" }

comment:26 Changed 3 years ago by Ryan J Ollos

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

comment:27 Changed 3 years ago by 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

comment:28 Changed 3 years ago by Ryan J Ollos

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

comment:29 in reply to:  28 Changed 3 years ago by 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…

comment:30 Changed 3 years ago by 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.

comment:31 Changed 3 years ago by Ryan J Ollos

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

comment:32 Changed 3 years ago by 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?

comment:33 in reply to:  32 Changed 3 years ago by 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 ;-) ).

comment:34 Changed 3 years ago by 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.

comment:35 Changed 3 years ago by Jun Omae

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

comment:36 Changed 3 years ago by Christian Boos

Sorry, what?

comment:37 Changed 3 years ago by Jun Omae

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

  • trac/ticket/templates/ticket.html

    diff --git a/trac/ticket/templates/ticket.html b/trac/ticket/templates/ticket.html
    index 48e00918b..acda2d4e9 100644
    a b  
    5555              input = $("#propertyform input[name=cc_update]").val([]);
    5656            }
    5757            input.change();
    58             $(this).closest("li").remove();
     58            // Remove a row to revert field
     59            if ($(this).closest("tbody").children("tr").length === 1)
     60              $(this).closest(".trac-change-panel").remove();
     61            else
     62              $(this).closest("tr").remove();
    5963            return false;
    6064          });
    6165        }

comment:38 Changed 3 years ago by Christian Boos

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

comment:39 in reply to:  37 Changed 3 years ago by Jun Omae

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

Applied in [14763].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.