Merge branch '35955-searchable-settings-puts-expand-collapse-toggle-in-an-invalid-state' into 'master'

Resolve "Searchable settings puts expand/collapse toggle in an invalid state."

Closes #35955, #37520, and #39207

See merge request gitlab-org/gitlab-ce!13363

8 files changed, 55 insertions, 51 deletions

diff --git a/app/assets/javascripts/settings_panels.js b/app/assets/javascripts/settings_panels.js
index 8635ccece6e..d34a21b37e1 100644
--- a/app/assets/javascripts/settings_panels.js
+++ b/app/assets/javascripts/settings_panels.js
@@ -1,34 +1,26 @@
-function expandSectionParent($section, $content) {
-  $section.addClass('expanded');
-  $content.off('animationend.expandSectionParent');
-}
-
 function expandSection($section) {
   $section.find('.js-settings-toggle').text('Collapse');
-
-  const $content = $section.find('.settings-content');
-  $content.addClass('expanded').off('scroll.expandSection').scrollTop(0);
-
-  if ($content.hasClass('no-animate')) {
-    expandSectionParent($section, $content);
-  } else {
-    $content.on('animationend.expandSectionParent', () => expandSectionParent($section, $content));
+  $section.find('.settings-content').off('scroll.expandSection').scrollTop(0);
+  $section.addClass('expanded');
+  if (!$section.hasClass('no-animate')) {
+    $section.addClass('animating')
+      .one('animationend.animateSection', () => $section.removeClass('animating'));
   }
 }
 
 function closeSection($section) {
   $section.find('.js-settings-toggle').text('Expand');
-
-  const $content = $section.find('.settings-content');
-  $content.removeClass('expanded').on('scroll.expandSection', () => expandSection($section));
-
+  $section.find('.settings-content').on('scroll.expandSection', () => expandSection($section));
   $section.removeClass('expanded');
+  if (!$section.hasClass('no-animate')) {
+    $section.addClass('animating')
+      .one('animationend.animateSection', () => $section.removeClass('animating'));
+  }
 }
 
 function toggleSection($section) {
-  const $content = $section.find('.settings-content');
-  $content.removeClass('no-animate');
-  if ($content.hasClass('expanded')) {
+  $section.removeClass('no-animate');
+  if ($section.hasClass('expanded')) {
     closeSection($section);
   } else {
     expandSection($section);
@@ -39,10 +31,19 @@ export default function initSettingsPanels() {
   $('.settings').each((i, elm) => {
     const $section = $(elm);
     $section.on('click.toggleSection', '.js-settings-toggle', () => toggleSection($section));
-    $section.find('.settings-content:not(.expanded)').on('scroll.expandSection', () => expandSection($section));
+
+    if (!$section.hasClass('expanded')) {
+      $section.find('.settings-content').on('scroll.expandSection', () => {
+        $section.removeClass('no-animate');
+        expandSection($section);
+      });
+    }
   });
 
   if (location.hash) {
-    expandSection($(location.hash));
+    const $target = $(location.hash);
+    if ($target.length && $target.hasClass('.settings')) {
+      expandSection($target);
+    }
   }
 }
diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index 41a6ba2023a..968a94c68cf 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -23,15 +23,14 @@
 }
 
 .settings {
-  overflow: hidden;
   border-bottom: 1px solid $gray-darker;
 
   &:first-of-type {
     margin-top: 10px;
   }
 
-  &.expanded {
-    overflow: visible;
+  &.animating {
+    overflow: hidden;
   }
 }
 
@@ -56,14 +55,18 @@
   overflow-y: scroll;
   padding-right: 110px;
   animation: collapseMaxHeight 300ms ease-out;
+  // Keep the section from expanding when we scroll over it
+  pointer-events: none;
 
-  &.expanded {
+  .settings.expanded & {
     max-height: none;
     overflow-y: visible;
     animation: expandMaxHeight 300ms ease-in;
+    // Reset and allow clicks again when expanded
+    pointer-events: auto;
   }
 
-  &.no-animate {
+  .settings.no-animate & {
     animation: none;
   }
 
diff --git a/app/views/projects/_export.html.haml b/app/views/projects/_export.html.haml
index 623d3bc91c6..c5b1897c492 100644
--- a/app/views/projects/_export.html.haml
+++ b/app/views/projects/_export.html.haml
@@ -3,7 +3,7 @@
 - project = local_assigns.fetch(:project)
 - expanded = Rails.env.test?
 
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Export project
@@ -11,7 +11,7 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       Export this project with all its related data in order to move your project to a new GitLab instance. Once the export is finished, you can import the file from the "New Project" page.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     .bs-callout.bs-callout-info
       %p.append-bottom-0
         %p
diff --git a/app/views/projects/deploy_keys/_index.html.haml b/app/views/projects/deploy_keys/_index.html.haml
index 45985a5ecef..e75ae87e771 100644
--- a/app/views/projects/deploy_keys/_index.html.haml
+++ b/app/views/projects/deploy_keys/_index.html.haml
@@ -1,5 +1,5 @@
 - expanded = Rails.env.test?
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Deploy Keys
@@ -7,7 +7,7 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       Deploy keys allow read-only or read-write (if enabled) access to your repository. Deploy keys can be used for CI, staging or production servers. You can create a deploy key or add an existing one.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     %h5.prepend-top-0
       Create a new deploy key for this project
     = render @deploy_keys.form_partial_path
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index 893e536e289..5703ef1d4bb 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -4,7 +4,7 @@
 - expanded = Rails.env.test?
 
 .project-edit-container
-  %section.settings.general-settings
+  %section.settings.general-settings.no-animate{ class: ('expanded' if expanded) }
     .settings-header
       %h4
         General project settings
@@ -12,7 +12,7 @@
         = expanded ? 'Collapse' : 'Expand'
       %p
         Update your project name, description, avatar, and other general settings.
-    .settings-content.no-animate{ class: ('expanded' if expanded) }
+    .settings-content
       .project-edit-errors
       = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "edit-project" }, authenticity_token: true do |f|
         %fieldset
@@ -61,7 +61,7 @@
               = link_to 'Remove avatar', project_avatar_path(@project), data: { confirm: "Project avatar will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-avatar"
         = f.submit 'Save changes', class: "btn btn-save"
 
-  %section.settings.sharing-permissions
+  %section.settings.sharing-permissions.no-animate{ class: ('expanded' if expanded) }
     .settings-header
       %h4
         Permissions
@@ -69,13 +69,13 @@
         = expanded ? 'Collapse' : 'Expand'
       %p
         Enable or disable certain project features and choose access levels.
-    .settings-content.no-animate{ class: ('expanded' if expanded) }
+    .settings-content
       = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "sharing-permissions-form" }, authenticity_token: true do |f|
         %script.js-project-permissions-form-data{ type: "application/json" }= project_permissions_panel_data(@project)
         .js-project-permissions-form
         = f.submit 'Save changes', class: "btn btn-save"
 
-  %section.settings.merge-requests-feature{ class: ("hidden" if @project.project_feature.send(:merge_requests_access_level) == 0) }
+  %section.settings.merge-requests-feature.no-animate{ class: [('expanded' if expanded), ('hidden' if @project.project_feature.send(:merge_requests_access_level) == 0)] }
     .settings-header
       %h4
         Merge request settings
@@ -83,14 +83,14 @@
         = expanded ? 'Collapse' : 'Expand'
       %p
         Customize your merge request restrictions.
-    .settings-content.no-animate{ class: ('expanded' if expanded) }
+    .settings-content
       = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "merge-request-settings-form" }, authenticity_token: true do |f|
         = render 'merge_request_settings', form: f
         = f.submit 'Save changes', class: "btn btn-save"
 
   = render 'export', project: @project
 
-  %section.settings.advanced-settings
+  %section.settings.advanced-settings.no-animate{ class: ('expanded' if expanded) }
     .settings-header
       %h4
         Advanced settings
@@ -98,7 +98,7 @@
         = expanded ? 'Collapse' : 'Expand'
       %p
         Perform advanced options such as housekeeping, archiving, renaming, transferring, or removing your project.
-    .settings-content.no-animate{ class: ('expanded' if expanded) }
+    .settings-content
       .sub-section
         %h4 Housekeeping
         %p
diff --git a/app/views/projects/protected_branches/shared/_index.html.haml b/app/views/projects/protected_branches/shared/_index.html.haml
index 6a47cbdf724..ba7d98228c3 100644
--- a/app/views/projects/protected_branches/shared/_index.html.haml
+++ b/app/views/projects/protected_branches/shared/_index.html.haml
@@ -1,6 +1,6 @@
 - expanded = Rails.env.test?
 
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Protected Branches
@@ -8,7 +8,7 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       Keep stable branches secure and force developers to use merge requests.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     %p
       By default, protected branches are designed to:
       %ul
diff --git a/app/views/projects/protected_tags/shared/_index.html.haml b/app/views/projects/protected_tags/shared/_index.html.haml
index c07bd454ff6..e764a37bbd7 100644
--- a/app/views/projects/protected_tags/shared/_index.html.haml
+++ b/app/views/projects/protected_tags/shared/_index.html.haml
@@ -1,6 +1,6 @@
 - expanded = Rails.env.test?
 
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Protected Tags
@@ -8,7 +8,7 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       Limit access to creating and updating tags.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     %p
       By default, protected tags are designed to:
       %ul
diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml
index 62455d0d40d..664a4554692 100644
--- a/app/views/projects/settings/ci_cd/show.html.haml
+++ b/app/views/projects/settings/ci_cd/show.html.haml
@@ -4,7 +4,7 @@
 
 - expanded = Rails.env.test?
 
-%section.settings#js-general-pipeline-settings
+%section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       General pipelines settings
@@ -12,10 +12,10 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       Update your CI/CD configuration, like job timeout or Auto DevOps.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     = render 'projects/pipelines_settings/show'
 
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Runners settings
@@ -23,10 +23,10 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       Register and see your runners for this project.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     = render 'projects/runners/index'
 
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Secret variables
@@ -35,10 +35,10 @@
       = expanded ? 'Collapse' : 'Expand'
     %p
       = render "ci/variables/content"
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     = render 'ci/variables/index'
 
-%section.settings
+%section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       Pipeline triggers
@@ -48,5 +48,5 @@
       Triggers can force a specific branch or tag to get rebuilt with an API call.  These tokens will
       impersonate their associated user including their access to projects and their project
       permissions.
-  .settings-content.no-animate{ class: ('expanded' if expanded) }
+  .settings-content
     = render 'projects/triggers/index'