1149687 - Strange layout (with flexbox) in the list of links in the job results pane on profiled Talos runs

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P2)

Description

[Markus Stange [:mstange]]

Attachment: screenshot

On this push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=212ea0836d66

If you select one of the Linux x64 "o" Talos jobs, the stuff in the bottom pane looks strange. The list items of the "Open ... in cleopatra" links should be below each other, instead of besides each other.

Comment 1

[Daniel Holbert [:dholbert]]

The container for all those "open...in cleopatra" links is a row-oriented flex container right now.

For its children to stack vertically, you need to either give it:
 - flex-direction: column
...or probably-better...
 - display:block
(since it's just a bulleted list, and you're not actually making use of its flexiness at all. Flex containers have complicated perf properties, and if you don't actually need to make the "open in cleopatra" align or flex in any particular flexbox-defined way, then a block is probably better.)

Comment 2

[Daniel Holbert [:dholbert]]

The "Datazilla/Graphserver" bulleted <ul>-list doesn't seem to have "display:flex" -- it's just a block (as I'd recommend, per end of comment 1).

So, I suspect the other <ul> is getting *accidentally* styled as a flex container.  It's getting that style from this complicated CSS selector:
#bottom-center-bottom > *, #bottom-center-bottom > * > *, #bottom-center-bottom > * > * > *

I'll bet that style isn't intended to target this <ul> element (though it does).

If you put a wrapper <span> or <div> around it (as we have around the "Datazilla/Graphserver" list), then I think the CSS selector would target that wrapper instead of targeting the <ul> itself. Maybe that's what was intended here?

Comment 3

[Jonathan French (:jfrench)]

Attachment: spanApproachProposed

With the span/div addition, we'd have two columns for the data (attached) if you guys prefer that.

Otherwise yes, perhaps we could override the css and make the ul display:block and have everything in one tall single column.

I guess it depends on how narrow Talos users expect to work with the browser vs. having data off screen perhaps more frequently.

Comment 4

[Daniel Holbert [:dholbert]]

The <ul> should already be display:block -- the only reason it wasn't is because it was at an unexpected nesting-level, I think, and so the selector in comment 2 was hitting it when it wasn't intended to.

If you want everything to render in a single column, you'd want to just put both <ul> elements in the same <span>, instead of two different spans (& instead of having the second <ul> be a sibling of the first <span>, as is the case right now).

Comment 5

[Jonathan French (:jfrench)]

Attachment: proposed

Right, here's that look if you guys would like single column instead.

Comment 6

[Daniel Holbert [:dholbert]]

mstange said it used to look like that (single column for the bulleted lists), BTW - he linked to:
https://wiki.mozilla.org/images/0/04/Treeherder_talos_profiling_cleopatra_links.png

(I'm not sure if the switch to the current UI -- moving in the direction of two columns -- was intentional or not.)

Comment 7

[Jonathan French (:jfrench)]

I think single column makes more sense, agreed. I'll put up a PR momentarily so we have something workable. We can always tune/tweak later. Thanks for the guidance Daniel :)

Comment 8

[Jonathan French (:jfrench)]

Turns out wlach and I were working on this in parallel, so re-assigning :)

Comment 9

[William Lachance (:wlach)]

Attachment: Temporary fix

Temporary fix until root cause (bug 1094466) is fixed. This is probably a small improvement in any case though, as it makes a bit more sense IMO in how we separate the different elements.

Comment 10

[Jonathan French (:jfrench)]

Comment on attachment 8586395 [details] [review]
Temporary fix

Looks good, and I also tested the branch locally and it seems fine.

Comment 11

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/288d11413c5d10e825a94b03832275584925a79b
Bug 1149687 - SPS profile links shouldn't be in horizontal flex flow

Comment 12

[William Lachance (:wlach)]

Cool, going to resolve this bug for now.

Comment 13

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/beed62e9d2bd1bf3bb070514e287bf756447aef8
Bug 1149687 - SPS profile links shouldn't be in horizontal flex flow