JENKINS-40877 PR link on run details.

[imeredith]

Description

Having real issues lining up the icon.

See JENKINS-40877.

Currnenly it looks like. the following because of the vertical-align, but without the vertical-align its too high. And I cant figure out how to move it either way.

Submitter checklist

• Link to JIRA ticket in description, if appropriate.
• Change is code complete and matches issue description
• Appropriate unit or acceptance tests or explanation to why this change has no tests
• Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.
• Ran Acceptance Test Harness against PR changes.

Reviewer checklist

• Run the changes and verified the change matches the issue description
• Reviewed the code
• Verified that the appropriate tests have been written or valid explanation given

[kzantow]

@imeredith you should be able to use margin with vertical-align: middle and a specific line-height to get it positioned nicely

[michaelneale]

"Display the log"??? copy paste error?

Also, can the .url be null? What is it in the case of general SCMs like git vs github?

[imeredith]

Ah hadnt actually reviewed because I was having css issues and wnated help.

And yes the url can be null. Specifically if you have PR from an old version of github-branch-source it will be null

[brody]

If we could fix the layout of the results part of the header, it would allow more room.. but to address the overflow, I'd go with an ellipsis before the eject to Github link icon

[michaelneale]

@brody in terms of fix - do you mean that the timing info shoud bump to the right 1 column leaving even more space? or is it already moved in your screen shot?

The time values are not likely to be very big (I think they have the compact form now) so there is room to move them along if it didn't look bad.

[brody]

@michaelneale in short, yes. Branch/Commit takes up 1/4 of the width of the container, the times 1/4 of the width & the info on the right 1/2 the width. Branch will often be master which will make it perfectly balanced but will also look misweighted when theres a long branch name.

This definitely needs to be addressed down the line but the current header doesn't give us much flexibility in terms for working for both short and long branch names.

[michaelneale]

@brody ok - so you don't want to move the times along one to make it 1/6th? just to be clear, but just allow 1/4 for branch/commit ? Is this how it is now?

[i386]

@brody if you want to see this addressed soon please raise a ticket.

[scherler]

@imeredith see my old PR, there I solved the align bug you have https://github.com/jenkinsci/blueocean-plugin/pull/856/files#diff-04d14a1ec4673b18d1c8e5e71fa98952

[brody]

@michaelneale I'd be happy to move it along, so the columns are then 4-2-6, which would allow alooott more room than there is currently. My design was 3-3-6 but the implementation didn't quite stick to that.

[imeredith]

Spent to long on this, will get someone else to take a look. What needs doing is making the icon not disappear when the text ellipsis. And I still cant get the icon to center how i want.

[cliffmeyers]

@imeredith I can probably take a look at this one and nail it down once I land #895.

[cliffmeyers]

This won't have any effect unless you apply display: flex (or inline-flex)

[imeredith]

I dno't really know the implications of doing that.

[scherler]

With the following markup:

 branchSlug = (<span className="killSpace">
 +                { displayName }
 +                <a href={this.pager.branch.branch.url} target="_blank">
 +                    <Icon { ...{ style, size: 16, icon: 'launch' } } />
 +                </a>
 +            </span>);

The following css should center that icon. As @cliffmeyers said you need some flex in your life ;)

+.RunDetailsHeader-sources span.killSpace {
 +    display: flex;
 +    align-items: center;
 +    a svg {
 +        margin-left: 8px;
 +    }
 +}

[michaelneale]

functionally this looks 💯 - hopefully @cliffmeyers can shed some light on the positioning/alignment challenges and we can merge this in.

Nice one @imeredith !

[cliffmeyers]

@imeredith The vertical alignment should now be fixed. It wasn't a problem with the alignment rules, the icon was getting 3px of extra vertical space because svg elements are inline by default, and when you wrap inline elements in anchors they always get that extra little bit of fiddly space. I also removed the align-items property because it won't apply to elements that lack the display: flex|inline-flex property.

before:

after:

[cliffmeyers]

@imeredith I'm not sure if there was also an issue with some text truncation... if so, just lmk and I'll take a look at that too.

BTW, can you answer for me what kind of project I need to configure to get PR's to show up in the list? I debugged this just by dropping into the debugging and forcibly adding a pullRequest: { url: "..." } object to the run but it'd be helpful to test with real data.

[i386]

@cliffmeyers if you add https://github.com/i386/app-store-demo directly in classic you will get PRs

[michaelneale]

OK @imeredith update this to master to get ATH to pass (artifact test change) - then I think this is 🐝

[michaelneale]

🐝