[PATCH 2 of 2 RFC] summary: add "last comment" tooltip to status icon

[Mads Kiilerich]

On 02/18/2016 05:36 PM, Angel Ezquerra wrote:
> # HG changeset patch
> # User ezquerra
> # Date 1455813238 -3600
> #      Thu Feb 18 17:33:58 2016 +0100
> # Branch stable
> # Node ID e30cb2b8e73aa5917e811e8090c7c9f72d00d998
> # Parent  8087e93914290771452ad99d6d6a7f57c7fa02c3
> summary: add "last comment" tooltip to status icon
>
> This adds a tooltip to the "number of comments" indicator, indicating who did
> the last comment and also showing the first line (up to 80 characters) of that
> last comment. This lets you quickly check who last commented and what the
> comment was about.
>
> Note that as with the previous changeset in this series this is RFC.
> A final solution should be improved in several ways:
> - Apply this same change to the changelog template
> - Move the code that this change embeds in the HTML somewhere else
> - Maybe the calculations could be done once and stored in the database?
>
> Since this is the first time that I change this sort of code (I did not even
> know what mako was an hour ago!) I send this as an RFC in the hopes of getting
> pointers to where this could be changed more cleanly. I also worry whether this
> could have some negative performance impact?
>
> diff --git a/kallithea/templates/changelog/changelog_summary_data.html b/kallithea/templates/changelog/changelog_summary_data.html
> --- a/kallithea/templates/changelog/changelog_summary_data.html
> +++ b/kallithea/templates/changelog/changelog_summary_data.html
> @@ -10,6 +10,16 @@
>           last_cm = cm
>       return last_cm
>   
> +def find_last_non_empty_comment():
> +    """find the last non empty comment"""
> +    # comments might be emtpy if they only change the status
> +    last_cm = None
> +    for cm in c.comments[cs.raw_id]:
> +        if not cm.text:
> +            continue
> +        last_cm = cm
> +    return last_cm

But that might a different one from the one giving it the color?

It seems like this will put too much different info into too little 
space. I am not convinced that is a good idea.

> +
>   def get_comment_username(cm):
>       return cm.author.username
>   %>
> @@ -49,8 +59,13 @@
>           </td>
>           <td class="compact">
>                 %if c.comments.get(cs.raw_id,[]):
> +               <%last_comment_cm = find_last_non_empty_comment()%>
>                  <div class="comments-container">
> -                   <div title="${('comments')}">
> +                   %if last_comment_cm is None:
> +                   <div title="status change">
> +                   %else:
> +                   <div title="last comment from ${get_comment_username(last_comment_cm)}: ${last_comment_cm.text[:80].splitlines()[0]} ...">
> +                   %endif

I would prefer to keep the html template well structured only have one 
div and let the title be conditional. I think that makes it easier to 
read and maintain.

>                          <a href="${c.comments[cs.raw_id][0].url()}">
>                             <i class="icon-comment"></i>${len(c.comments[cs.raw_id])}
>                          </a>

/Mads