Merge branch...

Merge branch '339292-improve-accessibility-of-app-assets-javascripts-vue_shared-components-settings' into 'master'

Clean up `settings_block.vue`

See merge request gitlab-org/gitlab!68921

app/assets/javascripts/packages_and_registries/settings/project/components/registry_settings_app.vue

@@ -88,7 +88,7 @@ export default {
 <template>
   <section data-testid="registry-settings-app">
     <cleanup-policy-enabled-alert v-if="showCleanupPolicyOnAlert" :project-path="projectPath" />
-    <settings-block default-expanded>
+    <settings-block :collapsible="false">
       <template #title> {{ __('Clean up image tags') }}</template>
       <template #description>
         <span data-testid="description">

app/assets/javascripts/vue_shared/components/settings/settings_block.stories.js

+import SettingsBlock from './settings_block.vue';
+
+export default {
+  component: SettingsBlock,
+  title: 'vue_shared/components/settings/settings_block',
+};
+
+const Template = (args, { argTypes }) => ({
+  components: { SettingsBlock },
+  props: Object.keys(argTypes),
+  template: `
+  <settings-block v-bind="$props">
+    <template #title>Settings section title</template>
+    <template #description>Settings section description</template>
+    <template #default>
+      <p>Content</p>
+      <p>More content</p>
+      <p>Content</p>
+      <p>More content...</p>
+      <p>Content</p>
+    </template>
+  </settings-block>
+  `,
+});
+
+export const Default = Template.bind({});

app/assets/javascripts/vue_shared/components/settings/settings_block.vue

 <script>
 import { GlButton } from '@gitlab/ui';
+import { uniqueId } from 'lodash';
+
 import { __ } from '~/locale';
 
 export default {
@@ -15,50 +17,99 @@ export default {
       default: false,
       required: false,
     },
+    collapsible: {
+      type: Boolean,
+      default: true,
+      required: false,
+    },
   },
   data() {
     return {
-      sectionExpanded: false,
+      // Non-collapsible sections should always be expanded.
+      // For collapsible sections, fall back to defaultExpanded.
+      sectionExpanded: !this.collapsible || this.defaultExpanded,
     };
   },
   computed: {
-    expanded() {
-      return this.defaultExpanded || this.sectionExpanded;
-    },
     toggleText() {
-      return this.expanded ? __('Collapse') : __('Expand');
+      const { collapseText, expandText } = this.$options.i18n;
+      return this.sectionExpanded ? collapseText : expandText;
+    },
+    settingsContentId() {
+      return uniqueId('settings_content_');
+    },
+    settingsLabelId() {
+      return uniqueId('settings_label_');
+    },
+    toggleButtonAriaLabel() {
+      const { collapseAriaLabel, expandAriaLabel } = this.$options.i18n;
+      return this.sectionExpanded ? collapseAriaLabel : expandAriaLabel;
+    },
+    ariaExpanded() {
+      return String(this.sectionExpanded);
     },
   },
   methods: {
     toggleSectionExpanded() {
       this.sectionExpanded = !this.sectionExpanded;
+
+      if (this.sectionExpanded) {
+        this.$refs.settingsContent.focus();
+      }
     },
   },
+  i18n: {
+    collapseText: __('Collapse'),
+    expandText: __('Expand'),
+    collapseAriaLabel: __('Collapse settings section'),
+    expandAriaLabel: __('Expand settings section'),
+  },
 };
 </script>
 
 <template>
-  <section class="settings" :class="{ 'no-animate': !slideAnimated, expanded }">
+  <section class="settings" :class="{ 'no-animate': !slideAnimated, expanded: sectionExpanded }">
     <div class="settings-header">
       <h4>
         <span
+          v-if="collapsible"
+          :id="settingsLabelId"
           role="button"
           tabindex="0"
           class="gl-cursor-pointer"
-          data-testid="section-title"
+          :aria-controls="settingsContentId"
+          :aria-expanded="ariaExpanded"
+          data-testid="section-title-button"
           @click="toggleSectionExpanded"
+          @keydown.enter.space="toggleSectionExpanded"
         >
           <slot name="title"></slot>
         </span>
+        <template v-else>
+          <slot name="title"></slot>
+        </template>
       </h4>
-      <gl-button @click="toggleSectionExpanded">
+      <gl-button
+        v-if="collapsible"
+        :aria-controls="settingsContentId"
+        :aria-expanded="ariaExpanded"
+        :aria-label="toggleButtonAriaLabel"
+        @click="toggleSectionExpanded"
+      >
         {{ toggleText }}
       </gl-button>
       <p>
         <slot name="description"></slot>
       </p>
     </div>
-    <div class="settings-content">
+    <div
+      :id="settingsContentId"
+      ref="settingsContent"
+      :aria-labelledby="settingsLabelId"
+      tabindex="-1"
+      role="region"
+      class="settings-content"
+    >
       <slot></slot>
     </div>
   </section>

locale/gitlab.pot

@@ -8140,6 +8140,9 @@ msgstr ""
 msgid "Collapse replies"
 msgstr ""
 
+msgid "Collapse settings section"
+msgstr ""
+
 msgid "Collapse sidebar"
 msgstr ""
 
@@ -13686,6 +13689,9 @@ msgstr ""
 msgid "Expand pipeline"
 msgstr ""
 
+msgid "Expand settings section"
+msgstr ""
+
 msgid "Expand sidebar"
 msgstr ""
 

spec/frontend/vue_shared/components/settings/__snapshots__/settings_block_spec.js.snap

@@ -9,8 +9,11 @@ exports[`Settings Block renders the correct markup 1`] = `
   >
     <h4>
       <span
+        aria-controls="settings_content_3"
+        aria-expanded="false"
         class="gl-cursor-pointer"
-        data-testid="section-title"
+        data-testid="section-title-button"
+        id="settings_label_2"
         role="button"
         tabindex="0"
       >
@@ -21,6 +24,9 @@ exports[`Settings Block renders the correct markup 1`] = `
     </h4>
      
     <gl-button-stub
+      aria-controls="settings_content_3"
+      aria-expanded="false"
+      aria-label="Expand settings section"
       buttontextclasses=""
       category="primary"
       icon=""
@@ -40,7 +46,11 @@ exports[`Settings Block renders the correct markup 1`] = `
   </div>
    
   <div
+    aria-labelledby="settings_label_2"
     class="settings-content"
+    id="settings_content_3"
+    role="region"
+    tabindex="-1"
   >
     <div
       data-testid="default-slot"

spec/frontend/vue_shared/components/settings/settings_block_spec.js

 import { GlButton } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
-import component from '~/vue_shared/components/settings/settings_block.vue';
+import SettingsBlock from '~/vue_shared/components/settings/settings_block.vue';
 
 describe('Settings Block', () => {
   let wrapper;
 
   const mountComponent = (propsData) => {
-    wrapper = shallowMount(component, {
+    wrapper = shallowMount(SettingsBlock, {
       propsData,
       slots: {
         title: '<div data-testid="title-slot"></div>',
@@ -24,7 +24,19 @@ describe('Settings Block', () => {
   const findTitleSlot = () => wrapper.find('[data-testid="title-slot"]');
   const findDescriptionSlot = () => wrapper.find('[data-testid="description-slot"]');
   const findExpandButton = () => wrapper.findComponent(GlButton);
-  const findSectionTitle = () => wrapper.find('[data-testid="section-title"]');
+  const findSectionTitleButton = () => wrapper.find('[data-testid="section-title-button"]');
+
+  const expectExpandedState = ({ expanded = true } = {}) => {
+    const settingsExpandButton = findExpandButton();
+
+    expect(wrapper.classes('expanded')).toBe(expanded);
+    expect(settingsExpandButton.text()).toBe(
+      expanded ? SettingsBlock.i18n.collapseText : SettingsBlock.i18n.expandText,
+    );
+    expect(settingsExpandButton.attributes('aria-label')).toBe(
+      expanded ? SettingsBlock.i18n.collapseAriaLabel : SettingsBlock.i18n.expandAriaLabel,
+    );
+  };
 
   it('renders the correct markup', () => {
     mountComponent();
@@ -75,45 +87,41 @@ describe('Settings Block', () => {
     it('is collapsed by default', () => {
       mountComponent();
 
-      expect(wrapper.classes('expanded')).toBe(false);
+      expectExpandedState({ expanded: false });
     });
 
     it('adds expanded class when the expand button is clicked', async () => {
       mountComponent();
 
-      expect(wrapper.classes('expanded')).toBe(false);
-      expect(findExpandButton().text()).toBe('Expand');
-
       await findExpandButton().vm.$emit('click');
 
-      expect(wrapper.classes('expanded')).toBe(true);
-      expect(findExpandButton().text()).toBe('Collapse');
+      expectExpandedState({ expanded: true });
     });
 
     it('adds expanded class when the section title is clicked', async () => {
       mountComponent();
 
-      expect(wrapper.classes('expanded')).toBe(false);
-      expect(findExpandButton().text()).toBe('Expand');
+      await findSectionTitleButton().trigger('click');
 
-      await findSectionTitle().trigger('click');
-
-      expect(wrapper.classes('expanded')).toBe(true);
-      expect(findExpandButton().text()).toBe('Collapse');
+      expectExpandedState({ expanded: true });
     });
 
-    it('is expanded when `defaultExpanded` is true no matter what', async () => {
-      mountComponent({ defaultExpanded: true });
-
-      expect(wrapper.classes('expanded')).toBe(true);
+    describe('when `collapsible` is `false`', () => {
+      beforeEach(() => {
+        mountComponent({ collapsible: false });
+      });
 
-      await findExpandButton().vm.$emit('click');
+      it('does not render clickable section title', () => {
+        expect(findSectionTitleButton().exists()).toBe(false);
+      });
 
-      expect(wrapper.classes('expanded')).toBe(true);
-
-      await findExpandButton().vm.$emit('click');
+      it('contains expanded class', () => {
+        expect(wrapper.classes('expanded')).toBe(true);
+      });
 
-      expect(wrapper.classes('expanded')).toBe(true);
+      it('does not render expand toggle button', () => {
+        expect(findExpandButton().exists()).toBe(false);
+      });
     });
   });
 });