Skip to content

Commit 942cf81

Browse files
refactor: consolidate QTI interaction state management and improve text-entry parsing robustness
Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>
1 parent f666670 commit 942cf81

18 files changed

Lines changed: 341 additions & 305 deletions

File tree

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<template>
2+
3+
<KButton
4+
appearance="flat-button"
5+
:appearanceOverrides="buttonAppearanceOverrides"
6+
class="add-list-item-btn"
7+
@click="$emit('click')"
8+
>
9+
<div class="add-list-item-btn-content">
10+
<KIcon
11+
icon="plus"
12+
:color="$themePalette.blue.v_500"
13+
/>
14+
<span>{{ label }}</span>
15+
</div>
16+
</KButton>
17+
18+
</template>
19+
20+
21+
<script>
22+
23+
import { computed } from 'vue';
24+
import { themePalette } from 'kolibri-design-system/lib/styles/theme';
25+
26+
export default {
27+
name: 'AddListItemButton',
28+
setup() {
29+
const palette = themePalette();
30+
const buttonAppearanceOverrides = computed(() => ({
31+
backgroundColor: palette.blue.v_50,
32+
border: `1px dashed ${palette.blue.v_200}`,
33+
color: `${palette.blue.v_500} !important`,
34+
fontSize: '14px',
35+
fontWeight: '600',
36+
textTransform: 'none',
37+
':hover': {
38+
backgroundColor: palette.blue.v_100,
39+
},
40+
}));
41+
42+
return {
43+
buttonAppearanceOverrides,
44+
};
45+
},
46+
props: {
47+
label: {
48+
type: String,
49+
required: true,
50+
},
51+
},
52+
emits: ['click'],
53+
};
54+
55+
</script>
56+
57+
58+
<style scoped>
59+
60+
.add-list-item-btn {
61+
justify-content: center;
62+
width: 100%;
63+
padding: 11px 16px !important;
64+
margin-top: 10px;
65+
line-height: unset !important;
66+
border-radius: 4px !important;
67+
}
68+
69+
.add-list-item-btn-content {
70+
display: flex;
71+
gap: 10px;
72+
align-items: center;
73+
justify-content: center;
74+
}
75+
76+
</style>

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteractionDescriptor.spec.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,12 @@ describe('useInteractionDescriptor', () => {
109109
});
110110

111111
describe('with malformed XML', () => {
112-
it('returns null parseError (text/html parser recovers silently from malformed input)', async () => {
113-
// inferFromXml now uses text/html which never throws — malformed fragments
114-
// are recovered gracefully by the browser HTML parser, so parseError stays null.
112+
it('returns a parse error for malformed XML', async () => {
113+
// inferFromXml uses text/xml which throws a parser error for malformed fragments.
115114
const { result } = renderDescriptor('<unclosed');
116115
await nextTick();
117-
expect(result.parseError.value).toBeNull();
116+
expect(typeof result.parseError.value).toBe('string');
117+
expect(result.parseError.value).toMatch(/parse error/);
118118
});
119119

120120
it('still returns a defined fallback descriptor on parse error', async () => {

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useInteractionDescriptor.js

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,8 @@ export default function useInteractionDescriptor(interactionRef) {
2424
return { descriptor: registry[DEFAULT_INTERACTION], questionType: null, error: null };
2525
}
2626
try {
27-
// Parse as text/html — the serialized bodyXml carries an xmlns attribute
28-
// (e.g. xmlns="http://www.imsglobal.org/xsd/imsqtiasi_v3p0") that causes
29-
// CSS querySelector to silently fail in Chrome/Firefox when using the
30-
// strict text/xml parser. text/html strips namespaces and always works.
31-
const doc = parseXML(xml, 'text/html');
32-
// In html mode the document is: <html><head/><body>…content…</body></html>
33-
// For block interactions the content IS the interaction element itself;
34-
// for inline interactions it is the <qti-item-body> wrapper.
35-
const root = doc.body.firstElementChild ?? doc.body;
36-
const interactionEl =
37-
descriptors.reduce((found, d) => {
38-
if (found) return found;
39-
// Try root itself first, then search descendants.
40-
if (d.matches(root)) return root;
41-
return root.querySelector(d.type) ?? null;
42-
}, null) ?? root;
43-
27+
const doc = parseXML(xml);
28+
const interactionEl = doc.documentElement;
4429
const desc = descriptors.find(d => d.matches(interactionEl)) ?? registry[DEFAULT_INTERACTION];
4530
return {
4631
descriptor: desc,

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,15 @@ export const ValidationError = Object.freeze({
9595
DUPLICATE_CHOICE_CONTENT: 'DUPLICATE_CHOICE_CONTENT',
9696
INVALID_NUMERIC_VALUE: 'INVALID_NUMERIC_VALUE',
9797
EMPTY_ANSWER_CONTENT: 'EMPTY_ANSWER_CONTENT',
98+
DUPLICATE_ANSWER_CONTENT: 'DUPLICATE_ANSWER_CONTENT',
9899
});
100+
101+
export const RESPONSE_IDENTIFIER = 'RESPONSE';
102+
103+
/**
104+
* Set of QTI interaction tag names that have `placement: 'inline'`.
105+
* Used by parseItem to decide whether to serialize the full `<qti-item-body>`
106+
* (inline) or just the interaction element (block).
107+
* Kept here to avoid a circular dependency with the descriptor registry.
108+
*/
109+
export const INLINE_INTERACTION_TAGS = new Set([QtiInteraction.TEXT_ENTRY]);

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionDescriptor.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { QtiInteraction, QuestionType, BaseType, Cardinality } from '../../constants';
2+
import { parseXML } from '../../serialization/parseItem';
23
import { parseChoiceInteraction, buildChoiceInteractionXML } from './parse';
34
import { validateChoiceInteraction } from './validation';
45

@@ -28,7 +29,7 @@ export class ChoiceInteractionDescriptor {
2829
*/
2930
getQuestionType(el, responseDeclarations = []) {
3031
if (responseDeclarations.length > 0) {
31-
const doc = new DOMParser().parseFromString(responseDeclarations[0], 'text/xml');
32+
const doc = parseXML(responseDeclarations[0]);
3233
const cardinality = doc.documentElement.getAttribute('cardinality');
3334
if (cardinality) {
3435
return cardinality === Cardinality.MULTIPLE

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -169,22 +169,12 @@
169169
</div>
170170

171171
<!-- Add choice button (edit only) -->
172-
<KButton
172+
<AddListItemButton
173173
v-if="mode === 'edit'"
174-
appearance="flat-button"
175-
:appearanceOverrides="buttonAppearanceOverrides"
176-
class="choice-editor-button"
174+
:label="addChoiceBtn$()"
177175
:aria-label="addChoiceBtn$()"
178176
@click="onAddChoice"
179-
>
180-
<div class="add-choice-btn-content">
181-
<KIcon
182-
icon="plus"
183-
:color="$themePalette.blue.v_500"
184-
/>
185-
<span>{{ addChoiceBtn$() }}</span>
186-
</div>
187-
</KButton>
177+
/>
188178
</div>
189179
</div>
190180

@@ -201,13 +191,14 @@
201191
import { useChoiceInteraction } from '../../composables/useChoiceInteraction';
202192
import CollapsibleToolbar from '../../components/CollapsibleToolbar/index.vue';
203193
import ValidationMessage from '../../components/ValidationMessage/index.vue';
194+
import AddListItemButton from '../../components/AddListItemButton/index.vue';
204195
import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor';
205196
import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService';
206197
207198
export default {
208199
name: 'ChoiceInteractionEditor',
209200
210-
components: { TipTapEditor, CollapsibleToolbar, ValidationMessage },
201+
components: { TipTapEditor, CollapsibleToolbar, ValidationMessage, AddListItemButton },
211202
212203
setup(props, { emit }) {
213204
const { windowIsSmall } = useKResponsiveWindow();
@@ -231,17 +222,6 @@
231222
232223
const palette = themePalette();
233224
const tokens = themeTokens();
234-
const buttonAppearanceOverrides = computed(() => ({
235-
backgroundColor: palette.blue.v_50,
236-
border: `1px dashed ${palette.blue.v_200}`,
237-
color: `${palette.blue.v_500} !important`,
238-
fontSize: '14px',
239-
fontWeight: '600',
240-
textTransform: 'none',
241-
':hover': {
242-
backgroundColor: palette.blue.v_100,
243-
},
244-
}));
245225
246226
// questionType prop is not a Ref — wrap it so useChoiceInteraction can react to changes.
247227
const questionTypeRef = computed(() => props.questionType);
@@ -511,7 +491,6 @@
511491
errorEmptyChoiceContent$,
512492
errorDuplicateChoiceContent$,
513493
questionLabel$,
514-
buttonAppearanceOverrides,
515494
};
516495
},
517496
@@ -693,20 +672,4 @@
693672
cursor: pointer;
694673
}
695674
696-
.choice-editor-button {
697-
justify-content: center;
698-
width: 100%;
699-
padding: 11px 16px !important;
700-
margin-top: 10px;
701-
line-height: unset !important;
702-
border-radius: 4px !important;
703-
}
704-
705-
.add-choice-btn-content {
706-
display: flex;
707-
gap: 10px;
708-
align-items: center;
709-
justify-content: center;
710-
}
711-
712675
</style>

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/index.js

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,4 @@ import defineInteraction from '../defineInteraction';
22
import ChoiceInteractionEditor from './ChoiceInteractionEditor.vue';
33
import { choiceInteractionDescriptor } from './ChoiceInteractionDescriptor';
44

5-
/**
6-
* @typedef {object} ChoiceAnswer
7-
* @property {string} id - QTI identifier, e.g. "choice_xlqTuVoq"
8-
* @property {string} content - HTML content of the <qti-simple-choice>
9-
* @property {boolean} correct - Whether this choice is in the correct response
10-
* @property {boolean} fixed - Whether this choice is fixed (round-trip only)
11-
*/
12-
13-
/**
14-
* @typedef {object} ChoiceState
15-
* @property {string} prompt - HTML content of <qti-prompt>; default ""
16-
* @property {ChoiceAnswer[]} answers
17-
* @property {number} maxChoices - From max-choices attribute (0 = unlimited)
18-
* @property {number} minChoices - From min-choices attribute; default 0
19-
* @property {boolean} shuffle - From shuffle attribute; default false
20-
* @property {string} orientation - From orientation attribute; default "vertical"
21-
*/
22-
235
export default defineInteraction(choiceInteractionDescriptor, ChoiceInteractionEditor);

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/parse.js

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,33 @@
11
import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration';
2-
import { parseXML, getPromptHTML } from '../../serialization/parseItem';
2+
import { getPromptHTML, parseXML } from '../../serialization/parseItem';
33
import { buildXmlNode } from '../../serialization/assembleItem';
44
import CorrectResponse from '../../serialization/qti/declarations/correctResponse';
55
import { generateRandomSlug } from '../../utils/generateRandomSlug';
6-
import { Orientation } from '../../constants';
6+
import { Orientation, RESPONSE_IDENTIFIER } from '../../constants';
7+
8+
/**
9+
* @typedef {object} ChoiceAnswer
10+
* @property {string} id - QTI identifier, e.g. "choice_xlqTuVoq"
11+
* @property {string} content - HTML content of the <qti-simple-choice>
12+
* @property {boolean} correct - Whether this choice is in the correct response
13+
* @property {boolean} fixed - Whether this choice is fixed (round-trip only)
14+
*/
15+
16+
/**
17+
* @typedef {object} ChoiceState
18+
* @property {string} prompt - HTML content of <qti-prompt>; default ""
19+
* @property {ChoiceAnswer[]} answers
20+
* @property {number} maxChoices - From max-choices attribute (0 = unlimited)
21+
* @property {number} minChoices - From min-choices attribute; default 0
22+
* @property {boolean} shuffle - From shuffle attribute; default false
23+
* @property {string} orientation - From orientation attribute; default "vertical"
24+
*/
725

826
const serializer = new XMLSerializer();
927

1028
export function _defaultState() {
1129
return {
12-
responseIdentifier: generateRandomSlug('response'),
30+
responseIdentifier: RESPONSE_IDENTIFIER,
1331
prompt: '',
1432
choices: [{ id: generateRandomSlug('choice'), content: '', correct: false }],
1533
maxChoices: 1,
@@ -65,8 +83,7 @@ export function parseChoiceInteraction(bodyXml, responseDeclarations) {
6583
return _defaultState();
6684
}
6785

68-
const responseIdentifier =
69-
root.getAttribute('response-identifier') || generateRandomSlug('response');
86+
const responseIdentifier = root.getAttribute('response-identifier') || RESPONSE_IDENTIFIER;
7087
const maxChoices = parseInt(root.getAttribute('max-choices') ?? '0', 10);
7188
const minChoices = parseInt(root.getAttribute('min-choices') ?? '0', 10);
7289
const shuffle = root.getAttribute('shuffle') === 'true';
@@ -95,7 +112,7 @@ export function parseChoiceInteraction(bodyXml, responseDeclarations) {
95112
*/
96113
export function buildChoiceInteractionXML(state, questionType, declarationSchema) {
97114
const {
98-
responseIdentifier = generateRandomSlug('response'),
115+
responseIdentifier = RESPONSE_IDENTIFIER,
99116
prompt,
100117
choices,
101118
maxChoices,

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/index.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,13 @@ export const registry = Object.fromEntries(descriptors.map(d => [d.type, d]));
3232
export function getDescriptorForQuestionType(questionType) {
3333
return descriptors.find(d => d.questionTypes.includes(questionType));
3434
}
35+
36+
/**
37+
* Find the interaction descriptor for a given QTI interaction tag name.
38+
*
39+
* @param {string} tagName
40+
* @returns {import('./defineInteraction').InteractionDescriptor|undefined}
41+
*/
42+
export function getDescriptorForInteractionType(tagName) {
43+
return registry[tagName];
44+
}

0 commit comments

Comments
 (0)