Skip to content

Add QTI match interaction editor - #6168

Merged
AlexVelezLl merged 4 commits into
learningequality:unstablefrom
rtibblesbot:issue-6166-0a33c2
Sep 29, 2026
Merged

AlexVelezLl merged 4 commits into
learningequality:unstablefrom
rtibblesbot:issue-6166-0a33c2

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds a QTI match editor: rows take answers from a shared shuffled pool, plus distractors.
  • Extracts ShuffledResponsePool and EditableChipList from the associate editor; both editors use them.
  • Adds a padding prop to TipTapEditor, and a textCursor prop and public focus() to ClickableRegion.
  • Chip editors save via a Save button under the editor.
  • Aligns the question type selector with the question label.

References

Closes #6166. Builds on #6113.

Reviewer guidance

  • Blank row answers drop on close; keep them?
  • Designs returned 403; layout follows the issue text.
  • Id uniqueness and directedPair round-trip: match/__tests__/parse.spec.js.

QA steps

Setup: pnpm devsetup, sign in as admin, open /channels/<channel-id>/#/qti-demo on an editable channel. Q7 is Associate, Q8 Match; the pencil edits.

  1. Q8: click a row's answer area, type, Save. Save is right-aligned under the editor; the chip joins the row.
  2. Q8: edit a chip, Save. It commits; saved empty, it drops.
  3. Q8: repeat 1–2 by keyboard. Save takes focus; Enter or Space commits.
  4. Q8, Distractors: add a row answer's duplicate. An error shows until removed.
  5. Q7, Distractors: add and edit one. Save sits under the editor.
  6. Q8: open Response type's (?) info. It describes Match; headers read "Prompts and responses", "Correct matches".
  7. Q8, Eagle: clear Bird, Tab through the row, click the cell's blank space. No overlay tab stop; the blank chip opens. Removing Can fly focuses its replacement.
  8. Q8, Bat: clear Can fly, remove Mammal. The blank chip's edit button takes focus; clicking the cell opens it.
  9. Q8: type "Eagle" into Bat's prompt, then click Eagle, a chip, or Save. The click lands; the error follows. Right-click and touch tap (device emulation) also show it.
  10. Q8: clear Whale's match, Close, toggle Show answers; repeat with Whale alone. Missing-match and row-count errors need answers shown; others don't.
  11. Q7, Q8: toggle view and edit; open and close a chip unchanged. Pool order holds until a chip's text changes.

Evidence

Blank correct-match chip

match-blank-chip-flow.webm

Remove down to the minimum

match-remove-filled-leaves-blank.webm

Validation timing

Step Screenshot
Duplicate typed: no error yet Duplicate typed: no error yet
Eagle clicked: error shown Eagle clicked: error shown

View-mode errors

Step Screenshot
Answers hidden: no missing-match error Answers hidden: no missing-match error
Answers shown: missing-match error Answers shown: missing-match error

Pool order

pool-order-kept-until-text-changes.webm
More captures (26)
Step Screenshot
Draft Save Draft Save
Chip joined row Chip joined row
Chip edited Chip edited
Edit saved Edit saved
Cleared chip dropped Cleared chip dropped
Draft Save focused Draft Save focused
Chip Save focused Chip Save focused
Distractor duplicate error Distractor duplicate error
Distractor removed Distractor removed
Associate distractor Save Associate distractor Save
Type selector open Type selector open
Match info modal Match info modal
Match editor copy Match editor copy
Copy at 412px Copy at 412px
Cell click reopens blank chip Cell click reopens blank chip
Eagle blank chip open Eagle blank chip open
Eagle cell click Eagle cell click
Bat: Mammal plus blank chip Bat: Mammal plus blank chip
Focus on blank chip Focus on blank chip
Chip clicked: error shown Chip clicked: error shown
Touch tap: error shown Touch tap: error shown
Answers hidden: blank prompt error Answers hidden: blank prompt error
Answers hidden: no row-count error Answers hidden: no row-count error
Answers shown: row-count error Answers shown: row-count error
Pools before Pools before
Pools reshuffled Pools reshuffled

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

Criterion Why not
≠2 match sets show "cannot be edited here" They show "could not be loaded"; moving the gate into parseItem fixes that.
EditableChipList takes openIndex/draft props Per review, it owns its draft.

Committed 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?
  • Ran pre-flight CI checks (lint, format, tests) and verified all pass
  • Rebased onto the target branch and resolved any conflicts
  • Reorganized commit history into clean, logical commits
  • Audited the diff to ensure only issue-relevant files are changed
  • Built PR body from the repository's PR template with evidence blocks
@rtibblesbot

🟡 Waiting for feedback

Last updated: 2026-09-29 03:59 UTC

@rtibblesbot
rtibblesbot force-pushed the issue-6166-0a33c2 branch 3 times, most recently from 26e289d to 26822e8 Compare September 24, 2026 17:05
@rtibblesbot
rtibblesbot marked this pull request as ready for review September 24, 2026 17:34
@rtibblesbot
rtibblesbot requested review from a team and marcellamaki September 24, 2026 17:34

@AlexVelezLl AlexVelezLl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good initial implementation!

);

// Where focus goes once nothing nearer is left to take it.
function addControl() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be something like getAddControl instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +167 to +172
// 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();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we implement a click.stop within the ClickableRegion instead of this hard-coded check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +175 to +176
draftKey.value += 1;
emit('open-draft');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good abstraction, let's try to use only the refs directly wherever we need them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +280 to +288
// An empty list still needs a target to click.
&.is-region {
min-height: 64px;
cursor: text;

> ::v-deep .overlay-button {
cursor: text;
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we create a new prop for this instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ClickableRegion's textCursor prop: text cursor, no hover tint (8e0068c81). The v-deep on .overlay-button is gone.

Comment on lines +116 to +134
<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"
/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"
/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (22d646ff2): the row answers use the neutral border, and green is left to the pool.

@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Match editor after the review changes.

View mode, answers shown: two columns with headers; green only in the pool.
View mode

Edit mode: headers line up with the rows; a row's only answer has its delete disabled.
Edit mode

Answer draft with Save; the region is off while it is open.
Draft

Small screen: each stacked row labels its prompt and answers.
Small screen

Save adds the chip; deleting answers stops at the last one:

match-save-delete.webm
  • The first three are from the QA server. It built before the last stopPropagation fix, which does not change rendering.
  • The small-screen still and the video come from a local webpack build of the demo page, loaded in headless Chromium, because the QA server builds once per session.
  • axe on the edit view, with a draft open, reports only the existing AddListItemButton contrast issue.

@rtibblesbot
rtibblesbot force-pushed the issue-6166-0a33c2 branch 2 times, most recently from 5859ac6 to 202802b Compare September 27, 2026 01:44

@AlexVelezLl AlexVelezLl left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking better, still I found a couple of details :)

class="section-label"
:style="{ color: $themePalette.grey.v_700 }"
>
{{ matchingRowsLabel$() }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please match every string on the specs verbatim, don't change them

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 rows
  • Learners 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 row
  • Save

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I have a long formula in the prompts, it overflows. Would making this fix within the TipTapEditor be reasonable without risks of breaking other things? If so, it should be there; if not, then please fix this just as the associate interaction manages it.

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a placeholder for the prompt?

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Formula overflow before the fix
Selector aligned with the question label; formula clipped inside the row prompt
Row prompt placeholder; raised Save on an open answer chip

  • 1: before — formula and toolbar run under the answers box.
  • 2: after — the QA server keeps its first build, so the .row-prompt track rule is applied in devtools; everything else is the pushed build.
  • 3: blank row prompt placeholder, raised Save on an editing chip.

@rtibblesbot
rtibblesbot force-pushed the issue-6166-0a33c2 branch 2 times, most recently from cf36aa9 to bfff2bc Compare September 28, 2026 17:40
:style="{ borderColor: $themeTokens.fineLine }"
:suppressed="!isRegionActive"
:ariaLabel="regionLabel"
textCursor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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$() }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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."

Comment on lines +117 to +122
<KButton
:text="saveChipBtn$()"
appearance="raised-button"
:primary="true"
@click="onSave"
/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@rtibblesbot
rtibblesbot force-pushed the issue-6166-0a33c2 branch 2 times, most recently from 6b7f047 to 1a08bf1 Compare September 28, 2026 21:28
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

New answer: Save below the editor
Open answer: Save below the editor

@rtibblesbot
rtibblesbot force-pushed the issue-6166-0a33c2 branch 2 times, most recently from c359eb5 to bbcd896 Compare September 28, 2026 23:25
rtibblesbot and others added 4 commits September 28, 2026 20:27
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>

@AlexVelezLl AlexVelezLl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good. It looks very polished visually, and resulting xml's are correct. LGTM!

@AlexVelezLl
AlexVelezLl merged commit eb65f55 into learningequality:unstable Sep 29, 2026
13 checks passed
@rtibblesbot
rtibblesbot deleted the issue-6166-0a33c2 branch September 29, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[QTI] Implement Match Interaction editor

2 participants