summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAnnabel Dunstone Gray <annabel.dunstone@gmail.com>2016-12-19 20:14:46 +0000
committerDouglas Barbosa Alexandre <dbalexandre@gmail.com>2016-12-21 12:21:43 -0200
commite38a6f4ab5dcfccccc3b2e490b5542e2bbe32dc9 (patch)
treec3b622ea0374de16a04d4e13c77a845edd092630
parent6345aa8aface11aacd579ab0eacb77cde849eb6c (diff)
downloadgitlab-ce-e38a6f4ab5dcfccccc3b2e490b5542e2bbe32dc9.tar.gz
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
-rw-r--r--app/assets/stylesheets/pages/pipelines.scss74
-rw-r--r--app/views/ci/status/_graph_badge.html.haml9
-rw-r--r--app/views/projects/stage/_graph.html.haml11
-rw-r--r--app/views/projects/stage/_in_stage_group.html.haml4
-rw-r--r--changelogs/unreleased/pipeline-build-hitbox.yml4
-rw-r--r--spec/features/projects/pipelines/pipeline_spec.rb33
-rw-r--r--spec/javascripts/fixtures/pipeline_graph.html.haml9
7 files changed, 68 insertions, 76 deletions
diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss
index 7905d2f2e7c..c9d54b4f3d3 100644
--- a/app/assets/stylesheets/pages/pipelines.scss
+++ b/app/assets/stylesheets/pages/pipelines.scss
@@ -428,7 +428,7 @@
width: 21px;
height: 25px;
position: absolute;
- top: -32px;
+ top: -31px;
border-top: 2px solid $border-color;
}
@@ -456,32 +456,31 @@
}
.build {
- border: 1px solid $border-color;
- border-radius: 30px;
- background-color: $white-light;
position: relative;
- padding: 8px 4px 9px 10px;
width: 186px;
margin-bottom: 10px;
white-space: normal;
+ color: $gl-text-color-light;
- &:hover {
- background-color: $stage-hover-bg;
- border: 1px solid $stage-hover-border;
+ > .build-content {
+ display: inline-block;
+ padding: 8px 10px 9px;
+ width: 100%;
+ border: 1px solid $border-color;
+ border-radius: 30px;
+ background-color: $white-light;
- a,
- .dropdown-counter-badge,
- .dropdown-menu-toggle {
+ &:hover {
+ background-color: $stage-hover-bg;
+ border: 1px solid $stage-hover-border;
color: $gl-text-color;
}
+ }
- .grouped-pipeline-dropdown a {
- color: $gl-text-color-light;
-
- &:hover {
- color: $gl-text-color;
- }
- }
+ > .ci-action-icon-container {
+ position: absolute;
+ right: 4px;
+ top: 5px;
}
.ci-status-icon {
@@ -621,8 +620,8 @@
padding: 0;
width: 191px;
left: auto;
- right: -206px;
- top: -11px;
+ right: -195px;
+ top: -4px;
box-shadow: 0 1px 5px $black-transparent;
a {
@@ -650,30 +649,20 @@
.dropdown-build {
color: $gl-text-color-light;
- a.ci-action-icon-container {
- padding: 0;
- 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;
+ .build-content {
+ width: 100%;
}
.ci-action-icon-container {
+ font-size: 11px;
+ position: absolute;
+ right: 4px;
+
i {
width: 25px;
height: 25px;
+ font-size: 11px;
+ margin-top: 0;
&::before {
top: 1px;
@@ -682,6 +671,12 @@
}
}
+ &:hover {
+ background-color: $stage-hover-bg;
+ border-radius: 3px;
+ color: $gl-text-color;
+ }
+
.stage {
max-width: 100px;
width: 100px;
@@ -704,9 +699,6 @@
// Action Icons
.ci-action-icon-container .ci-action-icon-wrapper {
- float: right;
- margin-top: -4px;
-
i {
color: $border-color;
border-radius: 100%;
diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml
index 9f3a9c0c6b2..52b4d77d074 100644
--- a/app/views/ci/status/_graph_badge.html.haml
+++ b/app/views/ci/status/_graph_badge.html.haml
@@ -5,12 +5,13 @@
- klass = "ci-status-icon ci-status-icon-#{status.group}"
- 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)
- .ci-status-text= subject.name
+ .ci-status-text{ 'data-toggle' => 'tooltip', 'data-title' => "#{subject.name} - #{status.label}" }= subject.name
- else
- %span{ class: klass }= custom_icon(status.icon)
- .ci-status-text= subject.name
+ .build-content
+ %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?
= link_to status.action_path, method: status.action_method,
diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml
index b70b574e687..6919b210a00 100644
--- a/app/views/projects/stage/_graph.html.haml
+++ b/app/views/projects/stage/_graph.html.haml
@@ -10,12 +10,11 @@
- status_groups.each do |group_name, grouped_statuses|
- if grouped_statuses.one?
- status = grouped_statuses.first
- %li.build
+ %li.build{ 'id' => "ci-badge-#{group_name}" }
.curve
- .build-content
- = render 'ci/status/graph_badge', subject: status
+ = render 'ci/status/graph_badge', subject: status
- else
- %li.build
+ %li.build{ 'id' => "ci-badge-#{group_name}" }
.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
+
diff --git a/app/views/projects/stage/_in_stage_group.html.haml b/app/views/projects/stage/_in_stage_group.html.haml
index b03837d1211..b15f7eaeab2 100644
--- a/app/views/projects/stage/_in_stage_group.html.haml
+++ b/app/views/projects/stage/_in_stage_group.html.haml
@@ -1,8 +1,8 @@
- 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}"}
= ci_icon_for_status(group_status)
- %span.ci-status-text
+ %span.ci-status-text{ 'data-toggle' => 'tooltip', 'data-title' => "#{name} - #{group_status}" }
= name
%span.dropdown-counter-badge= subject.size
.dropdown-menu.grouped-pipeline-dropdown
diff --git a/changelogs/unreleased/pipeline-build-hitbox.yml b/changelogs/unreleased/pipeline-build-hitbox.yml
new file mode 100644
index 00000000000..051b538a9a3
--- /dev/null
+++ b/changelogs/unreleased/pipeline-build-hitbox.yml
@@ -0,0 +1,4 @@
+---
+title: Make CI badge hitboxes match parent
+merge_request:
+author:
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 57f1e75ea2c..1210e2745db 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -19,7 +19,7 @@ describe "Pipelines", feature: true, js: true do
@success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build')
@failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test')
@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')
end
@@ -41,37 +41,34 @@ describe "Pipelines", feature: true, js: true do
describe 'pipeline graph' do
context 'when pipeline has running builds' 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_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_content('deploy')
end
end
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')
end
end
context 'when pipeline has successful builds' do
- it 'shows the success icon and a retry action for the successfull build' do
- page.within('a[data-title="build - passed"]') do
+ it 'shows the success icon and a retry action for the successful build' do
+ page.within('#ci-badge-build') do
expect(page).to have_selector('.ci-status-icon-success')
expect(page).to have_content('build')
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')
end
end
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')
end
@@ -79,18 +76,18 @@ describe "Pipelines", feature: true, js: true do
context 'when pipeline has failed builds' 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_content('test')
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')
end
end
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')
end
@@ -98,18 +95,18 @@ describe "Pipelines", feature: true, js: true do
context 'when pipeline has manual builds' 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_content('manual')
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')
end
end
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')
end
@@ -167,7 +164,7 @@ describe "Pipelines", feature: true, js: true do
@success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build')
@failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test')
@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')
end
diff --git a/spec/javascripts/fixtures/pipeline_graph.html.haml b/spec/javascripts/fixtures/pipeline_graph.html.haml
index deca50ceaa7..c0b5ab4411e 100644
--- a/spec/javascripts/fixtures/pipeline_graph.html.haml
+++ b/spec/javascripts/fixtures/pipeline_graph.html.haml
@@ -8,8 +8,7 @@
%ul
%li.build
.curve
- .build-content
- %a
- %svg
- .ci-status-text
- stop_review
+ %a
+ %svg
+ .ci-status-text
+ stop_review