Merge branch 'ps-settings-search-remember-expansion' into 'master'

Settings search bar to remember expansion state

See merge request gitlab-org/gitlab!53659

app/assets/javascripts/search_settings/components/search_settings.vue

@@ -3,20 +3,37 @@ import { GlSearchBoxByType } from '@gitlab/ui';
 import { uniq } from 'lodash';
 import { EXCLUDED_NODES, HIDE_CLASS, HIGHLIGHT_CLASS, TYPING_DELAY } from '../constants';
 
+const origExpansions = new Map();
+
 const findSettingsSection = (sectionSelector, node) => {
   return node.parentElement.closest(sectionSelector);
 };
 
-const resetSections = ({ sectionSelector, expandSection, collapseSection }) => {
-  document.querySelectorAll(sectionSelector).forEach((section, index) => {
-    section.classList.remove(HIDE_CLASS);
-
-    if (index === 0) {
+const restoreExpansionState = ({ expandSection, collapseSection }) => {
+  origExpansions.forEach((isExpanded, section) => {
+    if (isExpanded) {
       expandSection(section);
     } else {
       collapseSection(section);
     }
   });
+
+  origExpansions.clear();
+};
+
+const saveExpansionState = (sections, { isExpanded }) => {
+  // If we've saved expansions before, don't override it.
+  if (origExpansions.size > 0) {
+    return;
+  }
+
+  sections.forEach((section) => origExpansions.set(section, isExpanded(section)));
+};
+
+const resetSections = ({ sectionSelector }) => {
+  document.querySelectorAll(sectionSelector).forEach((section) => {
+    section.classList.remove(HIDE_CLASS);
+  });
 };
 
 const clearHighlights = () => {
@@ -85,6 +102,12 @@ export default {
       type: String,
       required: true,
     },
+    isExpandedFn: {
+      type: Function,
+      required: false,
+      // default to a function that returns false
+      default: () => () => false,
+    },
   },
   data() {
     return {
@@ -97,6 +120,7 @@ export default {
         sectionSelector: this.sectionSelector,
         expandSection: this.expandSection,
         collapseSection: this.collapseSection,
+        isExpanded: this.isExpandedFn,
       };
 
       this.searchTerm = value;
@@ -104,7 +128,11 @@ export default {
       clearResults(displayOptions);
 
       if (value.length) {
+        saveExpansionState(document.querySelectorAll(this.sectionSelector), displayOptions);
+
         displayResults(displayOptions, search(this.searchRoot, value));
+      } else {
+        restoreExpansionState(displayOptions);
       }
     },
     expandSection(section) {

app/assets/javascripts/search_settings/mount.js

 import Vue from 'vue';
-import $ from 'jquery';
-import { expandSection, closeSection } from '~/settings_panels';
+import { expandSection, closeSection, isExpanded } from '~/settings_panels';
 import SearchSettings from '~/search_settings/components/search_settings.vue';
 
 const mountSearch = ({ el }) =>
@@ -12,10 +11,11 @@ const mountSearch = ({ el }) =>
         props: {
           searchRoot: document.querySelector('#content-body'),
           sectionSelector: '.js-search-settings-section, section.settings',
+          isExpandedFn: isExpanded,
         },
         on: {
-          collapse: (section) => closeSection($(section)),
-          expand: (section) => expandSection($(section)),
+          collapse: closeSection,
+          expand: expandSection,
         },
       }),
   });

app/assets/javascripts/settings_panels.js

 import $ from 'jquery';
 import { __ } from './locale';
 
-export function expandSection($section) {
+/**
+ * Returns true if the given section is expanded or not
+ *
+ * For legacy consistency, it supports both jQuery and DOM elements
+ *
+ * @param {jQuery | Element} section
+ */
+export function isExpanded(sectionArg) {
+  const section = sectionArg instanceof $ ? sectionArg[0] : sectionArg;
+
+  return section.classList.contains('expanded');
+}
+
+export function expandSection(sectionArg) {
+  const $section = $(sectionArg);
+
   $section.find('.js-settings-toggle:not(.js-settings-toggle-trigger-only)').text(__('Collapse'));
   // eslint-disable-next-line @gitlab/no-global-event-off
   $section.find('.settings-content').off('scroll.expandSection').scrollTop(0);
@@ -13,7 +28,9 @@ export function expandSection($section) {
   }
 }
 
-export function closeSection($section) {
+export function closeSection(sectionArg) {
+  const $section = $(sectionArg);
+
   $section.find('.js-settings-toggle:not(.js-settings-toggle-trigger-only)').text(__('Expand'));
   $section.find('.settings-content').on('scroll.expandSection', () => expandSection($section));
   $section.removeClass('expanded');
@@ -26,7 +43,7 @@ export function closeSection($section) {
 
 export function toggleSection($section) {
   $section.removeClass('no-animate');
-  if ($section.hasClass('expanded')) {
+  if (isExpanded($section)) {
     closeSection($section);
   } else {
     expandSection($section);
@@ -38,7 +55,7 @@ export default function initSettingsPanels() {
     const $section = $(elm);
     $section.on('click.toggleSection', '.js-settings-toggle', () => toggleSection($section));
 
-    if (!$section.hasClass('expanded')) {
+    if (!isExpanded($section)) {
       $section.find('.settings-content').on('scroll.expandSection', () => {
         $section.removeClass('no-animate');
         expandSection($section);

spec/frontend/search_settings/components/search_settings_spec.js

@@ -2,6 +2,7 @@ import { GlSearchBoxByType } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import SearchSettings from '~/search_settings/components/search_settings.vue';
 import { HIGHLIGHT_CLASS, HIDE_CLASS } from '~/search_settings/constants';
+import { isExpanded, expandSection, closeSection } from '~/settings_panels';
 
 describe('search_settings/components/search_settings.vue', () => {
   const ROOT_ID = 'content-body';
@@ -9,6 +10,8 @@ describe('search_settings/components/search_settings.vue', () => {
   const SEARCH_TERM = 'Delete project';
   const GENERAL_SETTINGS_ID = 'js-general-settings';
   const ADVANCED_SETTINGS_ID = 'js-advanced-settings';
+  const EXTRA_SETTINGS_ID = 'js-extra-settings';
+
   let wrapper;
 
   const buildWrapper = () => {
@@ -16,10 +19,15 @@ describe('search_settings/components/search_settings.vue', () => {
       propsData: {
         searchRoot: document.querySelector(`#${ROOT_ID}`),
         sectionSelector: SECTION_SELECTOR,
+        isExpandedFn: isExpanded,
+      },
+      // Add real listeners so we can simplify and strengthen some tests.
+      listeners: {
+        expand: expandSection,
+        collapse: closeSection,
       },
     });
   };
-
   const sections = () => Array.from(document.querySelectorAll(SECTION_SELECTOR));
   const sectionsCount = () => sections().length;
   const visibleSectionsCount = () =>
@@ -39,7 +47,10 @@ describe('search_settings/components/search_settings.vue', () => {
         <section id="${GENERAL_SETTINGS_ID}" class="settings">
           <span>General</span>
         </section>
-        <section id="${ADVANCED_SETTINGS_ID}" class="settings">
+        <section id="${ADVANCED_SETTINGS_ID}" class="settings expanded">
+          <span>Advanced</span>
+        </section>
+        <section id="${EXTRA_SETTINGS_ID}" class="settings">
           <span>${SEARCH_TERM}</span>
         </section>
       </div>
@@ -52,17 +63,6 @@ describe('search_settings/components/search_settings.vue', () => {
     wrapper.destroy();
   });
 
-  it('expands first section and collapses the rest', () => {
-    clearSearch();
-
-    const [firstSection, ...otherSections] = sections();
-
-    expect(wrapper.emitted()).toEqual({
-      expand: [[firstSection]],
-      collapse: otherSections.map((x) => [x]),
-    });
-  });
-
   it('hides sections that do not match the search term', () => {
     const hiddenSection = document.querySelector(`#${GENERAL_SETTINGS_ID}`);
     search(SEARCH_TERM);
@@ -72,12 +72,11 @@ describe('search_settings/components/search_settings.vue', () => {
   });
 
   it('expands section that matches the search term', () => {
-    const section = document.querySelector(`#${ADVANCED_SETTINGS_ID}`);
+    const section = document.querySelector(`#${EXTRA_SETTINGS_ID}`);
 
     search(SEARCH_TERM);
 
-    // Last called because expand is always called once to reset the page state
-    expect(wrapper.emitted().expand[1][0]).toBe(section);
+    expect(wrapper.emitted('expand')).toEqual([[section]]);
   });
 
   it('highlight elements that match the search term', () => {
@@ -86,21 +85,64 @@ describe('search_settings/components/search_settings.vue', () => {
     expect(highlightedElementsCount()).toBe(1);
   });
 
-  describe('when search term is cleared', () => {
-    beforeEach(() => {
-      search(SEARCH_TERM);
-    });
-
-    it('displays all sections', () => {
-      expect(visibleSectionsCount()).toBe(1);
-      clearSearch();
-      expect(visibleSectionsCount()).toBe(sectionsCount());
+  describe('default', () => {
+    it('test setup starts with expansion state', () => {
+      expect(sections().map(isExpanded)).toEqual([false, true, false]);
     });
 
-    it('removes the highlight from all elements', () => {
-      expect(highlightedElementsCount()).toBe(1);
-      clearSearch();
-      expect(highlightedElementsCount()).toBe(0);
+    describe('when searched and cleared', () => {
+      beforeEach(() => {
+        search('Test');
+        clearSearch();
+      });
+
+      it('displays all sections', () => {
+        expect(visibleSectionsCount()).toBe(sectionsCount());
+      });
+
+      it('removes the highlight from all elements', () => {
+        expect(highlightedElementsCount()).toBe(0);
+      });
+
+      it('should preserve original expansion state', () => {
+        expect(sections().map(isExpanded)).toEqual([false, true, false]);
+      });
+
+      it('should preserve state by emitting events', () => {
+        const [first, mid, last] = sections();
+
+        expect(wrapper.emitted()).toEqual({
+          expand: [[mid]],
+          collapse: [[first], [last]],
+        });
+      });
+
+      describe('after multiple searches and clear', () => {
+        beforeEach(() => {
+          search('Test');
+          search(SEARCH_TERM);
+          clearSearch();
+        });
+
+        it('should preserve last expansion state', () => {
+          expect(sections().map(isExpanded)).toEqual([false, true, false]);
+        });
+      });
+
+      describe('after user expands and collapses, search, and clear', () => {
+        beforeEach(() => {
+          const [first, mid] = sections();
+          closeSection(mid);
+          expandSection(first);
+
+          search(SEARCH_TERM);
+          clearSearch();
+        });
+
+        it('should preserve last expansion state', () => {
+          expect(sections().map(isExpanded)).toEqual([true, false, false]);
+        });
+      });
     });
   });
 });

spec/frontend/search_settings/mount_spec.js

-import $ from 'jquery';
 import { setHTMLFixture } from 'helpers/fixtures';
 import mount from '~/search_settings/mount';
 import { expandSection, closeSection } from '~/settings_panels';
@@ -24,13 +23,13 @@ describe('search_settings/mount', () => {
     const section = { name: 'section' };
     app.$refs.searchSettings.$emit('expand', section);
 
-    expect(expandSection).toHaveBeenCalledWith($(section));
+    expect(expandSection).toHaveBeenCalledWith(section);
   });
 
   it('calls settings_panel.closeSection when collapse event is emitted', () => {
     const section = { name: 'section' };
     app.$refs.searchSettings.$emit('collapse', section);
 
-    expect(closeSection).toHaveBeenCalledWith($(section));
+    expect(closeSection).toHaveBeenCalledWith(section);
   });
 });

spec/frontend/settings_panels_spec.js

 import $ from 'jquery';
-import initSettingsPanels from '~/settings_panels';
+import initSettingsPanels, { isExpanded } from '~/settings_panels';
 
 describe('Settings Panels', () => {
   preloadFixtures('groups/edit.html');
@@ -20,11 +20,11 @@ describe('Settings Panels', () => {
       // Our test environment automatically expands everything so we need to clear that out first
       panel.classList.remove('expanded');
 
-      expect(panel.classList.contains('expanded')).toBe(false);
+      expect(isExpanded(panel)).toBe(false);
 
       initSettingsPanels();
 
-      expect(panel.classList.contains('expanded')).toBe(true);
+      expect(isExpanded(panel)).toBe(true);
     });
   });
 
@@ -35,11 +35,11 @@ describe('Settings Panels', () => {
 
     initSettingsPanels();
 
-    expect(panel.classList.contains('expanded')).toBe(true);
+    expect(isExpanded(panel)).toBe(true);
 
     $(trigger).click();
 
-    expect(panel.classList.contains('expanded')).toBe(false);
+    expect(isExpanded(panel)).toBe(false);
     expect(trigger.textContent).toEqual(originalText);
   });
 });