1043785 - improve readability of the Failure Summary panel

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P1)

Description

[Cameron McCormack (:heycam)]

I think the Failure Summary panel is a bit hard to read at the moment.  I think it is because it is difficult to distinguish between the lines that come from the log and the bug suggestions.  On tbpl these are easily distinguished, as the bug suggestions are both indented and are shown in a different colour (due to their being links).  I find the grey background of the log lines serve to de-emphasise those lines (due to the reduced contrast between background and foreground, and despite the bold text), when in fact they should be emphasised.  I don't think the text there needs to be bold; bold text should be reserved for the matched keywords, so that you can scan the suggested bugs to find ones that have the best matches more easily.

Comment 1

[Ed Morley [:emorley]]

Yeah I agree - thank you for filing this :-)

Reading that panel to match failures against bug suggestions is something the sheriffs have to do many many times a day, so every second saved reading/grokking really helps.

Comment 2

[Jonathan French (:jfrench)]

I had a look at this; I have something that I think addresses all the points raised.

Cameron, Ed, would you like to see a 1-2px "expansion" in vertical padding to help readability also? It seems to make it easier to read, but where you might have read 9 lines of bugs previously, you would now only read 8. Or, I could leave the vertical padding as-is.

I have attached screen grabs before and after the change, and a third with extra padding.

Comment 3

[Jonathan French (:jfrench)]

Attachment: failureSummaryCurrent

Comment 4

[Jonathan French (:jfrench)]

Attachment: failureSummaryProposed

Comment 5

[Jonathan French (:jfrench)]

Attachment: failureSummaryProposedExtraPadding

Comment 6

[Cameron McCormack (:heycam)]

The indentation makes a lot of difference, thanks.  I think the vertical padding looks OK without the extra 1-2px.  One odd thing is that the suggestion that takes up two lines seems to have more padding at its top than the other lines (look at the spacing between each of the [+] buttons).

A few other points that you may or may not want to look at (as part of this bug, or at all):

* It would be good if the multi-line suggestion could use a hanging indent so that the second line of text starts where the bug number does.

* The [+] buttons don't look so great, IMO, though I don't have any concrete suggestions on what to do about them.  I do like the stars on tbpl. ;)

* I think I prefer the entire suggestion line being a link.

Comment 7

[Jonathan French (:jfrench)]

Thanks Cameron, I will go with the existing vertical padding in that case. Yes, there are weird line spacing differences even on production, between regular bug lines and strikethrough bug lines. I will try to fix that and the other items you mentioned, and put up another screen grab before opening a PR.

Comment 8

[Jonathan French (:jfrench)]

Attachment: failureSummaryProposed2

What do you think about using an icon like the pin glyph. It is less high-contrast than the [+] icon, and perhaps communicates more clearly what it does (adds the bug to the pin board).

Comment 9

[Cameron McCormack (:heycam)]

Yeah I think the pin looks better than the [+].

Comment 10

[Jonathan French (:jfrench)]

Cool on the pin icon. I've also solved that weird double-up line padding issue between the non-strike and strike list items. Every line is now evenly spaced and grouped with its accompanying error. A couple of other quick questions:

o Do you guys ever have to copy/paste this panel content? I agree it might look tidier but my concern about changing the list item text to do that hanging indent (via new divs or other means), may introduce more whitespace or line feed issues during copy/paste.

o Can you confirm broad consensus across all the sheriffs about making the entire line a link? I will try to make that change if so.

Other than that, I think we're getting close.

Comment 11

[Cameron McCormack (:heycam)]

Not being a sheriff, I'll defer to Ed on those.

Comment 12

[Jonathan French (:jfrench)]

Adding Ed as need info for that.

Comment 13

[Jonathan French (:jfrench)]

I was speaking with RyanVM on IRC and his take was that the Sheriffs copy/paste failure-summary panel content all the time, so I pose we leave that failure summary line wrap as-is for now, and not indent with more whitespace and containers.

He was uncommitted on whether or not to make the entire line blue(and a link) just because TBPL did it. But with the particular workflow in TBPL it served as a visual aid, for him to see he was starring his intended failure.

Perhaps in Treeherder there is enough clarity during the pin bug-addition process? If you still prefer all-blue content(links), I will see if it is possible.

Comment 14

[Jonathan French (:jfrench)]

It's an easy change in the template, if you want the entire bug line as a link. Though it seems it will make selection of partial lines for copy/paste more difficult and in some cases not possible, same as TBPL.

Comment 15

[Jonathan French (:jfrench)]

Attachment: failureSummaryCurrentLinks

Here's some screen grabs of the current links, and full links.

Comment 16

[Jonathan French (:jfrench)]

Attachment: failureSummaryFullLinks

Comment 17

[Ed Morley [:emorley]]

(In reply to Jonathan French (:jfrench) from comment #14)
> It's an easy change in the template, if you want the entire bug line as a
> link. Though it seems it will make selection of partial lines for copy/paste
> more difficult and in some cases not possible, same as TBPL.

The content we're copying and pasting is the log line itself, rather than the bug summary of suggestions, so making the whole line a line for the bugs doesn't affect that. I think we should just go with whichever is the most readable.

Comment 18

[Jonathan French (:jfrench)]

Clarifying w/Ed on IRC he thinks we should go with the current separate bug-number link for now, we can always make the full line a link later, if needed. I will open a PR shortly.

Comment 19

[Jonathan French (:jfrench)]

https://github.com/mozilla/treeherder-ui/pull/108

Comment 20

[Jonathan French (:jfrench)]

PR has been landed on master. I will mark fixed once pushed and checked on production.

Comment 21

[Jonathan French (:jfrench)]

I think we can be comfortable with verification on dev, prior to a later push to stage/production. It looks fine to me on dev, and all the agreed-upon changes are present. Marking fixed.