Commit f2945170 authored by Annabel Dunstone Gray's avatar Annabel Dunstone Gray

Merge branch 'pipeline-build-hitbox' into 'master'

Make CI badge hitboxes better match container

## What does this MR do?

Makes the Pipeline graph badge hitboxes accurately match their containers.
Currently, there's an issue where the badge highlights, as though, it's clickable, but clicking it does nothing:

![before](/uploads/668f841c640bc586044c89bed9f1e1e9/before.gif)

This is due to the `<a>` and `<button>` elements being padded into the parent elements. So, the parent would react to a cursor, but the cursor wouldn't be on the `<a>`/`<button>` yet.

![before-hitbox](/uploads/6ebaaa4be1096fbfc37f8d73da492c7b/before-hitbox.png)

## Are there points in the code the reviewer needs to double check?

Maybe the tests? I kept the style changes as minimal as possible.

## Why was this MR needed?

I was annoyed every time I clicked on a badge that I was _clearly on_ (it highlighted), but nothing happened.
If I found it irritating, perhaps someone else did too.

## Screenshots (if relevant)

How it behaves in the PR:
![finished](/uploads/c26fabe37ccf17f171f40b48522702b2/finished.gif)

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- Tests
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?
Closes #25547 

See merge request !8156
parents 75b64deb 66ff2ded
...@@ -428,7 +428,7 @@ ...@@ -428,7 +428,7 @@
width: 21px; width: 21px;
height: 25px; height: 25px;
position: absolute; position: absolute;
top: -32px; top: -31px;
border-top: 2px solid $border-color; border-top: 2px solid $border-color;
} }
...@@ -456,32 +456,31 @@ ...@@ -456,32 +456,31 @@
} }
.build { .build {
border: 1px solid $border-color;
border-radius: 30px;
background-color: $white-light;
position: relative; position: relative;
padding: 8px 4px 9px 10px;
width: 186px; width: 186px;
margin-bottom: 10px; margin-bottom: 10px;
white-space: normal; white-space: normal;
color: $gl-text-color-light;
&:hover { > .build-content {
background-color: $stage-hover-bg; display: inline-block;
border: 1px solid $stage-hover-border; padding: 8px 10px 9px;
width: 100%;
border: 1px solid $border-color;
border-radius: 30px;
background-color: $white-light;
a, &:hover {
.dropdown-counter-badge, background-color: $stage-hover-bg;
.dropdown-menu-toggle { border: 1px solid $stage-hover-border;
color: $gl-text-color; color: $gl-text-color;
} }
}
.grouped-pipeline-dropdown a { > .ci-action-icon-container {
color: $gl-text-color-light; position: absolute;
right: 4px;
&:hover { top: 5px;
color: $gl-text-color;
}
}
} }
.ci-status-icon { .ci-status-icon {
...@@ -621,8 +620,8 @@ ...@@ -621,8 +620,8 @@
padding: 0; padding: 0;
width: 191px; width: 191px;
left: auto; left: auto;
right: -206px; right: -195px;
top: -11px; top: -4px;
box-shadow: 0 1px 5px $black-transparent; box-shadow: 0 1px 5px $black-transparent;
a { a {
...@@ -650,30 +649,20 @@ ...@@ -650,30 +649,20 @@
.dropdown-build { .dropdown-build {
color: $gl-text-color-light; color: $gl-text-color-light;
a.ci-action-icon-container { .build-content {
padding: 0; width: 100%;
font-size: 11px;
float: right;
margin-top: 4px;
display: inline-block;
position: relative;
i {
font-size: 11px;
margin-top: 0;
}
}
&:hover {
background-color: $stage-hover-bg;
border-radius: 3px;
color: $gl-text-color;
} }
.ci-action-icon-container { .ci-action-icon-container {
font-size: 11px;
position: absolute;
right: 4px;
i { i {
width: 25px; width: 25px;
height: 25px; height: 25px;
font-size: 11px;
margin-top: 0;
&::before { &::before {
top: 1px; top: 1px;
...@@ -682,6 +671,12 @@ ...@@ -682,6 +671,12 @@
} }
} }
&:hover {
background-color: $stage-hover-bg;
border-radius: 3px;
color: $gl-text-color;
}
.stage { .stage {
max-width: 100px; max-width: 100px;
width: 100px; width: 100px;
...@@ -704,9 +699,6 @@ ...@@ -704,9 +699,6 @@
// Action Icons // Action Icons
.ci-action-icon-container .ci-action-icon-wrapper { .ci-action-icon-container .ci-action-icon-wrapper {
float: right;
margin-top: -4px;
i { i {
color: $border-color; color: $border-color;
border-radius: 100%; border-radius: 100%;
......
...@@ -5,12 +5,13 @@ ...@@ -5,12 +5,13 @@
- klass = "ci-status-icon ci-status-icon-#{status.group}" - klass = "ci-status-icon ci-status-icon-#{status.group}"
- if status.has_details? - if status.has_details?
= link_to status.details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{status.label}" } do = link_to status.details_path, class: 'build-content' do
%span{ class: klass }= custom_icon(status.icon) %span{ class: klass }= custom_icon(status.icon)
.ci-status-text= subject.name .ci-status-text{ 'data-toggle' => 'tooltip', 'data-title' => "#{subject.name} - #{status.label}" }= subject.name
- else - else
%span{ class: klass }= custom_icon(status.icon) .build-content
.ci-status-text= subject.name %span{ class: klass }= custom_icon(status.icon)
.ci-status-text{ 'data-toggle' => 'tooltip', 'data-title' => "#{subject.name} - #{status.label}" }= subject.name
- if status.has_action? - if status.has_action?
= link_to status.action_path, method: status.action_method, = link_to status.action_path, method: status.action_method,
......
...@@ -10,12 +10,11 @@ ...@@ -10,12 +10,11 @@
- status_groups.each do |group_name, grouped_statuses| - status_groups.each do |group_name, grouped_statuses|
- if grouped_statuses.one? - if grouped_statuses.one?
- status = grouped_statuses.first - status = grouped_statuses.first
%li.build %li.build{ 'id' => "ci-badge-#{group_name}" }
.curve .curve
.build-content = render 'ci/status/graph_badge', subject: status
= render 'ci/status/graph_badge', subject: status
- else - else
%li.build %li.build{ 'id' => "ci-badge-#{group_name}" }
.curve .curve
.dropdown.inline.build-content = render 'projects/stage/in_stage_group', name: group_name, subject: grouped_statuses
= render 'projects/stage/in_stage_group', name: group_name, subject: grouped_statuses
- group_status = CommitStatus.where(id: subject).status - group_status = CommitStatus.where(id: subject).status
%button.dropdown-menu-toggle.has-tooltip{ type: 'button', data: { toggle: 'dropdown', title: "#{name} - #{group_status}" } } %button.dropdown-menu-toggle.build-content.has-tooltip{ type: 'button', data: { toggle: 'dropdown'} }
%span{class: "ci-status-icon ci-status-icon-#{group_status}"} %span{class: "ci-status-icon ci-status-icon-#{group_status}"}
= ci_icon_for_status(group_status) = ci_icon_for_status(group_status)
%span.ci-status-text %span.ci-status-text{ 'data-toggle' => 'tooltip', 'data-title' => "#{name} - #{group_status}" }
= name = name
%span.dropdown-counter-badge= subject.size %span.dropdown-counter-badge= subject.size
.dropdown-menu.grouped-pipeline-dropdown .dropdown-menu.grouped-pipeline-dropdown
......
---
title: Make CI badge hitboxes match parent
merge_request:
author:
...@@ -19,7 +19,7 @@ describe "Pipelines", feature: true, js: true do ...@@ -19,7 +19,7 @@ describe "Pipelines", feature: true, js: true do
@success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build')
@failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test')
@running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy')
@manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual build') @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual-build')
@external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external')
end end
...@@ -41,37 +41,34 @@ describe "Pipelines", feature: true, js: true do ...@@ -41,37 +41,34 @@ describe "Pipelines", feature: true, js: true do
describe 'pipeline graph' do describe 'pipeline graph' do
context 'when pipeline has running builds' do context 'when pipeline has running builds' do
it 'shows a running icon and a cancel action for the running build' do it 'shows a running icon and a cancel action for the running build' do
page.within('a[data-title="deploy - running"]') do page.within('#ci-badge-deploy') do
expect(page).to have_selector('.ci-status-icon-running') expect(page).to have_selector('.ci-status-icon-running')
expect(page).to have_content('deploy')
end
page.within('a[data-title="deploy - running"] + .ci-action-icon-container') do
expect(page).to have_selector('.ci-action-icon-container .fa-ban') expect(page).to have_selector('.ci-action-icon-container .fa-ban')
expect(page).to have_content('deploy')
end end
end end
it 'should be possible to cancel the running build' do it 'should be possible to cancel the running build' do
find('a[data-title="deploy - running"] + .ci-action-icon-container').trigger('click') find('#ci-badge-deploy .ci-action-icon-container').trigger('click')
expect(page).not_to have_content('Cancel running') expect(page).not_to have_content('Cancel running')
end end
end end
context 'when pipeline has successful builds' do context 'when pipeline has successful builds' do
it 'shows the success icon and a retry action for the successfull build' do it 'shows the success icon and a retry action for the successful build' do
page.within('a[data-title="build - passed"]') do page.within('#ci-badge-build') do
expect(page).to have_selector('.ci-status-icon-success') expect(page).to have_selector('.ci-status-icon-success')
expect(page).to have_content('build') expect(page).to have_content('build')
end end
page.within('a[data-title="build - passed"] + .ci-action-icon-container') do page.within('#ci-badge-build .ci-action-icon-container') do
expect(page).to have_selector('.ci-action-icon-container .fa-refresh') expect(page).to have_selector('.ci-action-icon-container .fa-refresh')
end end
end end
it 'should be possible to retry the success build' do it 'should be possible to retry the success build' do
find('a[data-title="build - passed"] + .ci-action-icon-container').trigger('click') find('#ci-badge-build .ci-action-icon-container').trigger('click')
expect(page).not_to have_content('Retry build') expect(page).not_to have_content('Retry build')
end end
...@@ -79,18 +76,18 @@ describe "Pipelines", feature: true, js: true do ...@@ -79,18 +76,18 @@ describe "Pipelines", feature: true, js: true do
context 'when pipeline has failed builds' do context 'when pipeline has failed builds' do
it 'shows the failed icon and a retry action for the failed build' do it 'shows the failed icon and a retry action for the failed build' do
page.within('a[data-title="test - failed"]') do page.within('#ci-badge-test') do
expect(page).to have_selector('.ci-status-icon-failed') expect(page).to have_selector('.ci-status-icon-failed')
expect(page).to have_content('test') expect(page).to have_content('test')
end end
page.within('a[data-title="test - failed"] + .ci-action-icon-container') do page.within('#ci-badge-test .ci-action-icon-container') do
expect(page).to have_selector('.ci-action-icon-container .fa-refresh') expect(page).to have_selector('.ci-action-icon-container .fa-refresh')
end end
end end
it 'should be possible to retry the failed build' do it 'should be possible to retry the failed build' do
find('a[data-title="test - failed"] + .ci-action-icon-container').trigger('click') find('#ci-badge-test .ci-action-icon-container').trigger('click')
expect(page).not_to have_content('Retry build') expect(page).not_to have_content('Retry build')
end end
...@@ -98,18 +95,18 @@ describe "Pipelines", feature: true, js: true do ...@@ -98,18 +95,18 @@ describe "Pipelines", feature: true, js: true do
context 'when pipeline has manual builds' do context 'when pipeline has manual builds' do
it 'shows the skipped icon and a play action for the manual build' do it 'shows the skipped icon and a play action for the manual build' do
page.within('a[data-title="manual build - manual play action"]') do page.within('#ci-badge-manual-build') do
expect(page).to have_selector('.ci-status-icon-manual') expect(page).to have_selector('.ci-status-icon-manual')
expect(page).to have_content('manual') expect(page).to have_content('manual')
end end
page.within('a[data-title="manual build - manual play action"] + .ci-action-icon-container') do page.within('#ci-badge-manual-build .ci-action-icon-container') do
expect(page).to have_selector('.ci-action-icon-container .fa-play') expect(page).to have_selector('.ci-action-icon-container .fa-play')
end end
end end
it 'should be possible to play the manual build' do it 'should be possible to play the manual build' do
find('a[data-title="manual build - manual play action"] + .ci-action-icon-container').trigger('click') find('#ci-badge-manual-build .ci-action-icon-container').trigger('click')
expect(page).not_to have_content('Play build') expect(page).not_to have_content('Play build')
end end
...@@ -167,7 +164,7 @@ describe "Pipelines", feature: true, js: true do ...@@ -167,7 +164,7 @@ describe "Pipelines", feature: true, js: true do
@success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build')
@failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test')
@running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy')
@manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual build') @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual-build')
@external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external')
end end
......
...@@ -8,8 +8,7 @@ ...@@ -8,8 +8,7 @@
%ul %ul
%li.build %li.build
.curve .curve
.build-content %a
%a %svg
%svg .ci-status-text
.ci-status-text stop_review
stop_review
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment