Add QTI match interaction editor - #6168
Conversation
26e289d to
26822e8
Compare
26822e8 to
94b8038
Compare
AlexVelezLl
left a comment
There was a problem hiding this comment.
Good initial implementation!
| ); | ||
|
|
||
| // Where focus goes once nothing nearer is left to take it. | ||
| function addControl() { |
There was a problem hiding this comment.
should this be something like getAddControl instead?
There was a problem hiding this comment.
Renamed to getAddControl in 8e0068c81. I searched every function the PR adds that returns a value: openMatchIndex, rowDraft, borderStyle and errorsWith in the match editor matched. The first two are gone with the draft refactor, and the others are now getBorderStyle / getErrorsWith.
| // An open chip's region is suppressed and stops no click, so a click into | ||
| // its editor reaches this box too — and must not start a draft. | ||
| function onRegionClick(event) { | ||
| if (event?.target?.closest('.chip-item')) return; | ||
| onAdd(); | ||
| } |
There was a problem hiding this comment.
Should we implement a click.stop within the ClickableRegion instead of this hard-coded check?
There was a problem hiding this comment.
The .closest check is gone (8e0068c81). The region is now suppressed while any of its editors is open, so a click inside an open chip never reaches an active region. I didn't put click.stop in ClickableRegion itself: a suppressed region that stopped clicks would swallow the document click TipTap closes on. The distractor box, which is always suppressed, would then stop closing other editors.
One case needed a stop anyway. With a native click, closing an editor re-renders before the click bubbles on, which unmounts the wrapper's @click.stop. Delete, Save and discard reopened a draft that way in the browser, so those three handlers now call stopPropagation() themselves.
| draftKey.value += 1; | ||
| emit('open-draft'); |
There was a problem hiding this comment.
Question: Is the content of the old draft reliably saved before remounting? Or is it rather an indirect, flaky behavior that may break under certain circumstances?
There was a problem hiding this comment.
Reliably, but through the blur. TipTap emits update on blur, and pressing add blurs the editor on mousedown (or on focus-out for the keyboard), so the content is committed before the click commits the draft. The list now keeps the draft's last-written content itself rather than reading a prop. In region mode the second press can't happen: the region is off while a draft is open.
There was a problem hiding this comment.
I don't think this is a good abstraction, let's try to use only the refs directly wherever we need them.
There was a problem hiding this comment.
Removed (8e0068c81). All 3 users now focus through refs after nextTick: EditableChipList, the associate pair rows and the match rows. Each focuses deleteButtons[min(index, last)], or the add control once nothing removable is left.
| // An empty list still needs a target to click. | ||
| &.is-region { | ||
| min-height: 64px; | ||
| cursor: text; | ||
|
|
||
| > ::v-deep .overlay-button { | ||
| cursor: text; | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we create a new prop for this instead?
There was a problem hiding this comment.
Added ClickableRegion's textCursor prop: text cursor, no hover tint (8e0068c81). The v-deep on .overlay-button is gone.
| <EditableChipList | ||
| class="row-answers" | ||
| addMode="region" | ||
| :chips="row.matches" | ||
| :openIndex="openMatchIndex(index)" | ||
| :draft="rowDraft(index)" | ||
| :addLabel="addMatchLabel$({ number: index + 1 })" | ||
| :listLabel="rowAnswersLabel$({ number: index + 1 })" | ||
| :chipLabel="position => editMatchLabel$({ number: index + 1, position })" | ||
| :deleteLabel="position => deleteMatchBtn$({ number: index + 1, position })" | ||
| :errorMessages="matchErrorMessages[index]" | ||
| @open-chip="position => openMatch(index, position)" | ||
| @update-chip="(position, html) => setMatchContent(index, position, html)" | ||
| @remove-chip="position => onRemoveMatch(index, position)" | ||
| @open-draft="openDraft(index)" | ||
| @update-draft="setDraftContent" | ||
| @discard-draft="discardDraft" | ||
| @close="closeOpenTarget" | ||
| /> |
There was a problem hiding this comment.
For region mode, could we suspend the background color change on hover and only have the text cursor? Also, let's disable the clickable region if a draft is already open. Also, let's please add a save button for both modes, and only add a new chip when pressed. If the editor is closed and the save button was not pressed, let's add the content anyway. The save closes the open TipTapEditor.
There was a problem hiding this comment.
Done (8e0068c81, 22d646ff2):
- Region mode: text cursor, no hover tint.
- The region is off while any of its editors is open.
- The draft has a Save button in both modes. Save commits and closes, and closing without saving still commits.
| @discard-draft="discardDraft" | ||
| @close="closeOpenTarget" | ||
| /> | ||
|
|
There was a problem hiding this comment.
Also, let's not allow deleting the last answer; we should always have at least one. If only one is left, the remove button should be disabled.
There was a problem hiding this comment.
Done (22d646ff2). EditableChipList takes minChips, and answer lists pass 1: the remove buttons are disabled at the minimum, and a blank last answer is kept on close rather than dropped. removeMatch also refuses to remove a row's last answer.
| <li | ||
| v-for="(choice, position) in row.matches" | ||
| :key="`${choice.id}-${position}`" | ||
| class="chip is-correct" |
There was a problem hiding this comment.
Let's not use the green background for correct answers listed in the answers section, only in the pool, if the associate. Just like the associate interaction does.
There was a problem hiding this comment.
Done (22d646ff2): the row answers use the neutral border, and green is left to the pool.
94b8038 to
ae35f88
Compare
5859ac6 to
202802b
Compare
| class="section-label" | ||
| :style="{ color: $themePalette.grey.v_700 }" | ||
| > | ||
| {{ matchingRowsLabel$() }} |
There was a problem hiding this comment.
The spec images are blocked by my network proxy, so I cannot read the design copy. Could you paste the spec strings as text? Current match strings, all in qtiEditorStrings.js:
Matching rowsLearners match responses from a shuffled set to each prompt. A prompt can have more than one answer. Include distractors to increase difficulty.Prompt/Answers(column headers)Enter a prompt/Enter an answer(placeholders)Add rowSave
There was a problem hiding this comment.
You need these changes:
- "Matching rows" -> "Prompts and responses"
- "Learners match responses from a shuffled set to each prompt. A prompt can have more than one answer. Include distractors to increase difficulty." -> "Learners match responses from a shuffled set to a fixed prompt. Include distractors to increase difficulty."
- "Prompt" / "Answers" -> "Prompt" / "Correct matches".
- "Learners must match each prompt with its correct answers." -> "Learners match each item in one list to one or more correct items in another, where items can have multiple valid matches, and distractors can be included to increase difficulty."
There was a problem hiding this comment.
Done: all four strings now match your list verbatim (qtiEditorStrings.js). The mobile row labels use the same column-header string, so they read Correct matches too.
| </template> | ||
|
|
||
| <template v-if="mode === 'edit'"> | ||
| <ClickableRegion |
There was a problem hiding this comment.
Fixed in the cards, not TipTap. TipTap already clips its own content; the card around it is display: grid with an implicit track that grows to the formula's width. Fixing it inside TipTap would change how every content-sized chip measures, so the cards now declare grid-template-columns: minmax(0, 1fr).
Searched QTIEditor/ for grid cards wrapping an editor: 2 matched (match .row-prompt, associate .pair-card), both changed. Associate's toolbar was spilling under the next card too.
| class="item-card-text" | ||
| :class="{ 'is-closed': !isRowPromptOpen(index) }" | ||
| > | ||
| <TipTapEditor |
There was a problem hiding this comment.
Added: a blank, closed row prompt shows Enter a prompt. That copy is mine until the spec strings arrive (see the strings thread).
Searched match for blank editors with no placeholder: 3 (question prompt, row prompts, distractors). Only row prompts changed. The question prompt has none in any editor, and a blank distractor drops when it closes.
| </div> | ||
| <KButton | ||
| :text="saveChipBtn$()" | ||
| appearance="flat-button" |
There was a problem hiding this comment.
Let's use a primary raised button instead, just as in the specs.
Also, could you show this save button on the editing rows too? Right now, it only appears if you are adding a new chip.
There was a problem hiding this comment.
Done: raised-button primary, and an open chip now has Save beside its delete button. Saving keeps focus on that chip. Searched for Save buttons: 1 component (EditableChipList), both of its instances changed.
202802b to
1026ca7
Compare
cf36aa9 to
bfff2bc
Compare
| :style="{ borderColor: $themeTokens.fineLine }" | ||
| :suppressed="!isRegionActive" | ||
| :ariaLabel="regionLabel" | ||
| textCursor |
There was a problem hiding this comment.
For some reason, I am not seeing the text cursor when I hover over this clickable region. It only works on the borders, where the chip-list is not present.
There was a problem hiding this comment.
I can't reproduce this on 1a08bf1 in headless Chromium, match item on the QTI demo page. I sampled elementFromPoint every 20×10px across all three answer regions. Every hit on ul.chip-list has computed cursor: text, inherited from the region root, and no stylesheet rule overrides it. Which browser and OS are you on? Does it happen with a row's answers closed and no other editor open?
There was a problem hiding this comment.
You were right: at bfff2bc the text cursor sat only on the overlay button, under the chip list. 1a08bf1 moved it to the region root, so the chip list now shows it too; no answer to my questions needed.
| class="section-label" | ||
| :style="{ color: $themePalette.grey.v_700 }" | ||
| > | ||
| {{ matchingRowsLabel$() }} |
There was a problem hiding this comment.
You need these changes:
- "Matching rows" -> "Prompts and responses"
- "Learners match responses from a shuffled set to each prompt. A prompt can have more than one answer. Include distractors to increase difficulty." -> "Learners match responses from a shuffled set to a fixed prompt. Include distractors to increase difficulty."
- "Prompt" / "Answers" -> "Prompt" / "Correct matches".
- "Learners must match each prompt with its correct answers." -> "Learners match each item in one list to one or more correct items in another, where items can have multiple valid matches, and distractors can be included to increase difficulty."
| <KButton | ||
| :text="saveChipBtn$()" | ||
| appearance="raised-button" | ||
| :primary="true" | ||
| @click="onSave" | ||
| /> |
There was a problem hiding this comment.
Let's have this button on a new line instead. It should be aligned to the right. The button should be aligned with the TipTapEditor.
There was a problem hiding this comment.
Done: Save now sits on its own line under the editor, right-aligned to the editor's edge. I searched saveChipBtn for the same layout and found 2 places, the new-answer draft and an open existing answer. Changed both. Screenshots: #6168 (comment)
6b7f047 to
1a08bf1
Compare
c359eb5 to
bbcd896
Compare
EditableChipList adds a region add mode, reachable by keyboard through ClickableRegion's overlay button. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The card body already insets its content, so the selector's own horizontal padding pushed it right of every editor's question label. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bbcd896 to
e7a9b44
Compare
AlexVelezLl
left a comment
There was a problem hiding this comment.
Code changes look good. It looks very polished visually, and resulting xml's are correct. LGTM!












Summary
ShuffledResponsePoolandEditableChipListfrom the associate editor; both editors use them.paddingprop toTipTapEditor, and atextCursorprop and publicfocus()toClickableRegion.References
Closes #6166. Builds on #6113.
Reviewer guidance
directedPairround-trip:match/__tests__/parse.spec.js.QA steps
Setup:
pnpm devsetup, sign in as admin, open/channels/<channel-id>/#/qti-demoon an editable channel. Q7 is Associate, Q8 Match; the pencil edits.Evidence
Blank correct-match chip
match-blank-chip-flow.webm
Remove down to the minimum
match-remove-filled-leaves-blank.webm
Validation timing
View-mode errors
Pool order
pool-order-kept-until-text-changes.webm
More captures (26)
Add a correct match:
match-add-correct-match-and-save.webm
Edit and clear a chip:
s2-edit-save-clear.webm
Match distractors:
match-distractor-add-edit-remove.webm
Associate distractors:
associate-distractor-add-edit.webm
AI usage
Claude Code planned and implemented this; verified with Jest, pre-commit, and browser QA with axe.
🤖 Generated with Claude Code
Deviations from the issue spec
parseItemfixes that.EditableChipListtakesopenIndex/draftpropsCommitted separately: no commit on this branch introduced the lines these changes touch, so they are a new commit rather than folded into the work they amend:
contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/__tests__/Editor.spec.js@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
🟡 Waiting for feedback
Last updated: 2026-09-29 03:59 UTC