Commit 8fe0878d authored by Andrew Fontaine's avatar Andrew Fontaine

Merge branch...

Merge branch '331774-fix-new-package-settings-page-to-not-animate-to-close-on-page-load' into 'master'

Fix settings_block animations glitches

See merge request gitlab-org/gitlab!74411
parents f62ca4eb 88aff5d7
...@@ -24,10 +24,13 @@ export default { ...@@ -24,10 +24,13 @@ export default {
}, },
}, },
data() { data() {
const forceOpen = !this.collapsible || this.defaultExpanded;
return { return {
// Non-collapsible sections should always be expanded. // Non-collapsible sections should always be expanded.
// For collapsible sections, fall back to defaultExpanded. // For collapsible sections, fall back to defaultExpanded.
sectionExpanded: !this.collapsible || this.defaultExpanded, sectionExpanded: forceOpen,
initialised: forceOpen,
animating: false,
}; };
}, },
computed: { computed: {
...@@ -53,7 +56,12 @@ export default { ...@@ -53,7 +56,12 @@ export default {
toggleSectionExpanded() { toggleSectionExpanded() {
this.sectionExpanded = !this.sectionExpanded; this.sectionExpanded = !this.sectionExpanded;
if (!this.initialised) {
this.initialised = true;
}
if (this.sectionExpanded) { if (this.sectionExpanded) {
this.animating = true;
this.$refs.settingsContent.focus(); this.$refs.settingsContent.focus();
} }
}, },
...@@ -68,7 +76,10 @@ export default { ...@@ -68,7 +76,10 @@ export default {
</script> </script>
<template> <template>
<section class="settings" :class="{ 'no-animate': !slideAnimated, expanded: sectionExpanded }"> <section
class="settings"
:class="{ 'no-animate': !slideAnimated, expanded: sectionExpanded, animating }"
>
<div class="settings-header"> <div class="settings-header">
<h4> <h4>
<span <span
...@@ -103,12 +114,14 @@ export default { ...@@ -103,12 +114,14 @@ export default {
</p> </p>
</div> </div>
<div <div
v-show="initialised"
:id="settingsContentId" :id="settingsContentId"
ref="settingsContent" ref="settingsContent"
:aria-labelledby="settingsLabelId" :aria-labelledby="settingsLabelId"
tabindex="-1" tabindex="-1"
role="region" role="region"
class="settings-content" class="settings-content"
@animationend="animating = false"
> >
<slot></slot> <slot></slot>
</div> </div>
......
...@@ -50,6 +50,7 @@ exports[`Settings Block renders the correct markup 1`] = ` ...@@ -50,6 +50,7 @@ exports[`Settings Block renders the correct markup 1`] = `
class="settings-content" class="settings-content"
id="settings_content_3" id="settings_content_3"
role="region" role="region"
style="display: none;"
tabindex="-1" tabindex="-1"
> >
<div <div
......
import { GlButton } from '@gitlab/ui'; import { GlButton } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import SettingsBlock from '~/vue_shared/components/settings/settings_block.vue'; import SettingsBlock from '~/vue_shared/components/settings/settings_block.vue';
describe('Settings Block', () => { describe('Settings Block', () => {
let wrapper; let wrapper;
const mountComponent = (propsData) => { const mountComponent = (propsData) => {
wrapper = shallowMount(SettingsBlock, { wrapper = shallowMountExtended(SettingsBlock, {
propsData, propsData,
slots: { slots: {
title: '<div data-testid="title-slot"></div>', title: '<div data-testid="title-slot"></div>',
...@@ -20,11 +20,13 @@ describe('Settings Block', () => { ...@@ -20,11 +20,13 @@ describe('Settings Block', () => {
wrapper.destroy(); wrapper.destroy();
}); });
const findDefaultSlot = () => wrapper.find('[data-testid="default-slot"]'); const findDefaultSlot = () => wrapper.findByTestId('default-slot');
const findTitleSlot = () => wrapper.find('[data-testid="title-slot"]'); const findTitleSlot = () => wrapper.findByTestId('title-slot');
const findDescriptionSlot = () => wrapper.find('[data-testid="description-slot"]'); const findDescriptionSlot = () => wrapper.findByTestId('description-slot');
const findExpandButton = () => wrapper.findComponent(GlButton); const findExpandButton = () => wrapper.findComponent(GlButton);
const findSectionTitleButton = () => wrapper.find('[data-testid="section-title-button"]'); const findSectionTitleButton = () => wrapper.findByTestId('section-title-button');
// we are using a non js class for this finder because this class determine the component structure
const findSettingsContent = () => wrapper.find('.settings-content');
const expectExpandedState = ({ expanded = true } = {}) => { const expectExpandedState = ({ expanded = true } = {}) => {
const settingsExpandButton = findExpandButton(); const settingsExpandButton = findExpandButton();
...@@ -62,6 +64,26 @@ describe('Settings Block', () => { ...@@ -62,6 +64,26 @@ describe('Settings Block', () => {
expect(findDescriptionSlot().exists()).toBe(true); expect(findDescriptionSlot().exists()).toBe(true);
}); });
it('content is hidden before first expansion', async () => {
// this is a regression test for the bug described here: https://gitlab.com/gitlab-org/gitlab/-/issues/331774
mountComponent();
// content is hidden
expect(findDefaultSlot().isVisible()).toBe(false);
// expand
await findSectionTitleButton().trigger('click');
// content is visible
expect(findDefaultSlot().isVisible()).toBe(true);
// collapse
await findSectionTitleButton().trigger('click');
// content is still visible (and we have a closing animation)
expect(findDefaultSlot().isVisible()).toBe(true);
});
describe('slide animation behaviour', () => { describe('slide animation behaviour', () => {
it('is animated by default', () => { it('is animated by default', () => {
mountComponent(); mountComponent();
...@@ -81,6 +103,20 @@ describe('Settings Block', () => { ...@@ -81,6 +103,20 @@ describe('Settings Block', () => {
expect(wrapper.classes('no-animate')).toBe(noAnimatedClass); expect(wrapper.classes('no-animate')).toBe(noAnimatedClass);
}, },
); );
it('sets the animating class only during the animation', async () => {
mountComponent();
expect(wrapper.classes('animating')).toBe(false);
await findSectionTitleButton().trigger('click');
expect(wrapper.classes('animating')).toBe(true);
await findSettingsContent().trigger('animationend');
expect(wrapper.classes('animating')).toBe(false);
});
}); });
describe('expanded behaviour', () => { describe('expanded behaviour', () => {
......
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