Edgewall Software

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#11717 closed enhancement (fixed)

Highlight the whole referenced comment — at Version 8

Reported by: vlastimil.zima@… Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.3
Component: ticket system Version: 1.0.1
Severity: normal Keywords:
Cc: Branch:
Release Notes:

The entire h3 is highlighted when linking to a comment rather than just the enclosed span.

API Changes:

On the ticket page the id #comment:%s was moved from the comment span to the enclosing h3.

Internal Changes:

Description

If you open a link with reference to a comment, e.g. ticket:3#comment:3 the selected comment is not much visible. Only the "comment:NUM" string in the top right corner is highlighted which doesn't show up that much.

The page is scrolled to the comment, but if the comment is one of the last the page doesn't scroll that far, actually showing a different part of history. If you get unlucky enough, page can scroll to the top of another comment, thus causing even more confusion.

I suggest to highlight the whole comment block, so its clearly visible which comment is referenced in the link.

Change History (9)

by Ryan J Ollos, 10 years ago

Attachment: HighlightH3.png added

comment:1 by Ryan J Ollos, 10 years ago

I have felt similarly. We could also consider highlighting the entire h3:

  • trac/ticket/templates/ticket_change.html

    diff --git a/trac/ticket/templates/ticket_change.html b/trac/ticket/templates/ticket_change.html
    index e5ecbea..82e6f83 100644
    a b Arguments:  
    3939  <py:def function="commentref(prefix, cnum, cls=None)">
    4040    <a href="#comment:$cnum" class="$cls">$prefix$cnum</a>
    4141  </py:def>
    42   <h3 class="change">
     42  <h3 class="change" id="comment:$cnum">
    4343    <span class="threading"
    4444          py:with="change_replies = replies.get(str(cnum), []) if 'cnum' in change else []">
    45       <span py:if="'cnum' in change" id="comment:$cnum" class="cnum">${commentref('comment:', cnum)}</span>
     45      <span py:if="'cnum' in change" class="cnum">${commentref('comment:', cnum)}</span>
    4646      <py:if test="'replyto' in change">
    4747        in reply to: ${commentref('&uarr;&nbsp;', change.replyto)}
    4848        <py:if test="change_replies">; </py:if>

Highlighting the entire commment or putting a border around it might work as well.

comment:2 by Ryan J Ollos, 10 years ago

Milestone: 1.1.3
Owner: set to Ryan J Ollos
Status: newassigned

comment:3 by Jun Omae, 10 years ago

Sounds good. However, if the change is attachment, the h3 element would have wrong id="comment:" attribute.

-  <h3 class="change">
+  <h3 class="change" id="${'comment:%s' % cnum if 'cnum' in change else None}">

comment:4 by Ryan J Ollos, 10 years ago

TracUpgrade#CustomizedTemplates states the we usually won't modify element ids or change CSS classes, and if we have to do so, this will be documented in the TracDev/ApiChanges pages. I haven't been rigorous about documenting template and CSS changes, but will keep it in mind for changes like the one in this ticket.

in reply to:  4 comment:5 by Ryan J Ollos, 10 years ago

Replying to rjollos:

TracUpgrade#CustomizedTemplates states the we usually won't modify element ids or change CSS classes, and if we have to do so, this will be documented in the TracDev/ApiChanges pages. I haven't been rigorous about documenting template and CSS changes, but will keep it in mind for changes like the one in this ticket.

I suppose the changes are captured in TracDev/ApiChanges through the ticket listing wiki:TracDev/ApiChanges/1.1.2#list. So perhaps the level of detail in wiki:TracDev/ApiChanges/1.1.2@15?action=diff and wiki:TracDev/ApiChanges/1.1.2@14?action=diff isn't necessary or useful and we should just make sure to capture the detail in the API Changes field for each ticket.

in reply to:  1 comment:6 by vlastimil.zima@…, 10 years ago

Replying to rjollos:

Highlighting the entire commment or putting a border around it might work as well.

I'd be rather for highlighting the entire comment.

comment:7 by Ryan J Ollos, 10 years ago

It's worth considering how the behavior compares to wiki TracLinks: TracReports#ChangingSortOrder. We can see in that case the whole heading is highlighted.

If we choose to highlight only the heading, we can also try to make it easy enough to add some custom CSS for highlighting the whole comment (via TracInterfaceCustomization#SiteAppearance).

comment:8 by Ryan J Ollos, 9 years ago

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

We'll consider highlighting the entire comment when the user interface is refreshed in a future milestone.

Fixed in [13201].

Note: See TracTickets for help on using tickets.