Created attachment 317678 [details] [review]
chatView: Clean up handling of interactive tags

With the new status grouping where the header can be clicked to reveal
collapsed status messages, we now have a second text tag that responds
to input besides clickable links. This makes the code that handles them
centrally quite awkward, as we now need to differentiate not only on the
type of events, but also the tag type interacted with.
Clean this up by splitting out the generally useful event handling into
a ButtonTag class (with some help in TextView for hover-tracking), and
use its ::clicked, ::button-press-event and ::button-release-event signals
to implement links and status group headers and a much more natural way.

We should use _createHeaderTag function just like _createUrlTag as header tag will be listening to hover as well as click? I have taken care of this in my patch of bug 759032, .


Can you please rebase it and push. chatView was modified with commit 452017147. I'd rebase the patches of 759032 on top of it than.

Created attachment 317791 [details] [review]
chatView: Clean up handling of interactive tags

(In reply to Kunaal Jain from comment #2)
> We should use _createHeaderTag function just like _createUrlTag as header
> tag will be listening to hover as well as click?

I split out _createUrlTag() because _insertMessage() is already fairly unwieldy as-is, and in particular the right-click handling distracts from the overall control flow of _insertMessage(). That's much less of a concern for _updateStatusHeader() IMHO, even when we add a 'notify::hover' handler (it's obviously not *wrong* to split that out as well, I just don't think it's necessary).

Attachment 317791 [details] pushed as 6ba4640 - chatView: Clean up handling of interactive tags