Skip to content

Show the first hint popover after selecting a template type

Paul Gascou-Vaillancourt requested to merge fix-suggest-gitlab-ci-popover into master

What does this MR do?

This improves the UX of the tour that guides user through adding a new file to a repository and applying .gitlab-ci.yml templates.

This also potentially fixes a flaky feature spec that has been failing due to a popover that wasn't properly hidden in some scenarios: #299393 (closed)

Currently, the first step in the tour only appears when hovering over or manually focusing the template selector.

This MR implements two possible approaches to fix this:

Keeping the manual trigger

!58120 (3ea40c1a)

The popover currently has triggers set to manual, which might indicate that original intent was to keep the popover opened as long the user hasn't manually dismissed it. This seems to be contradicted by the feature specs that seem to expect the popover to become hidden implicitly after selecting a template.

This approach requires the most changes as it simply can't work in the current state of things: since the template selector only appears once a template type has been selected, we need to wait until the target is actually visible before showing the popover.

Switch to default triggers

!58120 (diffs)

This approach is the one currently implemented in this MR. It requires the least amount of changes as it involves using GlPopover's default triggers (focus hover) to show each step in the tour. This means that we only need to make sure that the target element gets the focus when needed.

Besides being simple to implement, this approach seems to be doing what the feature specs expect, which might in turn resolve the flaky issues we've been seeing over the last few months 🤷

How to test this?

  1. Apply the following patch to force the tour to start:
diff --git a/app/views/projects/blob/_editor.html.haml b/app/views/projects/blob/_editor.html.haml
index 7d3a0c4a026..3b218f280d1 100644
--- a/app/views/projects/blob/_editor.html.haml
+++ b/app/views/projects/blob/_editor.html.haml
@@ -19,7 +19,7 @@
       = text_field_tag 'file_name', params[:file_name], placeholder: "File name",
         required: true, class: 'form-control gl-form-input new-file-name js-file-path-name-input', value: params[:file_name] || (should_suggest_gitlab_ci_yml? ? '.gitlab-ci.yml' : '')
       = render 'template_selectors'
-      - if should_suggest_gitlab_ci_yml?
+      - if true
         .js-suggest-gitlab-ci-yml{ data: { target: '#gitlab-ci-yml-selector',
           track_label: 'suggest_gitlab_ci_yml',
           merge_request_path: params[:mr_path],
diff --git a/app/views/projects/blob/new.html.haml b/app/views/projects/blob/new.html.haml
index 8722819fe4f..b3d8e5116ee 100644
--- a/app/views/projects/blob/new.html.haml
+++ b/app/views/projects/blob/new.html.haml
@@ -11,7 +11,7 @@
     = hidden_field_tag 'content', '', id: 'file-content'
     = render 'projects/commit_button', ref: @ref,
               cancel_path: project_tree_path(@project, @id)
-    - if should_suggest_gitlab_ci_yml?
+    - if true
       .js-suggest-gitlab-ci-yml-commit-changes{ data: { target: '#commit-changes',
         merge_request_path: params[:mr_path],
         track_label: 'suggest_commit_first_project_gitlab_ci_yml',
  1. Add a file to any project at /:namespace/:project/-/new/master/.
  2. In the Select a template type dropdown, select .gitlab-ci.yml.
  • The template selector should appear with the first hint shown next to it.
  1. Select any template in the template selector.
  • The first hint should disappear.
  • You should now see the second hint next to the Commit changes button.

Screenshots (strongly suggested)

Before After
before after

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Paul Gascou-Vaillancourt

Merge request reports