diff options
author | Scott Hampton <shampton@gitlab.com> | 2019-08-23 21:28:46 +0000 |
---|---|---|
committer | Mayra Cabrera <mcabrera@gitlab.com> | 2019-08-23 21:28:46 +0000 |
commit | f093ceb4a0b15420cfe64fa9d97e73531b5fb35c (patch) | |
tree | 06df7eaa8b947b6fb9a06eb25d5e7be82e351287 | |
parent | 1d462d23a94e6ee8a0151444a01ea5858350886b (diff) | |
download | gitlab-ce-f093ceb4a0b15420cfe64fa9d97e73531b5fb35c.tar.gz |
Change misleading pipeline status tooltip
Some pipeline status icon tooltips were showing
"Commit: ..." which customers found to be
misleading since it was not the commit that was
failing but the pipeline.
We are changing all status icon tooltips to say
"Pipeline: ..." instead of "Commit: ..." now.
9 files changed, 93 insertions, 11 deletions
diff --git a/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue b/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue index 03281aa1317..12ee1ce2f0c 100644 --- a/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue +++ b/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue @@ -38,7 +38,9 @@ export default { }, computed: { statusTitle() { - return sprintf(s__('Commits|Commit: %{commitText}'), { commitText: this.ciStatus.text }); + return sprintf(s__('PipelineStatusTooltip|Pipeline: %{ciStatus}'), { + ciStatus: this.ciStatus.text, + }); }, }, mounted() { diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index f2b5b82b013..144df676304 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -105,14 +105,13 @@ module CiStatusHelper path = pipelines_project_commit_path(project, commit, ref: ref) render_status_with_link( - 'commit', commit.status(ref), path, tooltip_placement: tooltip_placement, icon_size: 24) end - def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) + def render_status_with_link(status, path = nil, type: _('pipeline'), tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) klass = "ci-status-link ci-status-icon-#{status.dasherize} d-inline-flex #{cssclass}" title = "#{type.titleize}: #{ci_label_for_status(status)}" data = { toggle: 'tooltip', placement: tooltip_placement, container: container } diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 1249b98221f..fdaacb732c7 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,13 +1,10 @@ - status = local_assigns.fetch(:status) - size = local_assigns.fetch(:size, 16) -- type = local_assigns.fetch(:type, 'pipeline') - tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") - path = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil) - option_css_classes = local_assigns.fetch(:option_css_classes, '') - css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip #{option_css_classes}" - title = s_("PipelineStatusTooltip|Pipeline: %{ci_status}") % {ci_status: status.label} -- if type == 'commit' - - title = s_("PipelineStatusTooltip|Commit: %{ci_status}") % {ci_status: status.label} - if path = link_to path, class: css_classes, title: title, data: { placement: tooltip_placement } do diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index b7474d891dc..573ed36d7f4 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -89,7 +89,7 @@ - if pipeline_status && can?(current_user, :read_cross_project) && project.pipeline_status.has_status? && can?(current_user, :read_build, project) - pipeline_path = pipelines_project_commit_path(project.pipeline_status.project, project.pipeline_status.sha, ref: project.pipeline_status.ref) %span.icon-wrapper.pipeline-status - = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top', path: pipeline_path + = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), tooltip_placement: 'top', path: pipeline_path .updated-note %span = _('Updated') diff --git a/changelogs/unreleased/55999-misleading-pipeline-tooltip-messages-and-misleading-ci-status-icons.yml b/changelogs/unreleased/55999-misleading-pipeline-tooltip-messages-and-misleading-ci-status-icons.yml new file mode 100644 index 00000000000..a937614be38 --- /dev/null +++ b/changelogs/unreleased/55999-misleading-pipeline-tooltip-messages-and-misleading-ci-status-icons.yml @@ -0,0 +1,5 @@ +--- +title: Remove "Commit" from pipeline status tooltips +merge_request: 31861 +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d57ab4bf66f..f8cebc22d93 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8026,7 +8026,7 @@ msgstr "" msgid "PipelineSheduleIntervalPattern|Custom" msgstr "" -msgid "PipelineStatusTooltip|Commit: %{ci_status}" +msgid "PipelineStatusTooltip|Pipeline: %{ciStatus}" msgstr "" msgid "PipelineStatusTooltip|Pipeline: %{ci_status}" @@ -13943,6 +13943,9 @@ msgstr "" msgid "pending comment" msgstr "" +msgid "pipeline" +msgstr "" + msgid "private" msgstr "" diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index e2100c8562b..e4728f37217 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -169,7 +169,7 @@ describe 'Dashboard Projects' do expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']") expect(page).to have_css('.ci-status-link') expect(page).to have_css('.ci-status-icon-success') - expect(page).to have_link('Commit: passed') + expect(page).to have_link('Pipeline: passed') end end @@ -189,7 +189,7 @@ describe 'Dashboard Projects' do expect(page).not_to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']") expect(page).not_to have_css('.ci-status-link') expect(page).not_to have_css('.ci-status-icon-success') - expect(page).not_to have_link('Commit: passed') + expect(page).not_to have_link('Pipeline: passed') end end end diff --git a/spec/features/projects/show/user_sees_last_commit_ci_status_spec.rb b/spec/features/projects/show/user_sees_last_commit_ci_status_spec.rb index a1cad261875..fdc238d55cf 100644 --- a/spec/features/projects/show/user_sees_last_commit_ci_status_spec.rb +++ b/spec/features/projects/show/user_sees_last_commit_ci_status_spec.rb @@ -18,7 +18,7 @@ describe 'Projects > Show > User sees last commit CI status' do page.within '.blob-commit-info' do expect(page).to have_content(project.commit.sha[0..6]) - expect(page).to have_link('Commit: skipped') + expect(page).to have_link('Pipeline: skipped') end end end diff --git a/spec/helpers/ci_status_helper_spec.rb b/spec/helpers/ci_status_helper_spec.rb index bc2422aba90..4f665dc0514 100644 --- a/spec/helpers/ci_status_helper_spec.rb +++ b/spec/helpers/ci_status_helper_spec.rb @@ -53,4 +53,80 @@ describe CiStatusHelper do expect(helper.pipeline_status_cache_key(pipeline_status)).to eq("pipeline-status/123abc-success") end end + + describe "#render_status_with_link" do + subject { helper.render_status_with_link("success") } + + it "renders a passed status icon" do + is_expected.to include("<span class=\"ci-status-link ci-status-icon-success d-inline-flex") + end + + it "has 'Pipeline' as the status type in the title" do + is_expected.to include("title=\"Pipeline: passed\"") + end + + it "has the success status icon" do + is_expected.to include("ci-status-icon-success") + end + + context "when pipeline has commit path" do + subject { helper.render_status_with_link("success", "/commit-path") } + + it "links to commit" do + is_expected.to include("href=\"/commit-path\"") + end + + it "does not contain a span element" do + is_expected.not_to include("<span") + end + + it "has 'Pipeline' as the status type in the title" do + is_expected.to include("title=\"Pipeline: passed\"") + end + + it "has the correct status icon" do + is_expected.to include("ci-status-icon-success") + end + end + + context "when different type than pipeline is provided" do + subject { helper.render_status_with_link("success", type: "commit") } + + it "has the provided type in the title" do + is_expected.to include("title=\"Commit: passed\"") + end + end + + context "when tooltip_placement is provided" do + subject { helper.render_status_with_link("success", tooltip_placement: "right") } + + it "has the provided tooltip placement" do + is_expected.to include("data-placement=\"right\"") + end + end + + context "when additional CSS classes are provided" do + subject { helper.render_status_with_link("success", cssclass: "extra-class") } + + it "has appended extra class to icon classes" do + is_expected.to include("class=\"ci-status-link ci-status-icon-success d-inline-flex extra-class\"") + end + end + + context "when container is provided" do + subject { helper.render_status_with_link("success", container: "my-container") } + + it "has the provided container in data" do + is_expected.to include("data-container=\"my-container\"") + end + end + + context "when icon_size is provided" do + subject { helper.render_status_with_link("success", icon_size: 24) } + + it "has the svg class to change size" do + is_expected.to include("<svg class=\"s24\">") + end + end + end end |