[PATCH] Change icons on timeline

[Markus.Staab]

would be nice to have icons which are easier to distinguish ..

for example, having a star or something like that on the lower right corner (like the green tick in the closed-ticket-icons)

[anonymous]

example taken from ​http://www.famfamfam.com

[sid]

Just looked at existing icons. All seem clear except the difference between new and updated tickets. So making this ticket specific to newly created tickets.

If there are any graphic artists out there that want to draw a new icon for this, that would be appreciated!

[anonymous]

yes, i had this 2 icons in my mind .. (new + updated)

[anonymous]

what do the other devs think? any ideas?

PS: could please someone set the reporter to "Markus.Staab", sorry i missed it at the beginning

[Christian Boos]

Yes, this would be nice.

Alternatively, keep the current new ticket icon and do something different for the ticket edit one (like a dimmer background).

[Piotr Kuczynski <piotr.kuczynski@…>]

here is my proposal :)

[martinporto@…]

It would be nice set icon depend on ticket type.

[Mikael Relbe]

First of three patches to be applied on trunk r10150

[Mikael Relbe]

2nd patch

[Mikael Relbe]

3rd patch

[Mikael Relbe]

Showing the result of the three patches

[Mikael Relbe]

Here's my take at this:

Patch 1

Icon borders are merely colorized for a more distinct impression of new/edited/closed tickets.
(t6492_patch1_colorized-icon borders-r10150.patch?​)

Patch 2

Distinction made for new and edited wiki pages.
(t6492_patch2_icons-for-new-and-edited-wiki pages-r10150.patch​, note that Trac diff browser does not view a renaming of an image file.)

Patch 3

An icon on the timeline occasionally had a shadow effect, due to presentation of duplicated images; one for the DT tag and one for the A tag. Code remark stated that this was needed to avoid flickering in IE.

I removed the duplicate to get rid of the shadow effect, and tested this in IE8 without discovering flickering problems. Icons presented on the timeline now have a more consistent look-and-feel. Someone, please validate this in IE7 and IE9 browsers!
(t6492_patch3_icon-positioning-updated-r10150.patch​)

The final result looks like this:

Edited wiki page icon is unchanged (as well as attachments and changesets, not shown above). The others are altered, and the created wiki page icon is new.

[osimons]

The border took a trained eye to spot… :-) For the 'new items, I suggest using a big green '+' on the side. This is the most common way of decorating new-ness for a resource - at least what I've discovered during my travels in software & web…

[Mikael Relbe]

Replying to osimons:

The border took a trained eye to spot… :-)

Yes, I expected a response something like that ;)
I aimed for changing the coloring only, to start with…

For the 'new items, I suggest using a big green '+' on the side. This is the most common way of decorating new-ness for a resource - at least what I've discovered during my travels in software & web…

What about this suggestion?

[Mikael Relbe]

Suggestion for comment:12

[Mikael Relbe]

Patch 4 to obtain suggestion in comment:12 (apply on patch 3)

[Mikael Relbe]

However, I am not too comfortable with this suggestion since the width of the icons deviates too much…

[osimons]

I think they are nice and works well and are easy to identify. I like the icons on the sides rather than trying to stamp them on top. Red is a bold choice, a real attention grabber. For new tickets I think that works really well.

I don't get the same mental association for new wiki pages - that is more green & good to me. It is really only on t.e.o where most new pages are spam and junk… :-)

If you want to trim width, then move them partially onto the sheet? I don't know - suggest what you like best, this is mostly a matter of personal taste and ideas of what things should look like. Notoriously difficult…

[Mikael Relbe]

Replying to osimons:

I don't get the same mental association for new wiki pages - that is more green & good to me. It is really only on t.e.o where most new pages are spam and junk… :-)

Thanks for your feedback, especially about the new wiki page icon.

If you want to trim width, then move them partially onto the sheet?

Two minds, one thought…

I think the following should please most people:

(Apply patches 1 + 2 + 3 + 4b onto r10150 to get this)

[Mikael Relbe]

Suggestion for comment:15

[Mikael Relbe]

Alternative patch 4 to obtain suggestion in comment:15 (apply on patch 3)

[Mikael Relbe]

I really want this in 0.13…

[Mikael Relbe]

(Something weird happened with the status above when I just re-assigned the ticket to me…)

[Christian Boos]

Can't we do this with overlays? Keep 'wiki', 'ticket', etc. and add another class if needed 'new', 'closed', 'edited'. The last ones would add a circled '+', 'x', '!' or the like as needed.

See for example how I did something similar in #7197.

[Mikael Relbe]

To do that, I think we need to rewrite the event provider mechanism. An event is currently defined as (kind, date, author, data) tuple. When I approached #2469, I added an owner element to this tuple, just to "discover" that old variants for Trac 0.10 and 0.11 are handled as well by counting the number of elements in the tuple *sigh*

Having an event provider communicate the "status" (new/closed/edited) in general will require yet another element… which will make it even harder to handle 0.10-0.12 events provided by existing plugins.

I like what you did in #7197, but the conclusion is that I cannot see how that can be implemented without re-factoring the "timeline module".

Such a change will have a major impact on all timeline event providers in Trac 0.13 (which I've already been toying with for #2469), however care must be taken to provide backwards compatibility for existing plugins.

I'm already on it, but I fear that you will dislike it, since my approach is to define a TimelineEvent class, instead of the tuples, and have all event providers in Trac 0.13 generate lists of TimelineEvent objects. This will make it easier to extend event with a general attribute (such as owner). The "trick" will be to recognize tuples received by ancient plugins…

What do you say? Stop or go?

[Mikael Relbe]

Suggestion for overlay icons (result of t6492_icon-overlays-10150.patch​ applied on trunk r10150)

[Mikael Relbe]

Patch for overlay icons, apply on trunk r10150

[Mikael Relbe]

Ok, so I had a go on this anyway…

This is how it looks, when applying t6492_icon-overlays-10150.patch​ on trunk r10150 (skipping all other patches):

Icons for resource types (ticket, milestone (remade), wiki etc.) are overlayed with status icons (checkmark, red/black crosses). Defining a combination and placement of a type-status icon pair is only about modifying the style sheet.

Note in the example above that a new ticket has a red cross, while a reopened ticket has a black cross, same as the new wiki page status overlay.

The downside is that the hovering area of each item now excludes the icons. (Have you got any ideas on how to include the icons while hovering, without "block-out"?)

To implement this, I had to rewrite parts of the timeline module, and introduce a TimelineEvent class. The code should be backwards compatible. (I'm very sure regarding Trac 0.11-0.12 style of events, since existing event providers worked with the modified timeline module, before I adapted them to use the new TimelineEvent class instead. However, I haven't tested any Trac 0.10 plugin, using the old fashioned event definition.)

From here, highlighting owned tickets (#2469) should be a trivial task…

The filters have not been touched, they need some makeover if we want to have hierarchical settings as proposed in e.g. #9098 (and #2469 too for that matter). But that's another story.

What do you think?

[Christian Boos]

I think it's great!

In a second step, we could push the refactoring a bit further, from:

   def render(self, field, context): 
         if field == 'url': 
             return context.href.milestone(self.milestone.id) 
         elif field == 'title': 
             return tag_('Milestone %(name)s completed', 
                         name=tag.em(self.milestone.id)) 
         elif field == 'description':
             ...

to:

   def render_url(self, context): 
         return context.href.milestone(self.milestone.id) 
   def render_title(self, context): 
         return tag_('Milestone %(name)s completed', name=tag.em(self.milestone.id))
   def render_description(self, context):
             ...

The only thing I'm not quite happy with is the choice of color for the tickets ;-) "red" for new and "green" for closed as invalid feels wrong. I think everyone would be OK with a "green" checkmark for a fixed ticket. Then, a "red" "(x)" could be used for an opposite resolution (wontfix, invalid and the like). But new is maybe best expressed as neither green or red, why not "blue", like in TortoiseSvn?

[Remy Blank]

FWIW, I also like the direction this is taking, so keep pushing Mikael!

[osimons]

Thanks for your efforts, mrelbe. I like the status and visuals. I'm not so keen on this API change though…

I have a much simpler solution that only makes a slight change to the timeline api, and is non-obtrusive and we do not have to deprecate anything:

1. In the API text add a line like:

   Since 0.13 `kind` may be a tuple of `(category, status)`.
   

2. In the TimelineModule all that is needed is:

   if isinstance(kind, basestring):
       kind = (kind, None)
   

I will support Timeline as part of the ​th:XmlRpcPlugin soon (much requested feature). That is essentially a reimplementation of the TimelineModule for API-only access. It makes me think of events slighly differently, and if we were to do anything I would want to disconnect the actual raw event data from the notion of 'rendering' them - rendering should be at the discretion of the TimelineModule or my future TimelineRPC class. To me that would be worth making "breaking changes", and there should be no tag() or context or format_to() involved. I'd it much like today, but returning a Resource (so url can be obtained depending on context in the receiver of the event) and perhaps description as wiki markup so that the reciever can render it as it wants (in full, shortened, plain text etc).

This entanglement of event data & rendering goes a long way back, and is the main reason why RPC has not yet provided Timeline support.

Anyway, I'm fine with adding (category, status) as that is a more rational way to access the data, and also makes more sense for a remote client that can only use the 'ticket' part to visualize all tickets - without needing to know that 'newticket' and 'ticket' are the same. It may work for the hard-coded Trac realms that all know about, but as a plugin author I can't expect others to know that 'build' and 'brokenbuild' would be the same (Bitten).

BTW: The hypothetical 'broken' verb in last example is a good illustration for why something like the Timeline should never presume anything about the providers - as suggested in the patch for api documentation:

:param status: string stating "new", "edit", "closed", or "deleted"

[osimons]

Replying to osimons:

1. In the API text add a line like:
2. In the TimelineModule all that is needed is:

Would also need the icons, css and template changes similar to your patch of course :-)

[Mikael Relbe]

Replying to cboos:

I think it's great!

Thank you!

In a second step, we could push the refactoring a bit further, from: /…/

Good suggestion! That will be part of next patch.

The only thing I'm not quite happy with is the choice of color for the tickets ;-) /…/

Your'e right again, I did think of TSVN, but I've been focusing on coding. The next patch will nail it (I hope).

Replying to rblank:

FWIW, I also like the direction this is taking, so keep pushing Mikael!

Thank you!

Replying to osimons:

Thanks for your efforts, mrelbe. I like the status and visuals. I'm not so keen on this API change though…

I have a much simpler solution that only makes a slight change to the timeline api, and is non-obtrusive and we do not have to deprecate anything: /…/

Yes, I've been thinking about that too. But I find the code "hackish" and hard to follow with the excessive use of lists and tuples of different formats for different versions. Adding other features, such as highlighting owned tickets, will just make life even harder.

However, the changes I'm doing are backwards compatible! (That's the intention, at least…) This means that existing plugins generating events for the timeline on Trac 0.10-0.12 format will still show up on a timeline in Trac 0.13.

Using event data on the new format should be even easier than before, I think. If your problem will be the same as for Trac, i.e. to handle different versions of event data format, then have a look at _event_data() in timeline/web_ui.py (in the upcoming patch) which demonstrates how all variants are treated.

BTW: The hypothetical 'broken' verb in last example is a good illustration for why something like the Timeline should never presume anything about the providers - as suggested in the patch for api documentation:

:param status: string stating "new", "edit", "closed", or "deleted"

Ha, you got me there! ;) Thanks for addressing this.

I don't fully understand your concerns, but the reason for that is my lack of experience, I hope that's tolerated.

However, since I've already done the job, I will upload the patches soon, and perhaps we can discuss this further. (When I was ready to upload them, you had just entered comment:23, they're all ready to go…)

I really need Christian and Remy to jump in here, because, as I wrote, I am not capable of judging these matters.

[Mikael Relbe]

New patch for overlay icons, apply on trunk r10150 (replaces former patch)

[Mikael Relbe]

Round five…

The patch t6492_icon-overlays-2-10150.patch​ replaces all former patches and shall be applied on trunk r10150. (I think it is easier to handle full patches since the changes are quite major.)

The following has changed since the last version:

Icons and refactoring

• Icons modified (response to comment:21)
• Refactored timeline rendering (response to comment:21)

The icons are:

Added timeline options for ticket overlay icons

The following trac.ini options controls the appereance of closed but unfixed ticket icons on the timeline:

[timeline]
ticket_inform_unfixed = duplicate invalid
ticket_warn_unfixed = cantfix worksforme

Each option defines a list of ticket resolutions that applies to a closed, unfixed ticket that is deemed:

• non-critical for the author of it (inform),
• critical for the author of it (warn).

Such tickets are presented on the timeline with a distinct icon (e.g. a red or black stop sign).

By default, a fixed ticket is represented by a checkmark icon on the timeline.

Ok, what now regarding osimons comments in comment:23?

[Remy Blank]

I think I can see where the refactoring bothers Simon. It mixes the timeline data structure (previously a tuple, now a class) and the rendering of that data, which is only relevant when rendering as HTML.

Using a tuple as a data structure really makes extension difficult (and no, the length of the tuple is not a good type system), so I still like the idea of making the timeline event a class. However, I would leave the rendering code in the individual subsystems, and keep the event a plain data structure. Also, I would make sure not to include representation-related data in the event (or at least, not as a public API).

[osimons]

Replying to rblank:

I think I can see where the refactoring bothers Simon. It mixes the timeline data structure (previously a tuple, now a class) and the rendering of that data, which is only relevant when rendering as HTML.

As mentioned above "This entanglement of event data & rendering goes a long way back" - it has been part of Trac for many, many versions and is not a problem of the refactoring in this patch as that just does the same as what is currently done but in a slightly different form. My problem is with the API doing HTML rendering at all, and that is certainly not a problem introduced by the patch! :-)

[Mikael Relbe]

Replying to rblank:

I think I can see where the refactoring bothers Simon. It mixes the timeline data structure (previously a tuple, now a class) and the rendering of that data, which is only relevant when rendering as HTML.

/…/ I would leave the rendering code in the individual subsystems, and keep the event a plain data structure. Also, I would make sure not to include representation-related data in the event (or at least, not as a public API).

I still cannot see how to avoid presentation-related classes in the API.

The timeline module consumes events produced by other components in order to present them, presentation details must be an internal affair of each component. A common (minimal) definition has to be provided to all parties, right? (I've done SW for almost 30 years now I'm feeling a bit stupid right now, asking these silly questions — however I am new to Python and Trac.)

What about this for timeline/api.py?

class ITimelineEventProvider(Interface):

    def get_timeline_events(req, start, stop, filters):
        .... # return list of TimelineEvent objects; pure event data

    def get_timeline_renderable_events(req, start, stop, filters):
        .... # return list of TimelineRenderableEvent objects; filthy data

class TimelineEvent(object):
    ... # pure event data

class TimelineRenderableEvent(TimelineEvent):
    ... # pure event data + filthy data & methods

This means that the existing event providers shall only provide "pure" TimelineEvent objects, no carrier data for rendering. Methods for handling the renderable events has to be defined for each component, of course.

The internals of the timeline module could then use the renderable class, other consumers of events could use the pure event class.

If we put the renderable class outside of the API, how would other components import it?

Replying to osimons:

My problem is with the API doing HTML rendering at all, and that is certainly not a problem introduced by the patch! :-)

Ok, I feel a little more relaxed ;)

[osimons]

Certainly approaching "offtopic" for the this ticket about new icons, but here it goes :-)

class ITimelineEventProvider(Interface):

    def get_timeline_filters(req):
        # same

    def get_timeline_events(req, start, stop, filters):
        # return (resource, action, date, author, wiki_title, wiki_description)

The receiver (for Trac it is the TimelineModule) would then know all it needs:

• It already knows the context or how to get it, and it knows if it should render absolute urls or whatever.
• The url itself is generated from the returned resource - resource.get_resource_url(href).
• Remember also that resource have three parts: realm, id, version
• Depending on timeline settings for full event display or not, the receiver of the events calls the proper format_to() method.
• 'category' can be assumed from the resource realm, so 'kind' is not needed - only the 'action' ('status') marking any transitional information from previous version (if any) to this new version.

You'd have no problem rendering the timeline directly using this data only. Constructing wiki description will not provide quite the same level of fancy output that for instance the full changeset display - unless of course your module also provide a fancy macro that renders the same using [[Changeset(123)]] wiki content… :-)

Anything I missed? Maybe.

Anyway, as an experiment, imagine a custom receiver. Like some Twitter bot implementation for instance. It looks for timeline updates since last time, and uses some custom TwitterFormatter for title+description combination to render plain text at the number of allowed characters.

I think people would object if ITicketListener provided a rendered version of the ticket instead of the raw ticket details. In its current state, the timeline event api is unusable by anyone but the Timeline web + rss display. That is a shame in my opinion because potentially it is such a powerful interface to integrate against for the 'real-time web'.

[Mikael Relbe]

Replying to osimons:

Now I understand your intentions, thank you for taking time to elaborate on your thoughts. Topics like these— about Trac internal architecture —are currently out of my sight due to my limited knowledge of the Trac code. I actually find it quite hard to grasp the big picture regarding Trac's design, however your statement is something I really appreciate from a design point of view.

I suggest that we review and— hopefully —apply the patch t6492_icon-overlays-2-10150.patch​ introduced in comment:27, and that a separate ticket is created adressing the need for re-factoring the timeline event rendering.

[Mikael Relbe]

Quick control question: Where would you put the rendering code for a plugin that defines its own resources whose actions you would like to show up on the timeline?

[Christian Boos]

Replying to mrelbe:

Quick control question: Where would you put the rendering code for a plugin that defines its own resources whose actions you would like to show up on the timeline?

I think that's the point missing in osimons reasoning. We would either need to have an intermediate representation of content, or dedicated (pluggable) formatters for his approach to work.

Let's push this further. It's simply a problem of extensibility along two axis (N x M). Say you have N event providers (wiki, ticket, changesets, …), and M end user presentation contexts (timeline page HTML, RSS, twitter feed, xmlrpc, …).

The changeset timeline event provider is particularly interesting, as you'll quickly see​ that it doesn't lend itself at all to the pure "data" approach suggested by Simon. The highlighted lines above show the "private" data generated by the ChangesetModule.get_timeline_events() method and consumed by the ChangesetModule.render_timeline_event() method. You effectively have a high coupling between the two, but that's unavoidable if you want to retain the same degree of flexibility for deciding what ends up in the output, in this case the (changeset, timeline html) cell in that (N,M) matrix.

So far, the plugin system addresses the N axis (each subsystem provides its ITimelineEventProvider), and the M axis is dealt with inside each such provider (by looking at the ?format request parameter).

If we want to enable "real" extensibility along the M axis, constraining the timeline events to have some kind of standardized representation of data (as suggested in comment:31) would be a regression, as this would prevent rich content (like the one in the above changeset example) to be produced.

Therefore to achieve true extensibility for the formatting, we need to introduce a new "extensibility axis" for M (e.g. ITimelineEventRenderer). Such a renderer has:

• either to known about the particular details of a source (to add extra informations as needed) and the format(s),
• or it takes any kind of timeline event and only cares about the output format(s), in which case it has to deal with some kind of intermediate representation for content, amenable to any kind of output format (HTML, RSS, tweet, …) which we don't have yet (think WikiDom).

The first approach would have the extra advantage of not extracting more information than needed (e.g. imagine having to extract for each changeset the costly file change information, only to have to discard it a bit later in the renderer). Well, actually it's perhaps not "either … or" and we could probably support both, the intermediate representation can be used as a "bridge" to support a wide range of output formats.

[Mikael Relbe]

I think I am maturing since I also had a ITimelineEventRenderer in mind ;)

However, can we now please go back to the main topic for this ticket, I really want your feedback on my proposal in comment:27.

[Mikael Relbe]

New patch for overlay icons, apply on trunk r10198 (replaces all formerer patches)

[Mikael Relbe]

Suggestion for overlay icons (result of t6492_icon-overlays-3-10198.patch​ applied on trunk r10198)

[Mikael Relbe]

Flag setting introduced in t6492_icon-overlays-3-10198.patch​

[Mikael Relbe]

Round six…

Since it's late at night here, I'll have to keep this short.

The patch t6492_icon-overlays-3-10198.patch​ shall be applied on trunk r10198, thus replaces all former patches. It adds the following features (compared to the former patch presented in comment:27):

• The user may select to highlight his own changes and/or his owned tickets, which is indicated in either way by a star icon to the left. (This should take care of #2469.)

• Unread events (#9282) are marked with bold font, the green vertical bar is removed (see comment:27:ticket:9282 and onwards).

I hope you see the same complexity as I do: Lots of requested features are intertwined (at least this ticket and #2469 and #9282). My ambition is to solve them all by this suggestion.

Anyway, the timeline option form has been enriched with two flags to enable/disable highlighting of own changes and/or owned tickets:

The result is demonstrated by the following example (where the above settings apply and the user is "mikael", owing ticket 62). It also shows the visual impression when the mouse is hoovering above one event.

This ticket contains an interesting debate on whether to re-factor or not to re-factor the timeline event interface. I addressed this on the ​mail forum, and got a very good response, which I interpreted that we should do the re-factoring step-by-step. That debate is however not finished!

The important fact is that this proposal should be backwards compatible (I say should because I'm still considering myself a newbie around here…).

Anyway, this contribution sums up what I would like to add to the timeline function in Trac. At the moment. ;)

Please note that this is a suggestion, a bit rough one I would say. The icons could/should be polished, the new user selectable flags are questionable. I am all open to remarks regarding this.

(Did I say I would keep this short?)

[Mikael Relbe]

Fixes a glitch with storing and retreiving timeline flag settings, apply on former patch t6492_icon-overlays-3-10198.patch​ (see comment:37)

[mpotter@…]

Can this idea be expanded further to support custom workflows? Specifically I am desiring an icon for tickets that transition to 'Testing'.

This idea may be more appropriately filed as a new ticket to allow this ticket to be completed.

[Mikael Relbe]

Just replying to mpotter@… to show that I am still alive and kicking :)

Can this idea be expanded further to support custom workflows? Specifically I am desiring an icon for tickets that transition to 'Testing'.

Interesting idea. I will have this in mind when things settle down at work and I will able to focus on this on my spare time.

[Mikael Relbe]

Trac 0.13 announced to be released soon, but this ticket will not be finished soon…