diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md index 3beea680fa..06f8615e01 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md @@ -9,6 +9,8 @@ Edits an exercise's assessment items stored as [QTI 3](https://www.imsglobal.org 4. The interaction's `Editor.vue` (`interactions/index.js`) edits a plain state object through `composables/useInteraction.js`: `descriptor.parse` → state → `descriptor.buildXML` → `descriptor.validate`. 5. `useQtiItem` rebuilds the full item with `serialization/assembleItem.js`; `QTIItemEditor` emits `update:rawData`. `validateItem.js` `isEditableItem` shows read-only any item holding content outside the body that this rebuild doesn't write or regenerate. +Changing a question's type goes through `conversion/`: the old descriptor exports its state, the new one imports it, and whatever the change would remove or change is listed in `components/ChangeQuestionTypeModal` for the author to confirm first. + `validateItem.js` `validateQtiItem` runs the same parse and validation headless, without the Vue editors. > [!IMPORTANT] @@ -19,15 +21,17 @@ Edits an exercise's assessment items stored as [QTI 3](https://www.imsglobal.org - `Descriptor.js` — subclass of `InteractionDescriptor`: matches the element, picks the question type, delegates to `parse.js` and `validation.js`. - `parse.js` — XML ↔ state. - `validation.js` — state → error codes. + - `convert.js` — state ↔ the neutral facets a question type change carries (answers, pairs, a passage). - `Editor.vue` — the UI; absent for headless interactions (`HEADLESS_INTERACTIONS`). - `interactions/descriptors.js` — descriptor registry, free of `.vue` imports. `interactions/index.js` adds the editors. - `serialization/` — item-level parse/assemble, hints (`qti-catalog-info`), XML helpers (`xml.js`), response declarations (`qti/`). - `components/` — shared UI: hints, chip lists, toolbars, question type selector. ### Adding an interaction -1. Create `interactions//` with `Descriptor.js`, `parse.js`, `validation.js` and `Editor.vue`. +1. Create `interactions//` with `Descriptor.js`, `parse.js`, `validation.js`, `convert.js` and `Editor.vue`. 2. Register the descriptor in `interactions/descriptors.js` and the editor in `interactions/index.js`. 3. Add its question types to `constants.js` and strings to `qtiEditorStrings.js`. +4. Override `getConversionFormat`, `exportContent` and `importContent` with `convert.js`, so its answers carry over when an author changes the question's type, and add a filled question of each type to `conversion/__tests__/questions.js`. Until it does, a type change from it keeps only the question without telling the author, which is why the conversion tests fail without it. ### Rich text fields Prompts, choices, items and hints are edited in `TipTapEditor` with `format="html"`. diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ChangeQuestionTypeModal/__tests__/ChangeQuestionTypeModal.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ChangeQuestionTypeModal/__tests__/ChangeQuestionTypeModal.spec.js new file mode 100644 index 0000000000..2478037f54 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ChangeQuestionTypeModal/__tests__/ChangeQuestionTypeModal.spec.js @@ -0,0 +1,71 @@ +import { render, screen, within } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; +import VueRouter from 'vue-router'; +import ChangeQuestionTypeModal from '../index.vue'; +import { ConversionLoss } from '../../../constants'; +import { qtiEditorStrings as tr } from '../../../qtiEditorStrings'; + +const renderModal = (props = {}) => + render(ChangeQuestionTypeModal, { + props: { + questionTypeLabel: 'Numeric input', + losses: [ConversionLoss.INCORRECT_ANSWERS_REMOVED, ConversionLoss.FORMATTING_REMOVED], + ...props, + }, + routes: new VueRouter(), + }); + +describe('ChangeQuestionTypeModal', () => { + it('asks to confirm the change, naming the new type', () => { + renderModal(); + const dialog = screen.getByRole('dialog'); + expect(within(dialog).getByText(tr.$tr('changeQuestionTypeTitle'))).toBeInTheDocument(); + expect( + within(dialog).getByText(tr.$tr('changeQuestionTypeIntro', { type: 'Numeric input' })), + ).toBeInTheDocument(); + }); + + it('lists what the change would remove or change, one sentence each', () => { + renderModal(); + const items = screen.getAllByRole('listitem').map(item => item.textContent.trim()); + expect(items).toEqual([tr.$tr('incorrectAnswersRemovedLoss'), tr.$tr('formattingRemovedLoss')]); + }); + + it('shows each blank of a folded passage as it will appear in the question', () => { + renderModal({ losses: [ConversionLoss.PASSAGE_ADDED_TO_QUESTION] }); + expect(screen.getByRole('listitem')).toHaveTextContent(/"_____"/); + }); + + it.each(Object.values(ConversionLoss))('has a sentence for %s', loss => { + renderModal({ losses: [loss] }); + expect(screen.getByRole('listitem').textContent.trim()).not.toBe(''); + }); + + it('confirms with the Change button', async () => { + const user = userEvent.setup(); + const { emitted } = renderModal(); + await user.click(screen.getByRole('button', { name: tr.$tr('changeQuestionTypeAction') })); + expect(emitted().submit).toHaveLength(1); + expect(emitted().cancel).toBeUndefined(); + }); + + it('cancels with the Cancel button', async () => { + const user = userEvent.setup(); + const { emitted } = renderModal(); + await user.click(screen.getByRole('button', { name: tr.$tr('cancelAction') })); + expect(emitted().cancel).toHaveLength(1); + expect(emitted().submit).toBeUndefined(); + }); + + it('can be used with the keyboard only', async () => { + const user = userEvent.setup(); + const { emitted } = renderModal(); + await user.keyboard('{Escape}'); + expect(emitted().cancel).toHaveLength(1); + + const change = screen.getByRole('button', { name: tr.$tr('changeQuestionTypeAction') }); + change.focus(); + await user.keyboard('{Enter}'); + expect(emitted().submit).toHaveLength(1); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ChangeQuestionTypeModal/index.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ChangeQuestionTypeModal/index.vue new file mode 100644 index 0000000000..4f1a286cf7 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/ChangeQuestionTypeModal/index.vue @@ -0,0 +1,99 @@ + + + + + + + diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js index 83c931c7b7..d7eb5caef4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js @@ -1,4 +1,5 @@ -import { render, screen, fireEvent, within } from '@testing-library/vue'; +import { render, screen, within, fireEvent } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; import { nextTick } from 'vue'; import VueRouter from 'vue-router'; import InteractionSection from '../index.vue'; @@ -9,7 +10,12 @@ import { CHOICE_MULTI_SELECT_XML, UNKNOWN_INTERACTION_XML, MATCH_THREE_SETS_XML, + CHOICE_SINGLE_DECL_XML, + TEXT_ENTRY_BODY_XML, + TEXT_ENTRY_NUMERIC_DECL_XML, + TEXT_ENTRY_STRING_DECL_XML, mockInteractionBlock as interactionBlock, + mockInteractionBlockWithDecl, } from '../../../utils/testingFixtures'; jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor'); @@ -77,20 +83,26 @@ describe('InteractionSection', () => { // Feeds each emitted interaction back in, as the item editor does. const SwitchableSection = { components: { InteractionSection }, + props: { + initial: { + type: Object, + default: () => interactionBlock(CHOICE_SINGLE_SELECT_XML), + }, + language: { + type: String, + default: '', + }, + }, template: ` `, data() { - return { - interactionBlock: { - bodyXml: CHOICE_SINGLE_SELECT_XML, - responseDeclarations: [], - }, - }; + return { interactionBlock: this.initial }; }, methods: { onUpdate(val) { @@ -100,43 +112,194 @@ describe('InteractionSection', () => { }, }; + const renderSwitchable = (initial, { language } = {}) => + render(SwitchableSection, { + props: { ...(initial && { initial }), ...(language !== undefined && { language }) }, + routes: new VueRouter(), + }); + + const selector = () => screen.getByRole('group', { name: tr.$tr('typeLabel') }); + async function pickType(user, toLabel) { + await user.click(within(selector()).getByText(tr.$tr('responseTypeLabel'))); + await user.click(screen.getAllByText(tr.$tr(toLabel)).at(-1)); + await nextTick(); + } + + // Mercury, marked correct, with Venus and Earth. + const markedSingleSelect = () => + mockInteractionBlockWithDecl(CHOICE_SINGLE_SELECT_XML, CHOICE_SINGLE_DECL_XML); + describe('type switching', () => { - it('preserves the prompt but resets choices when switching from choice to text-entry', async () => { - const { emitted } = render(SwitchableSection, { - routes: new VueRouter(), - }); + it('switches straight away when nothing would be lost', async () => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(markedSingleSelect()); + await nextTick(); + + await pickType(user, 'multiSelectLabel'); + + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + const switched = emitted()['wrapper-update'].at(-1)[0]; + expect(switched.responseDeclarations[0]).toContain('cardinality="multiple"'); + const answers = screen.getByRole('group', { name: tr.$tr('answersLabel') }); + const checkboxes = within(answers).getAllByRole('checkbox'); + expect(checkboxes).toHaveLength(3); + expect(checkboxes[0]).toBeChecked(); + }); + + it('asks first when the change would remove part of the question', async () => { + const user = userEvent.setup(); + renderSwitchable(markedSingleSelect()); + await nextTick(); + + await pickType(user, 'textEntryLabel'); + + const dialog = screen.getByRole('dialog'); + expect(within(dialog).getByText(tr.$tr('incorrectAnswersRemovedLoss'))).toBeInTheDocument(); + expect( + within(dialog).getByText( + tr.$tr('changeQuestionTypeIntro', { type: tr.$tr('textEntryLabel') }), + ), + ).toBeInTheDocument(); + }); + it('converts the question once the author confirms', async () => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(markedSingleSelect()); await nextTick(); - const selectedOption = screen.getAllByText(tr.$tr('singleSelectLabel'))[0]; - await fireEvent.click(selectedOption); + await pickType(user, 'textEntryLabel'); + await user.click(screen.getByRole('button', { name: tr.$tr('changeQuestionTypeAction') })); + await nextTick(); - const textEntryOption = screen.getByText(tr.$tr('textEntryLabel')); - await fireEvent.click(textEntryOption); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + const switched = emitted()['wrapper-update'].at(-1)[0]; + expect(switched.bodyXml).toContain('Which planet is closest to the Sun?'); + expect(switched.bodyXml).toContain(' { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(markedSingleSelect()); await nextTick(); + const before = emitted()['wrapper-update']?.length ?? 0; - const emits = emitted()['wrapper-update']; - const switchXml = emits.at(-1)[0].bodyXml; + await pickType(user, 'textEntryLabel'); + await user.click(screen.getByRole('button', { name: tr.$tr('cancelAction') })); + await nextTick(); - expect(switchXml).toContain('Which planet is closest to the Sun?'); - expect(switchXml).toContain(' { + const user = userEvent.setup(); + renderSwitchable(markedSingleSelect()); + await nextTick(); + + await pickType(user, 'textEntryLabel'); + await user.click(screen.getByRole('button', { name: tr.$tr('cancelAction') })); + await nextTick(); + await pickType(user, 'textEntryLabel'); + + expect(screen.getByRole('dialog')).toBeInTheDocument(); + }); + + describe('focus', () => { + // A rich text editor focusing itself does so in a timeout and then an animation frame, as + // TipTap does, while the mock would at once. Timed like the real one, an editor remounted + // for the new type that took focus again would show here, after the select. + beforeEach(() => { + const focus = HTMLTextAreaElement.prototype.focus; + jest.spyOn(HTMLTextAreaElement.prototype, 'focus').mockImplementation(function () { + setTimeout(() => requestAnimationFrame(() => focus.call(this))); + }); + }); + + afterEach(() => { + HTMLTextAreaElement.prototype.focus.mockRestore(); + }); + + const settle = () => + new Promise(resolve => setTimeout(() => requestAnimationFrame(() => setTimeout(resolve)))); + + it('stays on the type selector after a switch with nothing to confirm', async () => { + const user = userEvent.setup(); + renderSwitchable(markedSingleSelect()); + await nextTick(); + + // From choice to free response, then on to text entry, which loses nothing. + await pickType(user, 'freeResponseLabel'); + await user.click(screen.getByRole('button', { name: tr.$tr('changeQuestionTypeAction') })); + await settle(); + await pickType(user, 'textEntryLabel'); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + await settle(); + + expect(selector()).toContainElement(document.activeElement); + }); + + it.each([ + ['confirming', 'changeQuestionTypeAction'], + ['cancelling', 'cancelAction'], + ])('goes back to the type selector after %s', async (_, button) => { + const user = userEvent.setup(); + renderSwitchable(markedSingleSelect()); + await nextTick(); + + await pickType(user, 'textEntryLabel'); + await user.click(screen.getByRole('button', { name: tr.$tr(button) })); + await settle(); + + expect(selector()).toContainElement(document.activeElement); + }); + }); + + it('converts what the author has just typed', async () => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(markedSingleSelect()); + await nextTick(); + + await user.click( + screen.getByRole('button', { name: tr.$tr('editAnswerOptionLabel', { number: 1 }) }), + ); + const mercury = screen.getByDisplayValue('Mercury'); + await user.clear(mercury); + await user.type(mercury, 'Mercury, the smallest planet'); + await pickType(user, 'multiSelectLabel'); + + expect(emitted()['wrapper-update'].at(-1)[0].bodyXml).toContain( + 'Mercury, the smallest planet', + ); }); - }); - describe('switching to match', () => { it('produces a match item that reopens in the match editor', async () => { - const { emitted, unmount } = render(SwitchableSection, { routes: new VueRouter() }); + const user = userEvent.setup(); + const { emitted, unmount } = renderSwitchable(); await nextTick(); - await fireEvent.click(screen.getAllByText(tr.$tr('singleSelectLabel'))[0]); - await fireEvent.click(screen.getByText(tr.$tr('matchLabel'))); + await pickType(user, 'matchLabel'); + expect( + within(screen.getByRole('dialog')).getByText(tr.$tr('answersRemovedLoss')), + ).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: tr.$tr('changeQuestionTypeAction') })); await nextTick(); const switched = emitted()['wrapper-update'].at(-1)[0]; expect(switched.bodyXml).toContain('Which planet is closest to the Sun?'); expect(switched.bodyXml.match(/ { describe('switching to inline choice', () => { it('keeps the prompt as the question and shows the passage editor', async () => { - const { emitted } = render(SwitchableSection, { routes: new VueRouter() }); + const user = userEvent.setup(); + const { emitted } = renderSwitchable(); await nextTick(); - await fireEvent.click(screen.getAllByText(tr.$tr('singleSelectLabel'))[0]); - await fireEvent.click(screen.getByText(tr.$tr('inlineChoiceLabel'))); + await pickType(user, 'inlineChoiceLabel'); + await user.click(screen.getByRole('button', { name: tr.$tr('changeQuestionTypeAction') })); await nextTick(); const switched = emitted()['wrapper-update'].at(-1)[0]; @@ -167,6 +331,125 @@ describe('InteractionSection', () => { }); }); + // Changing the question type remounts the editor on the converted question, so numbers move + // between how the exercise language writes them and how they are stored as it opens. + describe('numeric answers across a type change', () => { + const numericAnswers = () => + screen.queryAllByRole('textbox', { name: tr.$tr('answerValuePlaceholder') }); + const latestDeclaration = emitted => + emitted()['wrapper-update'].at(-1)[0].responseDeclarations[0]; + const numeric = stored => + mockInteractionBlockWithDecl( + TEXT_ENTRY_BODY_XML, + TEXT_ENTRY_NUMERIC_DECL_XML.replace('42', stored), + ); + const textEntry = text => + mockInteractionBlockWithDecl( + TEXT_ENTRY_BODY_XML, + TEXT_ENTRY_STRING_DECL_XML.replace('H2O', text), + ); + + it.each([ + ['fr', '1234.5'], + ['en', '1.50'], + ['en', '0012'], + ['ar-EG', '30'], + ['ar-EG', '-3.14'], + ['fr', '1.2.3'], + ['he', '-0.5'], + ['sv', '-0.5'], + ['ar-EG', '-0.5'], + ])('gives text entry a numeric answer as it was shown in %s (%s)', async (language, stored) => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(numeric(stored), { language }); + await nextTick(); + const shown = numericAnswers()[0].value; + await pickType(user, 'textEntryLabel'); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(latestDeclaration(emitted)).toContain(`${shown}`); + }); + + it.each([ + ['fr', '11,2'], + ['de', '1.234,5'], + ['en', '1.5'], + ])( + 'gives back a text entry answer in %s (%s) after switching to numeric and back', + async (language, text) => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(textEntry(text), { language }); + await nextTick(); + await pickType(user, 'numericLabel'); + await nextTick(); + const shown = numericAnswers()[0].value; + await pickType(user, 'textEntryLabel'); + expect(latestDeclaration(emitted)).toContain(`${shown}`); + }, + ); + + it.each([ + ['fr', '1,5', '1,5', '1.5'], + ['fr', '1 234,5', '1234,5', '1234.5'], + // Read as xsd:double first, as German would read it as 15. + ['de', '1.5', '1,5', '1.5'], + ])( + 'reads a text entry answer in %s (%s) when switched to numeric', + async (language, text, shown, stored) => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(textEntry(text), { language }); + await nextTick(); + await pickType(user, 'numericLabel'); + await nextTick(); + expect(numericAnswers()[0]).toHaveValue(shown); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(latestDeclaration(emitted)).toContain(`${stored}`); + }, + ); + + it.each([ + ['de', '1.5', '1,5'], + ['es', '1234.5', '1234,5'], + ['fr', '1234.5', '1234,5'], + ['ar-EG', '30', '٣٠'], + ])( + 'shows a stored answer in %s again after switching to text entry and back (%s)', + async (language, stored, shown) => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(numeric(stored), { language }); + await nextTick(); + await pickType(user, 'textEntryLabel'); + await pickType(user, 'numericLabel'); + await nextTick(); + expect(numericAnswers()[0]).toHaveValue(shown); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(latestDeclaration(emitted)).toContain(`${stored}`); + }, + ); + + it.each([ + ['fr', '1,5'], + ['en', '1,234'], + ['de', '1.500'], + ['', ' 5 '], + ['fa', 'می\u200cخواهم'], + ['en', 'a\u00adb'], + ])( + 'gives text entry a typed numeric answer as it is shown in "%s" (%s)', + async (language, typed) => { + const user = userEvent.setup(); + const { emitted } = renderSwitchable(numeric('42'), { language }); + await nextTick(); + const [input] = numericAnswers(); + await fireEvent.input(input, { target: { value: typed } }); + // Leaving the answer shows it as stored, written in the language. + await fireEvent.blur(input); + const shown = input.value; + await pickType(user, 'textEntryLabel'); + expect(latestDeclaration(emitted)).toContain(`${shown.trim()}`); + }, + ); + }); + describe('unknown interaction type', () => { it('renders nothing editable', () => { renderSection({ interaction: interactionBlock(UNKNOWN_INTERACTION_XML), mode: 'edit' }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue index 81a97ccf4a..6a014f5ee6 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue @@ -10,15 +10,21 @@
+ + +
@@ -36,17 +50,22 @@

', + distractors: [], + }; + expect(exportGapMatchContent(state).passage.blanks).toEqual([{ answer: 'Sun', options: [] }]); + }); + + it('imports each blank as a gap holding its answer, and an unanswered one as an empty gap', () => { + const state = importGapMatchContent({ + prompt: '

Q

', + passage: passageFacet([ + { answer: 'Sun', options: [] }, + { answer: null, options: [] }, + ]), + }); + expect(state.prompt).toBe('

Q

'); + expect(getGaps(state.passage).map(({ answer }) => answer)).toEqual(['Sun', '']); + expect(state.distractors).toEqual([]); + }); + + it('pools every blank’s options with the distractors, each once and none an answer', () => { + const state = importGapMatchContent({ + prompt: '', + passage: passageFacet( + [ + { answer: 'Sun', options: ['Moon', 'Star'] }, + { answer: 'Moon', options: ['Sun', 'Star', 'Comet'] }, + ], + ['Comet', 'Planet', ''], + ), + }); + expect(state.distractors.map(({ content }) => content)).toEqual(['Star', 'Comet', 'Planet']); + }); + + it('keeps the question alone when there is no passage to import', () => { + expect(importGapMatchContent({ prompt: '

Q

' })).toEqual({ + responseIdentifier: 'RESPONSE', + prompt: '

Q

', + passage: '', + distractors: [], + }); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/convert.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/convert.js new file mode 100644 index 0000000000..95f2386da7 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/convert.js @@ -0,0 +1,87 @@ +import flatMap from 'lodash/flatMap'; +import { ContentLevel } from '../../constants'; +import { getContentHTML, parseHTMLBody } from '../../serialization/xml'; +import { BLANK_ATTR } from '../../conversion/passage'; +import { GAP, _defaultState, newDistractor, readGap, writeGap } from './parse'; +import { + hasRichTextContent, + richTextComparisonKey, +} from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + +/** + * @returns {import('../../conversion').ConversionFormat} + */ +export function getGapMatchConversionFormat() { + return { passage: { contentLevel: ContentLevel.INLINE } }; +} + +/** + * Each gap becomes a blank, its content the blank's answer. A gap has no options of its own: + * every wrong answer is in the shared pool of distractors. + * + * @param {import('./parse').GapMatchState} state + * @returns {import('../../conversion').ConvertibleContent} + */ +export function exportGapMatchContent(state) { + const body = parseHTMLBody(state.passage); + const blanks = [...body.querySelectorAll(GAP)].map((el, index) => { + const { answer } = readGap(el); + const placeholder = body.ownerDocument.createElement('span'); + placeholder.setAttribute(BLANK_ATTR, String(index)); + el.replaceWith(placeholder); + return { answer: hasRichTextContent(answer) ? answer : null, options: [] }; + }); + return { + prompt: state.prompt, + passage: { + html: getContentHTML(body), + blanks, + distractors: state.distractors.map(({ content }) => content), + contentLevel: ContentLevel.INLINE, + // Learners always see the pool shuffled; otherwise the answers would come in order. + shuffle: true, + }, + }; +} + +/** + * Each blank becomes a gap holding its answer. Every blank's own options join the shared + * pool, which is what a gap match learner chooses from. The conversion repeats an option + * offered in several dropdowns, or one that is another blank's answer, so those repeats are + * dropped rather than left for validation to flag. + * + * @param {import('../../conversion').ConvertibleContent} content + * @returns {import('./parse').GapMatchState} + */ +export function importGapMatchContent({ prompt, passage }) { + const state = _defaultState(); + if (!passage) { + return { ...state, prompt }; + } + const body = parseHTMLBody(passage.html); + for (const placeholder of body.querySelectorAll(`[${BLANK_ATTR}]`)) { + const blank = passage.blanks[Number(placeholder.getAttribute(BLANK_ATTR))]; + if (!blank) { + placeholder.remove(); + continue; + } + placeholder.replaceWith(writeGap(body.ownerDocument, blank.answer ?? '')); + } + + const offered = new Set( + passage.blanks.map(({ answer }) => richTextComparisonKey(answer)).filter(Boolean), + ); + const distractors = []; + for (const content of [ + ...flatMap(passage.blanks, ({ options }) => options), + ...passage.distractors, + ]) { + const key = richTextComparisonKey(content); + if (key && !offered.has(key)) { + offered.add(key); + distractors.push(newDistractor(content)); + } + } + + return { ...state, prompt, passage: getContentHTML(body), distractors }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index 08385dbfb4..799fe62d67 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -5,6 +5,7 @@ import { getContentHTML, getPromptHTML, isContentNode, + parseHTMLBody, parseXML, serializeAsHtml, wrapInlineRuns, @@ -19,7 +20,7 @@ import { const serializer = new XMLSerializer(); -const GAP = 'qti-gap'; +export const GAP = 'qti-gap'; const GAP_TEXT = 'qti-gap-text'; const PROMPT = 'qti-prompt'; const CORRECT_ATTR = 'data-studio-correct'; @@ -48,10 +49,45 @@ const SENTINEL_ID = 'studio_sentinel'; * @property {string} answer - HTML of the gap's correct answer; '' when blank */ -function defaultState() { +export function _defaultState() { return { responseIdentifier: RESPONSE_IDENTIFIER, prompt: '', passage: '', distractors: [] }; } +/** + * A new distractor, with a fresh identifier. + * + * @param {string} [content] + * @returns {GapMatchDistractor} + */ +export const newDistractor = (content = '') => ({ id: generateRandomSlug('choice'), content }); + +/** + * @param {Element} el - A `` as the passage holds it + * @returns {GapMatchGap} + */ +export function readGap(el) { + return { + id: el.getAttribute('identifier'), + answerId: el.getAttribute(CORRECT_ATTR), + answer: getContentHTML(el), + }; +} + +/** + * A new gap, with a fresh identifier, as the passage holds it: holding its answer, which the + * build writes out as its gap text. The reverse of readGap. + * + * @param {Document} doc - The document of the passage it goes in + * @param {string} answer - HTML of its correct answer; '' for a blank gap + * @returns {Element} + */ +export function writeGap(doc, answer) { + const gap = doc.createElement(GAP); + gap.setAttribute('identifier', generateRandomSlug('gap')); + gap.append(...parseHTMLBody(answer).childNodes); + return gap; +} + /** * Read every gap out of the passage, in document order. * @@ -60,11 +96,7 @@ function defaultState() { */ export function getGaps(passage) { const doc = parseXML(passage || '', 'text/html'); - return [...doc.querySelectorAll(GAP)].map(el => ({ - id: el.getAttribute('identifier'), - answerId: el.getAttribute(CORRECT_ATTR), - answer: getContentHTML(el), - })); + return [...doc.querySelectorAll(GAP)].map(readGap); } /** @@ -79,13 +111,13 @@ export function getGaps(passage) { * @returns {GapMatchState} */ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { - if (!bodyXml) return defaultState(); + if (!bodyXml) return _defaultState(); let root; try { root = parseXML(bodyXml).documentElement; } catch { - return defaultState(); + return _defaultState(); } const contentNodes = [...root.childNodes].filter(isContentNode); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Descriptor.js index 75fa995cab..108ff2be16 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Descriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Descriptor.js @@ -1,6 +1,11 @@ import { QtiInteraction, QuestionType, BaseType, Cardinality, Placement } from '../../constants'; import { InteractionDescriptor } from '../InteractionDescriptor'; import { parseInlineChoiceInteraction, buildInlineChoiceInteractionXML } from './parse'; +import { + exportInlineChoiceContent, + getInlineChoiceConversionFormat, + importInlineChoiceContent, +} from './convert'; import { validateInlineChoiceInteraction } from './validation'; /** @@ -82,6 +87,29 @@ class InlineChoiceInteractionDescriptor extends InteractionDescriptor { validate(state) { return validateInlineChoiceInteraction(state); } + + /** + * @returns {import('../../conversion').ConversionFormat} + */ + getConversionFormat() { + return getInlineChoiceConversionFormat(); + } + + /** + * @param {InlineChoiceState} state + * @returns {import('../../conversion').ConvertibleContent} + */ + exportContent(state) { + return exportInlineChoiceContent(state); + } + + /** + * @param {import('../../conversion').ConvertibleContent} content + * @returns {InlineChoiceState} + */ + importContent(content) { + return importInlineChoiceContent(content); + } } export const inlineChoiceInteractionDescriptor = new InlineChoiceInteractionDescriptor(); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue index 6153787ca5..51d81e101a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue @@ -233,10 +233,7 @@ watch( () => props.mode, newMode => { - if (newMode === 'edit') { - // Open the question, the first thing in the card, so the card opens at its start. - openPrompt(); - } else { + if (newMode !== 'edit') { isPromptOpen.value = false; isPassageOpen.value = false; passage.openDropdown(null); @@ -343,6 +340,9 @@ watch(errors, newVal => emit('update:errors', newVal), { immediate: true }); return { + // Public: opens the question for editing, which focuses it. + // eslint-disable-next-line vue/no-unused-properties + focus: openPrompt, EditorImageProcessor, state, errors, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/Editor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/Editor.spec.js index bb255de7e5..509ca5aa0a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/Editor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/Editor.spec.js @@ -7,6 +7,7 @@ import { inlineChoiceInteractionDescriptor as descriptor } from '../Descriptor'; import { getDropdowns } from '../parse'; import { QuestionType } from '../../../constants'; import { qtiEditorStrings as tr } from '../../../qtiEditorStrings'; +import { renderFocused } from '../../__tests__/renderFocused'; import { CHIP, chip } from './renderPassage'; import { dragSortStrings as dragTr } from 'shared/views/dragSort/dragSortStrings'; import { getTipTapEditorStrings } from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditorStrings'; @@ -142,8 +143,8 @@ describe('InlineChoiceEditor', () => { expect(passage).toHaveAccessibleDescription(tr.passageEditorDescription$()); }); - it('opens on the question, focused, with the passage closed', async () => { - render(InlineChoiceEditor, { + it('opens on the question, focused, with the passage closed, when its card opens', async () => { + renderFocused(InlineChoiceEditor, { props: { interaction: interactionOf(), questionType: QuestionType.INLINE_CHOICE, @@ -163,6 +164,23 @@ describe('InlineChoiceEditor', () => { expect(editPassage).toBeInTheDocument(); }); + it('leaves the question closed, and focus where it was, when it only mounts', async () => { + render(InlineChoiceEditor, { + props: { + interaction: interactionOf(), + questionType: QuestionType.INLINE_CHOICE, + mode: 'edit', + teleportTargetId: 'settings-target', + }, + routes: new VueRouter(), + }); + await screen.findByRole('button', { name: tr.editPassageLabel$() }); + expect( + await screen.findByRole('button', { name: tr.editQuestionLabel$() }), + ).toBeInTheDocument(); + expect(document.body).toHaveFocus(); + }); + it('shows no options panel until a chip is opened', async () => { await renderEditor(); expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/convert.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/convert.spec.js new file mode 100644 index 0000000000..44e1547af2 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/__tests__/convert.spec.js @@ -0,0 +1,120 @@ +import flatMap from 'lodash/flatMap'; +import { ContentLevel, ConversionLoss, QuestionType } from '../../../constants'; +import { carryContent } from '../../../conversion'; +import { inlineChoiceInteractionDescriptor as descriptor } from '../Descriptor'; +import { exportInlineChoiceContent, importInlineChoiceContent } from '../convert'; +import { getDropdowns } from '../parse'; + +const dropdown = (name, options, correct) => + `' + + options + .map(text => `${text}`) + .join('') + + ''; + +/** A gap match passage, as gap match exports it: answers inline, one shared pool. */ +const gapMatchPassage = { + html: '

Now is the of our .

', + blanks: [ + { answer: 'winter', options: [] }, + { answer: 'discontent', options: [] }, + ], + distractors: ['summer', 'winter'], + contentLevel: ContentLevel.INLINE, + shuffle: true, +}; + +const summary = passage => + getDropdowns(passage).map(({ options, correctId }) => ({ + options: options.map(({ text }) => text), + correct: options.find(({ id }) => id === correctId)?.text ?? null, + })); + +describe('inline choice conversion', () => { + it('exports each dropdown as a blank, its correct option the answer', () => { + const state = { + prompt: '

Q

', + passage: `

The ${dropdown('a', ['Moon', 'Sun'], 'Sun')} and ${dropdown('b', ['x'])}

`, + shuffle: true, + }; + expect(exportInlineChoiceContent(state)).toEqual({ + prompt: '

Q

', + passage: { + html: '

The and

', + blanks: [ + { answer: 'Sun', options: ['Moon'] }, + { answer: null, options: ['x'] }, + ], + distractors: [], + contentLevel: ContentLevel.PLAIN, + shuffle: true, + }, + }); + }); + + it('imports a passage from gap match, each dropdown offering the whole pool', () => { + const { carried, losses } = carryContent( + { prompt: '

Q

', passage: gapMatchPassage }, + descriptor.getConversionFormat(QuestionType.INLINE_CHOICE), + ); + // The answer was in italics, which an option can't hold. + expect(losses).toEqual([ConversionLoss.FORMATTING_REMOVED]); + + const state = importInlineChoiceContent(carried); + expect(state.prompt).toBe('

Q

'); + expect(state.shuffle).toBe(true); + expect(summary(state.passage)).toEqual([ + { options: ['winter', 'discontent', 'summer'], correct: 'winter' }, + { options: ['discontent', 'winter', 'summer'], correct: 'discontent' }, + ]); + }); + + it('gives a blank with no answer and nothing to offer one empty option, unmarked', () => { + const state = importInlineChoiceContent({ + prompt: '', + passage: { + html: '

', + blanks: [{ answer: null, options: [] }], + distractors: [], + contentLevel: ContentLevel.PLAIN, + }, + }); + expect(summary(state.passage)).toEqual([{ options: [''], correct: null }]); + expect(state.shuffle).toBe(false); + }); + + it('gives every dropdown its own identifiers', () => { + const { carried } = carryContent( + { prompt: '', passage: gapMatchPassage }, + descriptor.getConversionFormat(QuestionType.INLINE_CHOICE), + ); + const dropdowns = getDropdowns(importInlineChoiceContent(carried).passage); + const ids = flatMap(dropdowns, d => [d.responseIdentifier, ...d.options.map(o => o.id)]); + expect(new Set(ids).size).toBe(ids.length); + }); + + it('imports a new question with the question only, when there is no passage', () => { + expect(importInlineChoiceContent({ prompt: '

Q

' })).toEqual({ + prompt: '

Q

', + passage: '', + shuffle: false, + }); + }); + + it('writes a passage the inline choice editor reads back the same', () => { + const { carried } = carryContent( + { prompt: '

Q

', passage: gapMatchPassage }, + descriptor.getConversionFormat(QuestionType.INLINE_CHOICE), + ); + const state = importInlineChoiceContent(carried); + const { bodyXml, responseDeclarations } = descriptor.buildXML( + state, + QuestionType.INLINE_CHOICE, + ); + const reopened = descriptor.parse(bodyXml, responseDeclarations); + expect(summary(reopened.passage)).toEqual(summary(state.passage)); + expect(descriptor.validate(reopened)).toEqual([]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/convert.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/convert.js new file mode 100644 index 0000000000..6f83a3a6e2 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/convert.js @@ -0,0 +1,94 @@ +import { ContentLevel } from '../../constants'; +import { getContentHTML, parseHTMLBody } from '../../serialization/xml'; +import { BLANK_ATTR } from '../../conversion/passage'; +import { + DROPDOWN, + _defaultState, + newDropdown, + newOption, + readDropdown, + writeDropdown, +} from './parse'; + +/** + * @returns {import('../../conversion').ConversionFormat} + */ +export function getInlineChoiceConversionFormat() { + return { passage: { contentLevel: ContentLevel.PLAIN } }; +} + +/** + * Each dropdown becomes a blank, its correct option the blank's answer and the rest its own + * options. + * + * @param {import('./parse').InlineChoiceState} state + * @returns {import('../../conversion').ConvertibleContent} + */ +export function exportInlineChoiceContent(state) { + const body = parseHTMLBody(state.passage); + const blanks = [...body.querySelectorAll(DROPDOWN)].map((el, index) => { + const { options, correctId } = readDropdown(el); + const placeholder = body.ownerDocument.createElement('span'); + placeholder.setAttribute(BLANK_ATTR, String(index)); + el.replaceWith(placeholder); + + const correct = options.find(option => option.id === correctId); + return { + answer: correct ? correct.text : null, + options: options.filter(option => option !== correct).map(option => option.text), + }; + }); + return { + prompt: state.prompt, + passage: { + html: getContentHTML(body), + blanks, + distractors: [], + contentLevel: ContentLevel.PLAIN, + shuffle: state.shuffle, + }, + }; +} + +/** + * A dropdown offering everything a learner could have put in its blank: its answer, its own + * options, every other blank's answer and every distractor, as a gap match learner chooses + * from one shared pool. Its answer, when it has one, is its correct option. One with nothing + * to offer starts as an unfilled dropdown, with one empty option. + */ +function buildDropdown(doc, { answer, options }, answers, distractors) { + const texts = [ + ...new Set( + [answer, ...options, ...answers, ...distractors].map(text => text?.trim()).filter(Boolean), + ), + ]; + const dropdownOptions = (texts.length ? texts : ['']).map(text => newOption(text)); + return writeDropdown( + doc, + newDropdown({ + options: dropdownOptions, + // The answer leads the list, so it is the first option. + correctId: answer?.trim() ? dropdownOptions[0].id : null, + }), + ); +} + +/** + * @param {import('../../conversion').ConvertibleContent} content + * @returns {import('./parse').InlineChoiceState} + */ +export function importInlineChoiceContent({ prompt, passage }) { + const state = _defaultState(); + if (!passage) { + return { ...state, prompt }; + } + const body = parseHTMLBody(passage.html); + const answers = passage.blanks.map(({ answer }) => answer).filter(Boolean); + for (const placeholder of body.querySelectorAll(`[${BLANK_ATTR}]`)) { + const blank = passage.blanks[Number(placeholder.getAttribute(BLANK_ATTR))]; + placeholder.replaceWith( + blank ? buildDropdown(body.ownerDocument, blank, answers, passage.distractors) : '', + ); + } + return { prompt, passage: getContentHTML(body), shuffle: passage.shuffle ?? state.shuffle }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js index c1aade0468..2d0c092759 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js @@ -63,6 +63,47 @@ export function readDropdown(el) { }; } +/** + * A new option, with a fresh identifier. + * + * @param {string} [text] + * @returns {{ id: string, text: string }} + */ +export const newOption = (text = '') => ({ id: generateRandomSlug('choice'), text }); + +/** + * A new dropdown, with a fresh response identifier. + * + * @param {Partial} [fields] + * @returns {InlineChoiceDropdown} + */ +export const newDropdown = (fields = {}) => ({ + responseIdentifier: generateRandomSlug('response'), + options: [newOption()], + correctId: null, + ...fields, +}); + +/** + * A dropdown as the passage holds it. The reverse of readDropdown. + * + * @param {Document} doc - The document of the passage it goes in + * @param {InlineChoiceDropdown} dropdown + * @returns {Element} + */ +export function writeDropdown(doc, { responseIdentifier, options, correctId }) { + const el = doc.createElement(DROPDOWN); + el.setAttribute('response-identifier', responseIdentifier); + if (correctId !== null) el.setAttribute(CORRECT_ATTR, correctId); + for (const { id, text } of options) { + const option = doc.createElement(OPTION); + option.setAttribute('identifier', id); + option.textContent = text; + el.appendChild(option); + } + return el; +} + /** * A dropdown as it is copied, as a DOM output spec. An editor without dropdowns keeps only the * text of an element it doesn't know, so the text is the correct answer alone, or nothing with no diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Descriptor.js index 2318b037f0..9b2b7b638a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Descriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Descriptor.js @@ -1,6 +1,7 @@ import { QtiInteraction, QuestionType, BaseType, Cardinality } from '../../constants'; import { InteractionDescriptor } from '../InteractionDescriptor'; import { parseMatchInteraction, buildMatchInteractionXML, hasTwoMatchSets } from './parse'; +import { exportMatchContent, getMatchConversionFormat, importMatchContent } from './convert'; import { validateMatchInteraction } from './validation'; /** @@ -12,7 +13,6 @@ export class MatchInteractionDescriptor extends InteractionDescriptor { type: QtiInteraction.MATCH, questionTypes: [QuestionType.MATCH], }); - this.convertsFrom = []; } getTypeOptions(tr) { @@ -80,6 +80,29 @@ export class MatchInteractionDescriptor extends InteractionDescriptor { validate(state) { return validateMatchInteraction(state); } + + /** + * @returns {import('../../conversion').ConversionFormat} + */ + getConversionFormat() { + return getMatchConversionFormat(); + } + + /** + * @param {object} state - MatchState + * @returns {import('../../conversion').ConvertibleContent} + */ + exportContent(state) { + return exportMatchContent(state); + } + + /** + * @param {import('../../conversion').ConvertibleContent} content + * @returns {object} MatchState + */ + importContent(content) { + return importMatchContent(content); + } } /** Singleton — safe to import from any file in the match module tree. */ diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue index 7f3bf55681..2eba562bef 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue @@ -542,10 +542,7 @@ if (newMode !== 'edit') { openTarget.value = null; releaseErrors(); - return; } - // Open the question, the first thing in the card, so the card opens at its start. - openPrompt(); }, { immediate: true }, ); @@ -676,6 +673,9 @@ ]); return { + // Public: opens the question for editing, which focuses it. + // eslint-disable-next-line vue/no-unused-properties + focus: openPrompt, EditorImageProcessor, OpenTarget, windowIsSmall, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js index 1ebbe71224..e101813d60 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js @@ -1,9 +1,10 @@ -import { fireEvent, render, screen, within } from '@testing-library/vue'; +import { fireEvent, render, screen, waitFor, within } from '@testing-library/vue'; import userEvent from '@testing-library/user-event'; import { nextTick } from 'vue'; import VueRouter from 'vue-router'; import { themePalette, themeTokens } from 'kolibri-design-system/lib/styles/theme'; import MatchEditor from '../Editor.vue'; +import { renderFocused } from '../../__tests__/renderFocused'; import { MATCH_XML, @@ -144,8 +145,9 @@ const rowAnswers = number => const answerItems = number => within(rowAnswers(number)).queryAllByRole('listitem'); const distractorList = () => screen.getByRole('list', { name: tr.$tr('distractorsLabel') }); -const renderEditor = (props = {}) => - render(MatchEditor, { +// `focused` renders it as its card does when the card opens. +const renderEditor = (props = {}, { focused = false } = {}) => + (focused ? renderFocused : render)(MatchEditor, { props: { mode: 'edit', questionType: QuestionType.MATCH, @@ -194,10 +196,19 @@ describe('MatchEditor', () => { expect(screen.queryByText(tr.$tr('pairNumberLabel', { number: 1 }))).not.toBeInTheDocument(); }); - it('opens the question prompt for editing when it is already written', () => { - renderEditor(); + it('opens the question prompt for editing, and focuses it, when its card opens', async () => { + renderEditor({}, { focused: true }); + await nextTick(); expect(queryButton(tr.$tr('editQuestionLabel'))).not.toBeInTheDocument(); expect(button(editRow(1))).toBeInTheDocument(); + await waitFor(() => expect(openTextarea()).toHaveFocus()); + }); + + it('leaves the question prompt closed, and focus where it was, when it only mounts', async () => { + renderEditor(); + await nextTick(); + expect(button(tr.$tr('editQuestionLabel'))).toBeInTheDocument(); + expect(document.body).toHaveFocus(); }); it('shows a placeholder in a blank row prompt until it opens', async () => { @@ -209,8 +220,9 @@ describe('MatchEditor', () => { expect(within(rows()[0]).queryByText(placeholder)).not.toBeInTheDocument(); }); - it('opens the question prompt for editing when it is blank', () => { - renderEditor({ interaction: block('') }); + it('opens the question prompt for editing when its card opens and it is blank', async () => { + renderEditor({ interaction: block('') }, { focused: true }); + await nextTick(); expect(queryButton(tr.$tr('editQuestionLabel'))).not.toBeInTheDocument(); expect(button(editRow(1))).toBeInTheDocument(); }); @@ -235,8 +247,8 @@ describe('MatchEditor', () => { it('writes the typed question prompt into the emitted XML', async () => { const user = userEvent.setup(); - // The question prompt opens on mount. const { emitted } = renderEditor(); + await user.click(button(tr.$tr('editQuestionLabel'))); await user.type(openTextarea(), 'Which class?'); expect(latestBodyXml(emitted)).toContain('Which class?'); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/convert.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/convert.spec.js new file mode 100644 index 0000000000..fd0eb2a316 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/convert.spec.js @@ -0,0 +1,94 @@ +import { exportMatchContent, importMatchContent } from '../convert'; + +describe('match conversion', () => { + it('exports one pair per match, and a row with no match yet as a pair with nothing', () => { + const state = { + prompt: '

Q

', + rows: [ + { + id: 'row_a', + content: '

Bat

', + matches: [ + { id: 'choice_m', content: '

Mammal

' }, + { id: 'choice_f', content: '

Flyer

' }, + ], + }, + { id: 'row_b', content: '

Frog

', matches: [] }, + ], + distractors: [{ id: 'choice_r', content: '

Reptile

' }], + }; + expect(exportMatchContent(state)).toEqual({ + prompt: '

Q

', + pairs: { + items: [ + { source: '

Bat

', target: '

Mammal

' }, + { source: '

Bat

', target: '

Flyer

' }, + { source: '

Frog

', target: '' }, + ], + distractors: ['

Reptile

'], + }, + }); + }); + + it('imports each pair as a row of its own, with its target as the match', () => { + const state = importMatchContent({ + prompt: '

Q

', + pairs: { items: [{ source: '

A

', target: '

B

' }], distractors: ['

C

'] }, + }); + expect(state.rows).toEqual([ + { + id: expect.stringMatching(/^row_/), + content: '

A

', + matches: [{ id: expect.stringMatching(/^choice_/), content: '

B

' }], + }, + ]); + expect(state.distractors).toEqual([ + { id: expect.stringMatching(/^choice_/), content: '

C

' }, + ]); + }); + + it('gathers pairs from the same source into one row, matched to each target', () => { + const state = importMatchContent({ + prompt: '', + pairs: { + items: [ + { source: '

Bat

', target: '

Mammal

' }, + { source: '

Dog

', target: '

Mammal

' }, + { source: '

Bat

', target: '

Flyer

' }, + { source: '

Bat

', target: '

Flyer

' }, + ], + distractors: [], + }, + }); + expect( + state.rows.map(({ content, matches }) => [content, matches.map(m => m.content)]), + ).toEqual([ + ['

Bat

', ['

Mammal

', '

Flyer

']], + ['

Dog

', ['

Mammal

']], + ]); + }); + + it('keeps rows with no source apart, each with a match to fill in', () => { + const state = importMatchContent({ + prompt: '', + pairs: { + items: [ + { source: '', target: '' }, + { source: '

', target: '

B

' }, + ], + distractors: [], + }, + }); + expect(state.rows.map(({ matches }) => matches.map(m => m.content))).toEqual([ + [''], + ['

B

'], + ]); + }); + + it('starts with one blank row when there are no pairs', () => { + const state = importMatchContent({ prompt: '' }); + expect(state.rows).toHaveLength(1); + expect(state.rows[0].content).toBe(''); + expect(state.distractors).toEqual([]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/convert.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/convert.js new file mode 100644 index 0000000000..9dd48b1a21 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/convert.js @@ -0,0 +1,77 @@ +import flatMap from 'lodash/flatMap'; +import { newChoice } from '../associate/parse'; +import { _defaultState, newRow } from './parse'; +import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + +/** + * @returns {import('../../conversion').ConversionFormat} + */ +export function getMatchConversionFormat() { + return { pairs: true }; +} + +/** + * Each row gives one pair per match. A row with no match yet still gives one, with nothing + * to pair it with, so the row itself isn't lost. + * + * @param {import('./parse').MatchState} state + * @returns {import('../../conversion').ConvertibleContent} + */ +export function exportMatchContent(state) { + return { + prompt: state.prompt, + pairs: { + items: flatMap(state.rows, ({ content, matches }) => + matches.length + ? matches.map(match => ({ source: content, target: match.content })) + : [{ source: content, target: '' }], + ), + distractors: state.distractors.map(({ content }) => content), + }, + }; +} + +/** + * Pairs from the same source become one row, matched to each of their targets, so a row that + * went to associate as several pairs comes back as the row it was. A blank source is a row of + * its own, as two blank rows are two rows the author has yet to fill in. + * + * @param {Array<{ source: string, target: string }>} items + * @returns {import('./parse').MatchRow[]} + */ +function rowsFromPairs(items) { + const rows = []; + const rowByKey = new Map(); + for (const { source, target } of items) { + const key = richTextComparisonKey(source); + let row = key ? rowByKey.get(key) : null; + if (!row) { + row = newRow({ content: source, matches: [] }); + rows.push(row); + if (key) rowByKey.set(key, row); + } + const targetKey = richTextComparisonKey(target); + const repeated = row.matches.some(match => richTextComparisonKey(match.content) === targetKey); + if (targetKey && !repeated) row.matches.push(newChoice(target)); + } + // A row has a match to fill in, even before it has an answer. + for (const row of rows) { + if (!row.matches.length) row.matches.push(newChoice()); + } + return rows; +} + +/** + * @param {import('../../conversion').ConvertibleContent} content + * @returns {import('./parse').MatchState} + */ +export function importMatchContent({ prompt, pairs }) { + const state = _defaultState(); + const items = pairs?.items ?? []; + return { + ...state, + prompt, + rows: items.length ? rowsFromPairs(items) : state.rows, + distractors: (pairs?.distractors ?? []).map(content => newChoice(content)), + }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js index 4e81b80576..4b2575c40b 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js @@ -30,12 +30,16 @@ const serializer = new XMLSerializer(); */ /** + * A new row, with a fresh identifier and one match to fill in. + * + * @param {Partial} [fields] * @returns {MatchRow} */ -export const newRow = () => ({ +export const newRow = (fields = {}) => ({ id: generateRandomSlug('row'), content: '', matches: [newChoice()], + ...fields, }); /** diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Descriptor.js index 58bce294de..f16445c7ba 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Descriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Descriptor.js @@ -1,6 +1,11 @@ import { QtiInteraction, QuestionType, BaseType, Cardinality } from '../../constants'; import { InteractionDescriptor } from '../InteractionDescriptor'; import { parseOrderingInteraction, buildOrderingInteractionXML } from './parse'; +import { + exportOrderingContent, + getOrderingConversionFormat, + importOrderingContent, +} from './convert'; import { validateOrderingInteraction } from './validation'; /** @@ -12,7 +17,6 @@ export class OrderingInteractionDescriptor extends InteractionDescriptor { type: QtiInteraction.ORDER, questionTypes: [QuestionType.ORDERING], }); - this.convertsFrom = []; } getTypeOptions(tr) { @@ -75,6 +79,29 @@ export class OrderingInteractionDescriptor extends InteractionDescriptor { validate(state) { return validateOrderingInteraction(state); } + + /** + * @returns {import('../../conversion').ConversionFormat} + */ + getConversionFormat() { + return getOrderingConversionFormat(); + } + + /** + * @param {object} state - OrderingState + * @returns {import('../../conversion').ConvertibleContent} + */ + exportContent(state) { + return exportOrderingContent(state); + } + + /** + * @param {import('../../conversion').ConvertibleContent} content + * @returns {object} OrderingState + */ + importContent(content) { + return importOrderingContent(content); + } } /** Singleton — safe to import from any file in the ordering module tree. */ diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Editor.vue index 90c0092687..1e3a18e4d7 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/Editor.vue @@ -303,8 +303,6 @@ () => props.mode, newMode => { if (newMode === 'edit') { - // Open the question, the first thing in the card, so the card opens at its start. - openPrompt(); emit('update:interaction', workingInteraction.value); } else { isPromptOpen.value = false; @@ -402,6 +400,9 @@ } return { + // Public: opens the question for editing, which focuses it. + // eslint-disable-next-line vue/no-unused-properties + focus: openPrompt, EditorImageProcessor, state, isOnlyItem, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/Editor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/Editor.spec.js index 353aaf62e3..04c52b17a9 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/Editor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/Editor.spec.js @@ -1,9 +1,10 @@ -import { render, screen, fireEvent, within } from '@testing-library/vue'; +import { render, screen, fireEvent, within, waitFor } 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 OrderingEditor from '../Editor.vue'; +import { renderFocused } from '../../__tests__/renderFocused'; import { ORDERING_XML, @@ -43,8 +44,9 @@ const itemLabel = number => tr.$tr('orderingItemLabel', { number }); const moveUpName = number => dragTr.$tr('moveItemUpLabel', { item: itemLabel(number) }); const moveDownName = number => dragTr.$tr('moveItemDownLabel', { item: itemLabel(number) }); -const renderEditor = (props = {}) => - render(OrderingEditor, { +// `focused` renders it as its card does when the card opens. +const renderEditor = (props = {}, { focused = false } = {}) => + (focused ? renderFocused : render)(OrderingEditor, { props: { mode: 'edit', ...props }, routes: new VueRouter(), }); @@ -67,15 +69,30 @@ const openTextarea = () => screen.queryAllByRole('textbox').find(el => el.tagNam describe('OrderingEditor', () => { describe('edit mode rendering', () => { - it('opens the question for editing when it is already written', () => { - renderEditor({ - interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML), - questionType: QuestionType.ORDERING, - }); + it('opens the question for editing, and focuses it, when its card opens', async () => { + renderEditor( + { + interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML), + questionType: QuestionType.ORDERING, + }, + { focused: true }, + ); + await nextTick(); expect( screen.queryByRole('button', { name: tr.$tr('editQuestionLabel') }), ).not.toBeInTheDocument(); expect(openTextarea().value).toContain('Arrange the planets'); + await waitFor(() => expect(openTextarea()).toHaveFocus()); + }); + + it('leaves the question closed, and focus where it was, when it only mounts', async () => { + renderEditor({ + interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML), + questionType: QuestionType.ORDERING, + }); + await nextTick(); + expect(screen.getByRole('button', { name: tr.$tr('editQuestionLabel') })).toBeInTheDocument(); + expect(document.body).toHaveFocus(); }); it('renders the prompt text from the XML', () => { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/convert.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/convert.spec.js new file mode 100644 index 0000000000..438feb5d51 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/convert.spec.js @@ -0,0 +1,55 @@ +import { CorrectCriterion, ContentLevel } from '../../../constants'; +import { + exportOrderingContent, + getOrderingConversionFormat, + importOrderingContent, +} from '../convert'; + +describe('ordering conversion', () => { + it('takes rich ordered answers', () => { + expect(getOrderingConversionFormat()).toEqual({ + answers: { correctCriterion: CorrectCriterion.ORDERED, contentLevel: ContentLevel.RICH }, + }); + }); + + it('exports the items in their correct order and the layout, but not the shuffle setting', () => { + const state = { + prompt: '

Q

', + items: [ + { id: 'order_a', content: '

1

', fixed: true }, + { id: 'order_b', content: '

2

' }, + ], + shuffle: false, + orientation: 'horizontal', + }; + expect(exportOrderingContent(state)).toEqual({ + prompt: '

Q

', + answers: { + correctCriterion: CorrectCriterion.ORDERED, + contentLevel: ContentLevel.RICH, + items: [ + { content: '

1

', correct: false, fixed: true }, + { content: '

2

', correct: false, fixed: false }, + ], + orientation: 'horizontal', + }, + }); + }); + + it('imports each answer as a new item, and stays shuffled', () => { + const state = importOrderingContent({ + prompt: '

Q

', + answers: { items: [{ content: '

1

', correct: true }], shuffle: false }, + }); + expect(state.items).toEqual([ + { id: expect.stringMatching(/^order_/), content: '

1

', fixed: false }, + ]); + expect(state.shuffle).toBe(true); + }); + + it('starts with one blank item when there are no answers', () => { + expect(importOrderingContent({ prompt: '' }).items).toEqual([ + { id: expect.stringMatching(/^order_/), content: '' }, + ]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/convert.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/convert.js new file mode 100644 index 0000000000..5f07c43c06 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/convert.js @@ -0,0 +1,53 @@ +import { CorrectCriterion, ContentLevel } from '../../constants'; +import { _defaultState, newItem } from './parse'; + +/** + * Ordering's shuffle is neither given nor taken: authors can't change it, and an ordering + * question that isn't shuffled shows its items in the correct order. + */ + +/** + * @returns {import('../../conversion').ConversionFormat} + */ +export function getOrderingConversionFormat() { + return { + answers: { correctCriterion: CorrectCriterion.ORDERED, contentLevel: ContentLevel.RICH }, + }; +} + +/** + * @param {import('./parse').OrderingState} state + * @returns {import('../../conversion').ConvertibleContent} + */ +export function exportOrderingContent(state) { + return { + prompt: state.prompt, + answers: { + correctCriterion: CorrectCriterion.ORDERED, + contentLevel: ContentLevel.RICH, + items: state.items.map(({ content, fixed }) => ({ + content, + correct: false, + fixed: Boolean(fixed), + })), + orientation: state.orientation, + }, + }; +} + +/** + * @param {import('../../conversion').ConvertibleContent} content + * @returns {import('./parse').OrderingState} + */ +export function importOrderingContent({ prompt, answers }) { + const state = _defaultState(); + const items = answers?.items ?? []; + return { + ...state, + prompt, + items: items.length + ? items.map(({ content, fixed }) => newItem({ content, fixed: Boolean(fixed) })) + : state.items, + orientation: answers?.orientation ?? state.orientation, + }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/parse.js index d3b9203e97..ac3470cbba 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/parse.js @@ -24,11 +24,23 @@ import { Orientation, RESPONSE_IDENTIFIER } from '../../constants'; const serializer = new XMLSerializer(); +/** + * A new item, with a fresh identifier. + * + * @param {Partial} [fields] + * @returns {OrderingItem} + */ +export const newItem = (fields = {}) => ({ + id: generateRandomSlug('order'), + content: '', + ...fields, +}); + export function _defaultState() { return { responseIdentifier: RESPONSE_IDENTIFIER, prompt: '', - items: [{ id: generateRandomSlug('order'), content: '' }], + items: [newItem()], orientation: Orientation.VERTICAL, shuffle: true, }; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js index 1529e211e8..11d7c183b0 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js @@ -6,6 +6,11 @@ import { buildTextEntryInteractionXML, isSupportedTextEntryBody, } from './parse'; +import { + exportTextEntryContent, + getTextEntryConversionFormat, + importTextEntryContent, +} from './convert'; import { validateTextEntryInteraction } from './validation'; /** @@ -21,7 +26,6 @@ class TextEntryInteractionDescriptor extends InteractionDescriptor { questionTypes: [QuestionType.NUMERIC, QuestionType.TEXT_ENTRY, QuestionType.FREE_RESPONSE], placement: Placement.INLINE, }); - this.convertsFrom = []; } getTypeOptions(tr) { @@ -127,6 +131,33 @@ class TextEntryInteractionDescriptor extends InteractionDescriptor { validate(state, questionType, options) { return validateTextEntryInteraction(state, questionType, options); } + + /** + * @param {string} questionType + * @returns {import('../../conversion').ConversionFormat} + */ + getConversionFormat(questionType) { + return getTextEntryConversionFormat(questionType); + } + + /** + * @param {object} state - TextEntryState + * @param {string} questionType + * @param {{ language?: string }} [options] + * @returns {import('../../conversion').ConvertibleContent} + */ + exportContent(state, questionType, options) { + return exportTextEntryContent(state, questionType, options); + } + + /** + * @param {import('../../conversion').ConvertibleContent} content + * @param {string} questionType + * @returns {object} TextEntryState + */ + importContent(content, questionType) { + return importTextEntryContent(content, questionType); + } } /** Singleton — safe to import from any file in the textEntry module tree. */ diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Editor.vue index 457909ab11..7ba2b8d984 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Editor.vue @@ -365,8 +365,6 @@ isPromptOpen.value = false; return; } - // Open the question, the first thing in the card, so the card opens at its start. - openPrompt(); showStoredAnswerValue(); // Opening a card does not remount it, so report answers stored in another form here. if ( @@ -385,6 +383,9 @@ watch(errors, newVal => emit('update:errors', newVal), { immediate: true }); return { + // Public: opens the question for editing, which focuses it. + // eslint-disable-next-line vue/no-unused-properties + focus: openPrompt, state, windowIsSmall, isNumeric, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/Editor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/Editor.spec.js index 103ddfe9fe..4a7be88c97 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/Editor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/Editor.spec.js @@ -1,7 +1,8 @@ -import { render, screen, fireEvent } from '@testing-library/vue'; +import { render, screen, fireEvent, waitFor } from '@testing-library/vue'; import { nextTick } from 'vue'; import VueRouter from 'vue-router'; import TextEntryEditor from '../Editor.vue'; +import { renderFocused } from '../../__tests__/renderFocused'; import { TEXT_ENTRY_BODY_XML, @@ -23,8 +24,9 @@ jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => { }; }); -const renderEditor = (props = {}) => - render(TextEntryEditor, { +// `focused` renders it as its card does when the card opens. +const renderEditor = (props = {}, { focused = false } = {}) => + (focused ? renderFocused : render)(TextEntryEditor, { props: { mode: 'edit', ...props }, routes: new VueRouter(), }); @@ -36,15 +38,30 @@ const answerInputs = () => screen.queryAllByRole('textbox', { name: tr.$tr('answerValuePlaceholder') }); describe('TextEntryEditor — numeric', () => { - it('opens the question for editing when it is already written', () => { - renderEditor({ - interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, NUMERIC_DECL), - questionType: QuestionType.NUMERIC, - }); + it('opens the question for editing, and focuses it, when its card opens', async () => { + renderEditor( + { + interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, NUMERIC_DECL), + questionType: QuestionType.NUMERIC, + }, + { focused: true }, + ); + await nextTick(); expect( screen.queryByRole('button', { name: tr.$tr('editQuestionLabel') }), ).not.toBeInTheDocument(); expect(openTextarea().value).toContain('What is H2O?'); + await waitFor(() => expect(openTextarea()).toHaveFocus()); + }); + + it('leaves the question closed, and focus where it was, when it only mounts', async () => { + renderEditor({ + interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, NUMERIC_DECL), + questionType: QuestionType.NUMERIC, + }); + await nextTick(); + expect(screen.getByRole('button', { name: tr.$tr('editQuestionLabel') })).toBeInTheDocument(); + expect(document.body).toHaveFocus(); }); describe('answer list', () => { @@ -424,51 +441,6 @@ describe('TextEntryEditor — emits', () => { }, ); - it.each([ - ['fr', '1234.5', '1234.5'], - ['en', '1.50', '1.5'], - ['en', '0012', '12'], - ['ar-EG', '30', '30'], - ['ar-EG', '-3.14', '-3.14'], - ['fr', '1.2.3', '1.2.3'], - ['he', '-0.5', '-0.5'], - ['sv', '-0.5', '-0.5'], - ['ar-EG', '-0.5', '-0.5'], - ])( - 'keeps a stored numeric answer as stored when switched to text entry in %s (%s)', - async (language, stored, expected) => { - const { emitted, updateProps } = renderEditor({ - interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, NUMERIC_DECL.replace('42', stored)), - questionType: QuestionType.NUMERIC, - language, - }); - await updateProps({ questionType: QuestionType.TEXT_ENTRY }); - - const [latest] = emitted()['update:interaction'].at(-1); - expect(latest.responseDeclarations[0]).toContain(`${expected}`); - }, - ); - - it.each([ - ['fr', '1,5', '1,5', '1.5'], - ['fr', '1 234,5', '1234,5', '1234.5'], - ])( - 'reads a text entry answer in %s (%s) when switched to numeric', - async (language, text, shown, stored) => { - const { emitted, updateProps } = renderEditor({ - interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, STRING_DECL.replace('H2O', text)), - questionType: QuestionType.TEXT_ENTRY, - language, - }); - await updateProps({ questionType: QuestionType.NUMERIC }); - - expect(answerInputs()[0]).toHaveValue(shown); - expect(screen.queryByRole('alert')).not.toBeInTheDocument(); - const [latest] = emitted()['update:interaction'].at(-1); - expect(latest.responseDeclarations[0]).toContain(`${stored}`); - }, - ); - it('leaves a text entry answer as stored in any language', () => { renderEditor({ interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, STRING_DECL.replace('H2O', '1.5')), @@ -480,53 +452,6 @@ describe('TextEntryEditor — emits', () => { ); }); - it.each([ - ['de', '1.5', '1,5'], - ['es', '1234.5', '1234,5'], - ['fr', '1234.5', '1234,5'], - ['ar-EG', '30', '٣٠'], - ])( - 'shows a stored answer in %s again after switching to text entry and back (%s)', - async (language, stored, shown) => { - const { emitted, updateProps } = renderEditor({ - interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, NUMERIC_DECL.replace('42', stored)), - questionType: QuestionType.NUMERIC, - language, - }); - await updateProps({ questionType: QuestionType.TEXT_ENTRY }); - await updateProps({ questionType: QuestionType.NUMERIC }); - - expect(answerInputs()[0]).toHaveValue(shown); - expect(screen.queryByRole('alert')).not.toBeInTheDocument(); - const [latest] = emitted()['update:interaction'].at(-1); - expect(latest.responseDeclarations[0]).toContain(`${stored}`); - }, - ); - - it.each([ - ['fr', '1,5', '1.5'], - ['en', '1,234', '1234'], - ['de', '1.500', '1500'], - ['', ' 5 ', '5'], - ['fa', 'می\u200cخواهم'], - ['en', 'a­b'], - ])( - 'keeps a typed numeric answer as stored when switched to text entry in "%s" (%s)', - async (language, typed, expected = typed) => { - const { emitted, updateProps } = renderEditor({ - interaction: blockWithDecl(TEXT_ENTRY_BODY_XML, NUMERIC_DECL), - questionType: QuestionType.NUMERIC, - language, - }); - const [input] = answerInputs(); - await fireEvent.input(input, { target: { value: typed } }); - await updateProps({ questionType: QuestionType.TEXT_ENTRY }); - - const [latest] = emitted()['update:interaction'].at(-1); - expect(latest.responseDeclarations[0]).toContain(`${expected}`); - }, - ); - it.each([ ['', 'fr', '1,5', '1,5', '1.5'], ['fr', 'en', '1,234.5', '1234.5', '1234.5'], diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/convert.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/convert.spec.js new file mode 100644 index 0000000000..98cbfa62ba --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/convert.spec.js @@ -0,0 +1,112 @@ +import { CorrectCriterion, ContentLevel, QuestionType } from '../../../constants'; +import { + exportTextEntryContent, + getTextEntryConversionFormat, + importTextEntryContent, +} from '../convert'; +import { DEFAULT_EXPECTED_LENGTH } from '../parse'; + +const state = { + prompt: '

Q

', + answers: [{ id: 'answer_a', value: 'Paris', caseSensitive: true }], + expectedLength: 80, +}; + +describe('text entry conversion', () => { + it.each([QuestionType.NUMERIC, QuestionType.TEXT_ENTRY])( + 'takes plain accepted answers for %s', + questionType => { + expect(getTextEntryConversionFormat(questionType)).toEqual({ + answers: { correctCriterion: CorrectCriterion.ACCEPTED, contentLevel: ContentLevel.PLAIN }, + }); + }, + ); + + it('takes no answers for a free response', () => { + expect(getTextEntryConversionFormat(QuestionType.FREE_RESPONSE)).toEqual({}); + }); + + it('exports each answer as accepted, with its case sensitivity, and the expected length', () => { + expect(exportTextEntryContent(state, QuestionType.TEXT_ENTRY)).toEqual({ + prompt: '

Q

', + answers: { + correctCriterion: CorrectCriterion.ACCEPTED, + contentLevel: ContentLevel.PLAIN, + items: [{ content: 'Paris', correct: true, caseSensitive: true }], + expectedLength: 80, + }, + }); + }); + + it('exports no answers from a free response, which saves none, but its expected length', () => { + expect(exportTextEntryContent(state, QuestionType.FREE_RESPONSE).answers).toEqual({ + correctCriterion: CorrectCriterion.ACCEPTED, + contentLevel: ContentLevel.PLAIN, + items: [], + expectedLength: 80, + }); + }); + + describe('numeric answers', () => { + const numeric = values => ({ + prompt: '', + answers: values.map((value, i) => ({ id: `answer_${i}`, value, caseSensitive: false })), + expectedLength: 80, + }); + const exported = (values, options) => + exportTextEntryContent(numeric(values), QuestionType.NUMERIC, options).answers.items.map( + ({ content }) => content, + ); + + it('exports them as the exercise language shows them', () => { + expect(exported(['1.5', '1234.5'], { language: 'de' })).toEqual(['1,5', '1234,5']); + expect(exported(['1.5'], { language: 'en' })).toEqual(['1.5']); + }); + + it('exports them as stored when the exercise has no language', () => { + expect(exported(['1.5', '1234.5'])).toEqual(['1.5', '1234.5']); + }); + + it('exports an answer that is not a number as it was written', () => { + expect(exported(['abc'], { language: 'de' })).toEqual(['abc']); + }); + }); + + it('exports text entry answers as written, whatever the exercise language', () => { + const decimal = { ...state, answers: [{ id: 'answer_a', value: '1.5', caseSensitive: false }] }; + const { items } = exportTextEntryContent(decimal, QuestionType.TEXT_ENTRY, { + language: 'de', + }).answers; + expect(items.map(({ content }) => content)).toEqual(['1.5']); + }); + + it('imports the expected length that came with the answers', () => { + const content = { prompt: '', answers: { items: [], expectedLength: 10 } }; + expect(importTextEntryContent(content, QuestionType.TEXT_ENTRY).expectedLength).toBe(10); + }); + + it('imports each answer as a new one, with the default expected length otherwise', () => { + const content = { + prompt: '

Q

', + answers: { items: [{ content: 'Paris', correct: true, caseSensitive: true }] }, + }; + expect(importTextEntryContent(content, QuestionType.TEXT_ENTRY)).toEqual({ + prompt: '

Q

', + answers: [{ id: expect.stringMatching(/^answer_/), value: 'Paris', caseSensitive: true }], + expectedLength: DEFAULT_EXPECTED_LENGTH, + }); + }); + + it('makes no numeric answer case-sensitive', () => { + const content = { prompt: '', answers: { items: [{ content: '1', caseSensitive: true }] } }; + expect(importTextEntryContent(content, QuestionType.NUMERIC).answers[0].caseSensitive).toBe( + false, + ); + }); + + it('starts with one blank answer when there are none', () => { + expect(importTextEntryContent({ prompt: '' }, QuestionType.TEXT_ENTRY).answers).toEqual([ + { id: expect.stringMatching(/^answer_/), value: '', caseSensitive: false }, + ]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/convert.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/convert.js new file mode 100644 index 0000000000..48cb2b18a0 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/convert.js @@ -0,0 +1,72 @@ +import { CorrectCriterion, ContentLevel, QuestionType } from '../../constants'; +import { formatLocaleNumber } from '../../utils/localeNumbers'; +import { _defaultState, newAnswer } from './parse'; + +// A free response has no answers to give or take. +const takesAnswers = questionType => questionType !== QuestionType.FREE_RESPONSE; + +/** + * @param {string} questionType + * @returns {import('../../conversion').ConversionFormat} + */ +export function getTextEntryConversionFormat(questionType) { + if (!takesAnswers(questionType)) { + return {}; + } + return { + answers: { correctCriterion: CorrectCriterion.ACCEPTED, contentLevel: ContentLevel.PLAIN }, + }; +} + +/** + * @param {import('./parse').TextEntryState} state + * @param {string} questionType + * @param {{ language?: string }} [options] - The exercise language + * @returns {import('../../conversion').ConvertibleContent} + */ +export function exportTextEntryContent(state, questionType, { language } = {}) { + // Numeric answers are stored as xsd:double but shown in the exercise language; they leave as + // the author saw them, which is also how learners of that language write them. + const shown = value => + questionType === QuestionType.NUMERIC ? formatLocaleNumber(value, language) : value; + return { + prompt: state.prompt, + answers: { + correctCriterion: CorrectCriterion.ACCEPTED, + contentLevel: ContentLevel.PLAIN, + // A free response saves no answers, so it has none to carry, whatever its state holds. + items: takesAnswers(questionType) + ? state.answers.map(({ value, caseSensitive }) => ({ + content: shown(value), + correct: true, + caseSensitive, + })) + : [], + expectedLength: state.expectedLength, + }, + }; +} + +/** + * @param {import('../../conversion').ConvertibleContent} content + * @param {string} questionType + * @returns {import('./parse').TextEntryState} + */ +export function importTextEntryContent({ prompt, answers }, questionType) { + const state = _defaultState(); + const items = answers?.items ?? []; + return { + ...state, + prompt, + answers: items.length + ? items.map(({ content, caseSensitive }) => + newAnswer({ + value: content, + // Numeric answers are never case-sensitive. + caseSensitive: questionType === QuestionType.TEXT_ENTRY && Boolean(caseSensitive), + }), + ) + : state.answers, + expectedLength: answers?.expectedLength ?? state.expectedLength, + }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js index faf5116a3e..d1edbc7829 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js @@ -40,6 +40,19 @@ const serializer = new XMLSerializer(); */ export const DEFAULT_EXPECTED_LENGTH = 50; +/** + * A new answer, with a fresh identifier. + * + * @param {Partial} [fields] + * @returns {TextEntryAnswer} + */ +export const newAnswer = (fields = {}) => ({ + id: generateRandomSlug('answer'), + value: '', + caseSensitive: false, + ...fields, +}); + /** * Default state — used when bodyXml is absent or unparseable. * @@ -48,7 +61,7 @@ export const DEFAULT_EXPECTED_LENGTH = 50; export function _defaultState() { return { prompt: '', - answers: [{ id: generateRandomSlug('answer'), value: '', caseSensitive: false }], + answers: [newAnswer()], expectedLength: DEFAULT_EXPECTED_LENGTH, }; } @@ -145,12 +158,7 @@ function extractNumericAnswers(declarationEl) { const mapKeys = [...declarationEl.querySelectorAll(':scope > qti-mapping qti-map-entry')] .filter(entry => parseFloat(entry.getAttribute('mapped-value')) >= 1) .map(entry => (entry.getAttribute('map-key') ?? '').trim()); - const toAnswers = answerValues => - answerValues.map(value => ({ - id: generateRandomSlug('answer'), - value, - caseSensitive: false, - })); + const toAnswers = answerValues => answerValues.map(value => newAnswer({ value })); // The editor writes its first answer as the correct response and maps every answer, // so its map-keys are the answers as authored, repeats included. if (values.length === 1 && values[0] === mapKeys[0]) { @@ -202,11 +210,7 @@ export function _extractAnswers(responseDeclarations) { return []; } - const toAnswer = (value, caseSensitive) => ({ - id: generateRandomSlug('answer'), - value, - caseSensitive, - }); + const toAnswer = (value, caseSensitive) => newAnswer({ value, caseSensitive }); const mapEntries = declaration.mapping?.entries ?? []; // Key on the XML string form: both map-key and correct-response values are coerced // on parse (empty → null under QTI NULL semantics), so formatting both back matches diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js index 23bece2e84..6871a2e79d 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js @@ -643,4 +643,68 @@ export const qtiEditorStrings = createTranslator('QTIEditorStrings', { message: 'Insert at least one inline choice', context: 'Validation error when the passage of an inline choice question has no dropdown', }, + changeQuestionTypeTitle: { + message: 'Change question type?', + context: + 'Title of the dialog asking an author to confirm a change of question type that would remove or change some of the question', + }, + changeQuestionTypeIntro: { + message: 'Switching to {type} will change this question:', + context: + "Opens the list of what changing the question's type would remove or change. {type} is the new question type, e.g. 'Numeric input'", + }, + changeQuestionTypeAction: { + message: 'Change', + context: 'Button that confirms changing the question type', + }, + cancelAction: { + message: 'Cancel', + context: 'Button that closes a dialog without doing anything', + }, + answersRemovedLoss: { + message: 'All answers will be removed.', + context: + 'Listed when changing the question type: the new type has no place for the answers the question has', + }, + incorrectAnswersRemovedLoss: { + message: 'Answers not marked as correct will be removed.', + context: + 'Listed when changing a choice question to a type whose answers are all correct, such as numeric input or text entry', + }, + anyCorrectAnswerAcceptedLoss: { + message: 'Learners will only need to give one of the correct answers.', + context: + 'Listed when changing a choice question with several correct answers to numeric input or text entry, where any one accepted answer is enough', + }, + correctAnswersReducedLoss: { + message: 'Only the first correct answer will stay marked as correct.', + context: + 'Listed when changing a question with several correct answers to single choice, which can only have one', + }, + correctAnswersNotKeptLoss: { + message: 'Answers will no longer be marked as correct.', + context: + 'Listed when changing a choice question to ordering, where an answer is not correct or incorrect by itself', + }, + orderBecomesAnswerLoss: { + message: 'The current order will become the correct order.', + context: 'Listed when changing a question to ordering: the answers keep their current order', + }, + orderNotKeptLoss: { + message: "The correct order won't be kept.", + context: + 'Listed when changing an ordering question to a choice question, which has no correct order', + }, + formattingRemovedLoss: { + message: + 'Formatting, images and formulas in answers will be removed. Answers with only an image or formula will be removed entirely.', + context: + 'Listed when changing the question type to one whose answers can only hold text, such as text entry', + }, + passageAddedToQuestionLoss: { + message: + 'The passage will be added to the question, with each blank shown as "{blank}". Its answers will be removed.', + context: + 'Listed when changing an inline choice question to a type with no passage. {blank} is what each blank becomes, a row of underscores', + }, }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js index 61a64cca33..a5e0676571 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js @@ -66,6 +66,18 @@ export function getContentHTML(el) { return serializeAsHtml([...el.childNodes], el.namespaceURI); } +/** + * An HTML fragment, as a rich text editor writes it, parsed into the `` of a new + * document. As HTML, not XML: the fragment can hold `
`, ` ` and other markup an XML + * parser rejects. Read it back with getContentHTML. + * + * @param {string} html + * @returns {HTMLElement} + */ +export function parseHTMLBody(html) { + return parseXML(`${html ?? ''}`, 'text/html').body; +} + const xmlDoc = parser.parseFromString('', 'text/xml'); const XHTML_NS = 'http://www.w3.org/1999/xhtml'; const DROPPED_ELEMENTS = new Set(['noscript', 'script', 'style']); diff --git a/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditor.vue b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditor.vue index 60f5cd386b..48489c5cbd 100644 --- a/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditor.vue @@ -84,7 +84,16 @@