Commit 808240a6 authored by Paul Slaughter's avatar Paul Slaughter Committed by Enrique Alcantara

Settings search bar to remember expansion state

- This way the sections that only the sections expanded
  before we start a search, are expanded when the search
  is cleared. This helps prevent the userr from losing their
  place.

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53581#note_503655640
parent b07155c0
...@@ -3,20 +3,37 @@ import { GlSearchBoxByType } from '@gitlab/ui'; ...@@ -3,20 +3,37 @@ import { GlSearchBoxByType } from '@gitlab/ui';
import { uniq } from 'lodash'; import { uniq } from 'lodash';
import { EXCLUDED_NODES, HIDE_CLASS, HIGHLIGHT_CLASS, TYPING_DELAY } from '../constants'; import { EXCLUDED_NODES, HIDE_CLASS, HIGHLIGHT_CLASS, TYPING_DELAY } from '../constants';
const origExpansions = new Map();
const findSettingsSection = (sectionSelector, node) => { const findSettingsSection = (sectionSelector, node) => {
return node.parentElement.closest(sectionSelector); return node.parentElement.closest(sectionSelector);
}; };
const resetSections = ({ sectionSelector, expandSection, collapseSection }) => { const restoreExpansionState = ({ expandSection, collapseSection }) => {
document.querySelectorAll(sectionSelector).forEach((section, index) => { origExpansions.forEach((isExpanded, section) => {
section.classList.remove(HIDE_CLASS); if (isExpanded) {
if (index === 0) {
expandSection(section); expandSection(section);
} else { } else {
collapseSection(section); 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 = () => { const clearHighlights = () => {
...@@ -85,6 +102,12 @@ export default { ...@@ -85,6 +102,12 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
isExpandedFn: {
type: Function,
required: false,
// default to a function that returns false
default: () => () => false,
},
}, },
data() { data() {
return { return {
...@@ -97,6 +120,7 @@ export default { ...@@ -97,6 +120,7 @@ export default {
sectionSelector: this.sectionSelector, sectionSelector: this.sectionSelector,
expandSection: this.expandSection, expandSection: this.expandSection,
collapseSection: this.collapseSection, collapseSection: this.collapseSection,
isExpanded: this.isExpandedFn,
}; };
this.searchTerm = value; this.searchTerm = value;
...@@ -104,7 +128,11 @@ export default { ...@@ -104,7 +128,11 @@ export default {
clearResults(displayOptions); clearResults(displayOptions);
if (value.length) { if (value.length) {
saveExpansionState(document.querySelectorAll(this.sectionSelector), displayOptions);
displayResults(displayOptions, search(this.searchRoot, value)); displayResults(displayOptions, search(this.searchRoot, value));
} else {
restoreExpansionState(displayOptions);
} }
}, },
expandSection(section) { expandSection(section) {
......
import Vue from 'vue'; import Vue from 'vue';
import $ from 'jquery'; import { expandSection, closeSection, isExpanded } from '~/settings_panels';
import { expandSection, closeSection } from '~/settings_panels';
import SearchSettings from '~/search_settings/components/search_settings.vue'; import SearchSettings from '~/search_settings/components/search_settings.vue';
const mountSearch = ({ el }) => const mountSearch = ({ el }) =>
...@@ -12,10 +11,11 @@ const mountSearch = ({ el }) => ...@@ -12,10 +11,11 @@ const mountSearch = ({ el }) =>
props: { props: {
searchRoot: document.querySelector('#content-body'), searchRoot: document.querySelector('#content-body'),
sectionSelector: '.js-search-settings-section, section.settings', sectionSelector: '.js-search-settings-section, section.settings',
isExpandedFn: isExpanded,
}, },
on: { on: {
collapse: (section) => closeSection($(section)), collapse: closeSection,
expand: (section) => expandSection($(section)), expand: expandSection,
}, },
}), }),
}); });
......
import $ from 'jquery'; import $ from 'jquery';
import { __ } from './locale'; 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')); $section.find('.js-settings-toggle:not(.js-settings-toggle-trigger-only)').text(__('Collapse'));
// eslint-disable-next-line @gitlab/no-global-event-off // eslint-disable-next-line @gitlab/no-global-event-off
$section.find('.settings-content').off('scroll.expandSection').scrollTop(0); $section.find('.settings-content').off('scroll.expandSection').scrollTop(0);
...@@ -13,7 +28,9 @@ export function expandSection($section) { ...@@ -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('.js-settings-toggle:not(.js-settings-toggle-trigger-only)').text(__('Expand'));
$section.find('.settings-content').on('scroll.expandSection', () => expandSection($section)); $section.find('.settings-content').on('scroll.expandSection', () => expandSection($section));
$section.removeClass('expanded'); $section.removeClass('expanded');
...@@ -26,7 +43,7 @@ export function closeSection($section) { ...@@ -26,7 +43,7 @@ export function closeSection($section) {
export function toggleSection($section) { export function toggleSection($section) {
$section.removeClass('no-animate'); $section.removeClass('no-animate');
if ($section.hasClass('expanded')) { if (isExpanded($section)) {
closeSection($section); closeSection($section);
} else { } else {
expandSection($section); expandSection($section);
...@@ -38,7 +55,7 @@ export default function initSettingsPanels() { ...@@ -38,7 +55,7 @@ export default function initSettingsPanels() {
const $section = $(elm); const $section = $(elm);
$section.on('click.toggleSection', '.js-settings-toggle', () => toggleSection($section)); $section.on('click.toggleSection', '.js-settings-toggle', () => toggleSection($section));
if (!$section.hasClass('expanded')) { if (!isExpanded($section)) {
$section.find('.settings-content').on('scroll.expandSection', () => { $section.find('.settings-content').on('scroll.expandSection', () => {
$section.removeClass('no-animate'); $section.removeClass('no-animate');
expandSection($section); expandSection($section);
......
...@@ -2,6 +2,7 @@ import { GlSearchBoxByType } from '@gitlab/ui'; ...@@ -2,6 +2,7 @@ import { GlSearchBoxByType } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import SearchSettings from '~/search_settings/components/search_settings.vue'; import SearchSettings from '~/search_settings/components/search_settings.vue';
import { HIGHLIGHT_CLASS, HIDE_CLASS } from '~/search_settings/constants'; import { HIGHLIGHT_CLASS, HIDE_CLASS } from '~/search_settings/constants';
import { isExpanded, expandSection, closeSection } from '~/settings_panels';
describe('search_settings/components/search_settings.vue', () => { describe('search_settings/components/search_settings.vue', () => {
const ROOT_ID = 'content-body'; const ROOT_ID = 'content-body';
...@@ -9,6 +10,8 @@ describe('search_settings/components/search_settings.vue', () => { ...@@ -9,6 +10,8 @@ describe('search_settings/components/search_settings.vue', () => {
const SEARCH_TERM = 'Delete project'; const SEARCH_TERM = 'Delete project';
const GENERAL_SETTINGS_ID = 'js-general-settings'; const GENERAL_SETTINGS_ID = 'js-general-settings';
const ADVANCED_SETTINGS_ID = 'js-advanced-settings'; const ADVANCED_SETTINGS_ID = 'js-advanced-settings';
const EXTRA_SETTINGS_ID = 'js-extra-settings';
let wrapper; let wrapper;
const buildWrapper = () => { const buildWrapper = () => {
...@@ -16,10 +19,15 @@ describe('search_settings/components/search_settings.vue', () => { ...@@ -16,10 +19,15 @@ describe('search_settings/components/search_settings.vue', () => {
propsData: { propsData: {
searchRoot: document.querySelector(`#${ROOT_ID}`), searchRoot: document.querySelector(`#${ROOT_ID}`),
sectionSelector: SECTION_SELECTOR, 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 sections = () => Array.from(document.querySelectorAll(SECTION_SELECTOR));
const sectionsCount = () => sections().length; const sectionsCount = () => sections().length;
const visibleSectionsCount = () => const visibleSectionsCount = () =>
...@@ -39,7 +47,10 @@ describe('search_settings/components/search_settings.vue', () => { ...@@ -39,7 +47,10 @@ describe('search_settings/components/search_settings.vue', () => {
<section id="${GENERAL_SETTINGS_ID}" class="settings"> <section id="${GENERAL_SETTINGS_ID}" class="settings">
<span>General</span> <span>General</span>
</section> </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> <span>${SEARCH_TERM}</span>
</section> </section>
</div> </div>
...@@ -52,17 +63,6 @@ describe('search_settings/components/search_settings.vue', () => { ...@@ -52,17 +63,6 @@ describe('search_settings/components/search_settings.vue', () => {
wrapper.destroy(); 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', () => { it('hides sections that do not match the search term', () => {
const hiddenSection = document.querySelector(`#${GENERAL_SETTINGS_ID}`); const hiddenSection = document.querySelector(`#${GENERAL_SETTINGS_ID}`);
search(SEARCH_TERM); search(SEARCH_TERM);
...@@ -72,12 +72,11 @@ describe('search_settings/components/search_settings.vue', () => { ...@@ -72,12 +72,11 @@ describe('search_settings/components/search_settings.vue', () => {
}); });
it('expands section that matches the search term', () => { 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); search(SEARCH_TERM);
// Last called because expand is always called once to reset the page state expect(wrapper.emitted('expand')).toEqual([[section]]);
expect(wrapper.emitted().expand[1][0]).toBe(section);
}); });
it('highlight elements that match the search term', () => { it('highlight elements that match the search term', () => {
...@@ -86,21 +85,64 @@ describe('search_settings/components/search_settings.vue', () => { ...@@ -86,21 +85,64 @@ describe('search_settings/components/search_settings.vue', () => {
expect(highlightedElementsCount()).toBe(1); expect(highlightedElementsCount()).toBe(1);
}); });
describe('when search term is cleared', () => { describe('default', () => {
beforeEach(() => { it('test setup starts with expansion state', () => {
search(SEARCH_TERM); expect(sections().map(isExpanded)).toEqual([false, true, false]);
});
it('displays all sections', () => {
expect(visibleSectionsCount()).toBe(1);
clearSearch();
expect(visibleSectionsCount()).toBe(sectionsCount());
}); });
it('removes the highlight from all elements', () => { describe('when searched and cleared', () => {
expect(highlightedElementsCount()).toBe(1); beforeEach(() => {
clearSearch(); search('Test');
expect(highlightedElementsCount()).toBe(0); 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]);
});
});
}); });
}); });
}); });
import $ from 'jquery';
import { setHTMLFixture } from 'helpers/fixtures'; import { setHTMLFixture } from 'helpers/fixtures';
import mount from '~/search_settings/mount'; import mount from '~/search_settings/mount';
import { expandSection, closeSection } from '~/settings_panels'; import { expandSection, closeSection } from '~/settings_panels';
...@@ -24,13 +23,13 @@ describe('search_settings/mount', () => { ...@@ -24,13 +23,13 @@ describe('search_settings/mount', () => {
const section = { name: 'section' }; const section = { name: 'section' };
app.$refs.searchSettings.$emit('expand', 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', () => { it('calls settings_panel.closeSection when collapse event is emitted', () => {
const section = { name: 'section' }; const section = { name: 'section' };
app.$refs.searchSettings.$emit('collapse', section); app.$refs.searchSettings.$emit('collapse', section);
expect(closeSection).toHaveBeenCalledWith($(section)); expect(closeSection).toHaveBeenCalledWith(section);
}); });
}); });
import $ from 'jquery'; import $ from 'jquery';
import initSettingsPanels from '~/settings_panels'; import initSettingsPanels, { isExpanded } from '~/settings_panels';
describe('Settings Panels', () => { describe('Settings Panels', () => {
preloadFixtures('groups/edit.html'); preloadFixtures('groups/edit.html');
...@@ -20,11 +20,11 @@ describe('Settings Panels', () => { ...@@ -20,11 +20,11 @@ describe('Settings Panels', () => {
// Our test environment automatically expands everything so we need to clear that out first // Our test environment automatically expands everything so we need to clear that out first
panel.classList.remove('expanded'); panel.classList.remove('expanded');
expect(panel.classList.contains('expanded')).toBe(false); expect(isExpanded(panel)).toBe(false);
initSettingsPanels(); initSettingsPanels();
expect(panel.classList.contains('expanded')).toBe(true); expect(isExpanded(panel)).toBe(true);
}); });
}); });
...@@ -35,11 +35,11 @@ describe('Settings Panels', () => { ...@@ -35,11 +35,11 @@ describe('Settings Panels', () => {
initSettingsPanels(); initSettingsPanels();
expect(panel.classList.contains('expanded')).toBe(true); expect(isExpanded(panel)).toBe(true);
$(trigger).click(); $(trigger).click();
expect(panel.classList.contains('expanded')).toBe(false); expect(isExpanded(panel)).toBe(false);
expect(trigger.textContent).toEqual(originalText); expect(trigger.textContent).toEqual(originalText);
}); });
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment