Warn authors when a QTI question has more than 5 options

contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ValidationMessage/__tests__/ValidationMessage.spec.js

@@ -0,0 +1,46 @@
+import { render } from '@testing-library/vue';
+import VueRouter from 'vue-router';
+import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion';
+import ValidationMessage from '../index.vue';
+
+const renderWarnings = texts =>
+  render(
+    {
+      render: h =>
+        h(
+          'div',
+          texts.map(text => h(ValidationMessage, { props: { warning: true }, key: text }, text)),
+        ),
+    },
+    { routes: new VueRouter() },
+  );
+
+const nextFrame = () => new Promise(resolve => requestAnimationFrame(resolve));
+
+describe('ValidationMessage', () => {
+  beforeAll(() => {
+    useKLiveRegion()._mountLiveRegion();
+  });
+
+  it('announces every warning that appears at once', async () => {
+    renderWarnings(['Too many pairs.', 'Too many distractors.']);
+    await nextFrame();
+    expect(window.politeRegionEl).toHaveTextContent('Too many pairs.');
+    expect(window.politeRegionEl).toHaveTextContent('Too many distractors.');
+  });
+
+  it('does not announce a warning removed before it is read', async () => {
+    renderWarnings(['Too many rows.']).unmount();
+    renderWarnings(['Too many options.']);
+    await nextFrame();
+    expect(window.politeRegionEl).not.toHaveTextContent('Too many rows.');
+    expect(window.politeRegionEl).toHaveTextContent('Too many options.');
+  });
+
+  it('is not talked over by an order announcement queued for the next frame', async () => {
+    requestAnimationFrame(() => useKLiveRegion().sendPoliteMessage('Current order: 1. A'));
+    renderWarnings(['Too many options.']);
+    await nextFrame();
+    expect(window.politeRegionEl).toHaveTextContent('Too many options.');
+  });
+});

contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ValidationMessage/index.vue

@@ -15,7 +15,7 @@
     <p
       v-else
       class="validation-message"
-      :role="alert ? 'alert' : null"
+      :role="announce ? 'alert' : null"
     >
       <slot></slot>
     </p>
@@ -26,25 +26,49 @@
 
 <script>
 
-  import { onMounted, ref } from 'vue';
+  import { onBeforeUnmount, onMounted, ref } from 'vue';
   import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion';
 
+  // KDS writes each message over the live region's text, so a warning sent in
+  // the same frame as another would replace it before it is read.
+  let sameFrameWarnings = [];
+  let sameFrameRequest = null;
+
+  function announceSameFrameWarnings() {
+    sameFrameWarnings[0].sendPoliteMessage(sameFrameWarnings.map(w => w.text).join(' '));
+    sameFrameWarnings = [];
+    sameFrameRequest = null;
+  }
+
   export default {
     name: 'ValidationMessage',
 
     setup(props) {
       const { sendPoliteMessage } = useKLiveRegion();
       const warningText = ref(null);
+      let queuedWarning = null;
 
       // A polite region inserted already filled is often skipped by screen readers,
       // so announce through KDS's persistent live region instead.
       function announceWarning() {
-        if (props.warning) {
-          sendPoliteMessage(warningText.value.textContent.trim());
+        if (!props.warning || !props.announce) return;
+        queuedWarning = { text: warningText.value.textContent.trim(), sendPoliteMessage };
+        sameFrameWarnings.push(queuedWarning);
+        if (sameFrameRequest === null) {
+          sameFrameRequest = requestAnimationFrame(announceSameFrameWarnings);
+        }
+      }
+
+      function dropQueuedWarning() {
+        sameFrameWarnings = sameFrameWarnings.filter(w => w !== queuedWarning);
+        if (!sameFrameWarnings.length && sameFrameRequest !== null) {
+          cancelAnimationFrame(sameFrameRequest);
+          sameFrameRequest = null;
         }
       }
 
       onMounted(announceWarning);
+      onBeforeUnmount(dropQueuedWarning);
 
       return { warningText };
     },
@@ -56,10 +80,10 @@
         default: false,
       },
       /**
-       * Whether an error is an alert, read out as it appears. Off when the component showing
-       * it announces its messages itself.
+       * Whether the message is read out as it appears: an error as an alert, a warning through
+       * KDS's live region. Off when the component showing it announces its messages itself.
        */
-      alert: {
+      announce: {
         type: Boolean,
         default: true,
       },

contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js

@@ -134,6 +134,8 @@ export const ValidationError = Object.freeze({
   DUPLICATE_MATCH_CONTENT: 'DUPLICATE_MATCH_CONTENT',
 });
 
+export const RECOMMENDED_MAX_OPTIONS = 5;
+
 export const RESPONSE_IDENTIFIER = 'RESPONSE';
 
 export const SENTINEL_MARKER = 'data-studio-sentinel';

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/Editor.vue

@@ -55,10 +55,18 @@
     <div
       v-if="mode === 'edit' || showAnswers"
       class="editor-section"
+      data-testid="pairsSection"
     >
       <ValidationMessage v-if="tooFewPairsError">
         {{ errorTooFewPairs$() }}
       </ValidationMessage>
+      <ValidationMessage
+        v-if="tooManyPairsWarning"
+        key="too-many-pairs-warning"
+        warning
+      >
+        {{ warningTooManyPairs$({ count: RECOMMENDED_MAX_OPTIONS }) }}
+      </ValidationMessage>
 
       <div class="section-headers">
         <h4
@@ -196,7 +204,15 @@
     <div
       v-if="mode === 'edit'"
       class="editor-section"
+      data-testid="distractorsSection"
     >
+      <ValidationMessage
+        v-if="tooManyDistractorsWarning"
+        key="too-many-distractors-warning"
+        warning
+      >
+        {{ warningTooManyDistractors$({ count: RECOMMENDED_MAX_OPTIONS }) }}
+      </ValidationMessage>
       <div class="section-headers">
         <h4
           class="section-label"
@@ -241,7 +257,7 @@
   import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow';
   import { themeTokens } from 'kolibri-design-system/lib/styles/theme';
   import { qtiEditorStrings } from '../../qtiEditorStrings';
-  import { ValidationError } from '../../constants';
+  import { RECOMMENDED_MAX_OPTIONS, ValidationError } from '../../constants';
   import { useAssociateInteraction } from '../../composables/useAssociateInteraction';
   import ValidationMessage from '../../components/ValidationMessage/index.vue';
   import AddListItemButton from '../../components/AddListItemButton/index.vue';
@@ -300,6 +316,8 @@
         errorDuplicatePairContent$,
         errorDuplicateDistractorContent$,
         errorEmptyChoiceContent$,
+        warningTooManyPairs$,
+        warningTooManyDistractors$,
       } = qtiEditorStrings;
 
       const questionTypeRef = computed(() => props.questionType);
@@ -451,6 +469,14 @@
         errorCodes.value.includes(ValidationError.TOO_FEW_PAIRS),
       );
 
+      const tooManyPairsWarning = computed(
+        () => props.mode === 'edit' && state.value.pairs.length > RECOMMENDED_MAX_OPTIONS,
+      );
+
+      const tooManyDistractorsWarning = computed(
+        () => state.value.distractors.length > RECOMMENDED_MAX_OPTIONS,
+      );
+
       // Blankness is re-derived from the content rather than read off the
       // EMPTY_CHOICE_CONTENT error's id, because an id may repeat across choices.
       function isBlank(choice) {
@@ -577,6 +603,9 @@
         onAddPair,
         promptHasError,
         tooFewPairsError,
+        tooManyPairsWarning,
+        tooManyDistractorsWarning,
+        RECOMMENDED_MAX_OPTIONS,
         pairErrorMessages,
         distractorErrorMessages,
         pairItemStyles,
@@ -596,6 +625,8 @@
         addDistractorBtn$,
         editPairItemLabel$,
         errorTooFewPairs$,
+        warningTooManyPairs$,
+        warningTooManyDistractors$,
       };
     },
 

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/__tests__/Editor.spec.js

@@ -2,6 +2,7 @@ import { render, screen, within } from '@testing-library/vue';
 import userEvent from '@testing-library/user-event';
 import { nextTick } from 'vue';
 import VueRouter from 'vue-router';
+import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion';
 import AssociateEditor from '../Editor.vue';
 
 import {
@@ -15,6 +16,16 @@ import { parseXML } from '../../../serialization/xml';
 import { qtiEditorStrings as tr } from '../../../qtiEditorStrings';
 
 jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor');
+jest.mock('kolibri-design-system/lib/composables/useKLiveRegion');
+
+let sendPoliteMessage;
+
+const nextFrame = () => new Promise(resolve => requestAnimationFrame(resolve));
+
+beforeEach(() => {
+  sendPoliteMessage = jest.fn();
+  useKLiveRegion.mockReturnValue({ sendPoliteMessage });
+});
 
 let mockWindowIsLarge = true;
 let mockWindowIsSmall = false;
@@ -397,6 +408,140 @@ describe('AssociateEditor', () => {
     });
   });
 
+  describe('too many pairs and distractors warnings', () => {
+    const pairsWarning = () => tr.$tr('warningTooManyPairs', { count: 5 });
+    const distractorsWarning = () => tr.$tr('warningTooManyDistractors', { count: 5 });
+    const range = count => Array.from({ length: count }, (_, i) => i);
+    const choice = (id, content) =>
+      `<qti-simple-associable-choice identifier="${id}" match-max="1">${content}</qti-simple-associable-choice>`;
+    const countedXml = (
+      pairs,
+      distractors,
+    ) => `<qti-associate-interaction response-identifier="RESPONSE">
+      <qti-prompt><p>Match each character to his adversary.</p></qti-prompt>
+      ${range(pairs)
+        .map(i => choice(`left${i}`, `Left ${i}`) + choice(`right${i}`, `Right ${i}`))
+        .join('')}
+      ${range(distractors)
+        .map(i => choice(`extra${i}`, `Extra ${i}`))
+        .join('')}
+    </qti-associate-interaction>`;
+    const countedDecl =
+      pairs => `<qti-response-declaration identifier="RESPONSE" cardinality="multiple" base-type="pair">
+      <qti-correct-response>
+        ${range(pairs)
+          .map(i => `<qti-value>left${i} right${i}</qti-value>`)
+          .join('')}
+      </qti-correct-response>
+    </qti-response-declaration>`;
+    const renderCounted = (pairs, distractors, props = {}) =>
+      renderEditor({
+        interaction: blockWithDecl(countedXml(pairs, distractors), countedDecl(pairs)),
+        ...props,
+      });
+
+    it('does not warn at 5 pairs and 5 distractors', async () => {
+      renderCounted(5, 5);
+      expect(screen.queryByText(pairsWarning())).not.toBeInTheDocument();
+      expect(screen.queryByText(distractorsWarning())).not.toBeInTheDocument();
+      await nextFrame();
+      expect(sendPoliteMessage).not.toHaveBeenCalled();
+    });
+
+    it('warns only about pairs at 6 pairs, without reporting an error', async () => {
+      const { emitted } = renderCounted(6, 5);
+      expect(
+        within(screen.getByTestId('pairsSection')).getByText(pairsWarning()),
+      ).toBeInTheDocument();
+      await nextFrame();
+      expect(sendPoliteMessage).toHaveBeenCalledWith(pairsWarning());
+      expect(screen.queryByText(distractorsWarning())).not.toBeInTheDocument();
+      expect(emitted()['update:errors'].at(-1)[0]).toEqual([]);
+    });
+
+    it('warns only about distractors at 6 distractors, without reporting an error', async () => {
+      const { emitted } = renderCounted(5, 6);
+      expect(
+        within(screen.getByTestId('distractorsSection')).getByText(distractorsWarning()),
+      ).toBeInTheDocument();
+      await nextFrame();
+      expect(sendPoliteMessage).toHaveBeenCalledWith(distractorsWarning());
+      expect(screen.queryByText(pairsWarning())).not.toBeInTheDocument();
+      expect(emitted()['update:errors'].at(-1)[0]).toEqual([]);
+    });
+
+    it('warns and announces both at 6 pairs and 6 distractors', async () => {
+      renderCounted(6, 6);
+      expect(
+        within(screen.getByTestId('pairsSection')).getByText(pairsWarning()),
+      ).toBeInTheDocument();
+      expect(
+        within(screen.getByTestId('distractorsSection')).getByText(distractorsWarning()),
+      ).toBeInTheDocument();
+      await nextFrame();
+      const announced = sendPoliteMessage.mock.calls.at(-1)[0];
+      expect(announced).toContain(pairsWarning());
+      expect(announced).toContain(distractorsWarning());
+    });
+
+    it('warns as soon as a blank 6th pair is added', async () => {
+      const user = userEvent.setup();
+      renderCounted(5, 0);
+      await user.click(button(tr.$tr('addPairBtn')));
+      expect(
+        within(screen.getByTestId('pairsSection')).getByText(pairsWarning()),
+      ).toBeInTheDocument();
+      await nextFrame();
+      expect(sendPoliteMessage).toHaveBeenCalledWith(pairsWarning());
+    });
+
+    it('warns once a 6th distractor is committed', async () => {
+      const user = userEvent.setup();
+      renderCounted(1, 5);
+      await user.click(button(tr.$tr('addDistractorBtn')));
+      await user.type(openTextarea(), 'Demetrius');
+      await user.click(button(editPairItem(1, 1)));
+      expect(
+        within(screen.getByTestId('distractorsSection')).getByText(distractorsWarning()),
+      ).toBeInTheDocument();
+      await nextFrame();
+      expect(sendPoliteMessage).toHaveBeenCalledWith(distractorsWarning());
+    });
+
+    it('stops warning once a pair is deleted, leaving 5', async () => {
+      const user = userEvent.setup();
+      renderCounted(6, 0);
+      await user.click(button(tr.$tr('deletePairBtn', { number: 6 })));
+      expect(screen.queryByText(pairsWarning())).not.toBeInTheDocument();
+    });
+
+    it('stops warning once a distractor is deleted, leaving 5', async () => {
+      const user = userEvent.setup();
+      renderCounted(1, 6);
+      await user.click(button(tr.$tr('deleteDistractorBtn', { number: 6 })));
+      expect(screen.queryByText(distractorsWarning())).not.toBeInTheDocument();
+    });
+
+    it('still adds a 7th pair', async () => {
+      const user = userEvent.setup();
+      renderCounted(6, 0);
+      await user.click(button(tr.$tr('addPairBtn')));
+      expect(
+        within(screen.getByTestId('pairsSection')).getByText(
+          tr.$tr('pairNumberLabel', { number: 7 }),
+        ),
+      ).toBeInTheDocument();
+    });
+
+    it('does not warn in view mode', async () => {
+      renderCounted(6, 6, { mode: 'view', showAnswers: true });
+      expect(screen.queryByText(pairsWarning())).not.toBeInTheDocument();
+      expect(screen.queryByText(distractorsWarning())).not.toBeInTheDocument();
+      await nextFrame();
+      expect(sendPoliteMessage).not.toHaveBeenCalled();
+    });
+  });
+
   describe('view mode', () => {
     const viewProps = { mode: 'view' };