Skip to content

Add QTI match interaction editor - #6168

Open
rtibblesbot wants to merge 3 commits into
learningequality:unstablefrom
rtibblesbot:issue-6166-0a33c2
Open

rtibblesbot wants to merge 3 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: each row takes one or more answers from a shared shuffled pool, plus optional distractors.
  • Extracts the associate editor's response pool and distractor list into shared ShuffledResponsePool and EditableChipList, used by both editors.

References

Closes #6166. Builds on #6113.

Reviewer guidance

Open questions:

  • Non-two-set matches show "could not be loaded", not "cannot be edited here". Right?
  • Blank row answers drop on close. Keep them with an error instead?
  • Designs returned 403; layout follows the issue text. Matches?

axe: only the existing AddListItemButton contrast violation.

QA steps

Setup: pnpm devsetup, pnpm devserver, sign in as admin, open /channels/<channel-id>/#/qti-demo on an editable channel, click Question 8's (Match) pencil.

  1. Distractors: "Add distractor", type, "Save": it becomes a chip. Add another, type, click its close: the draft is gone and focus is on "Add distractor". Open a distractor, clear it, click away: it drops out. Retype a distractor as "Mammal": no duplicate error while open; on close, it and the matching answers show "Distractors cannot repeat another item".

Evidence

Match distractors

Step Screenshot
Draft open with Save and close Draft open with Save and close
Existing distractor cleared, open Existing distractor cleared, open
After: duplicate flagged once the chip closes After: duplicate flagged once the chip closes

Authoring a new match item

Step Screenshot
New match question New match question
Empty interaction Empty interaction
Answer draft open Answer draft open
Blank answer placeholder Blank answer placeholder
match-author-from-empty.webm

Editing and deleting answers

Step Screenshot
Initial answers Initial answers
Answer chip open Answer chip open
Blank last answer shows placeholder Blank last answer shows placeholder
match-edit-delete-answers.webm

Switching the open editor

Step Screenshot
Row 3 answer draft open Row 3 answer draft open
Distractor open; draft committed Distractor open; draft committed
Row 3 chip open after switches Row 3 chip open after switches
s3-switch-open-editor.webm

Validation errors

Step Screenshot
Missing question prompt Missing question prompt
Blank row prompt Blank row prompt
Duplicate row prompts Duplicate row prompts
Duplicate answer in a row Duplicate answer in a row
Distractor repeats an answer Distractor repeats an answer
Distractor repeats a distractor Distractor repeats a distractor
Too few rows Too few rows
s5-validation-trigger-clear.webm

Keyboard row deletion

Step Screenshot
Row 2 delete focused Row 2 delete focused
Focus moves to the next row Focus moves to the next row
One row left: Add row focused One row left: Add row focused
match-keyboard-row-delete.webm

Leaving edit mode discards drafts

Step Screenshot
Distractor draft open Distractor draft open
After close: distractor draft gone After close: distractor draft gone
Row 2 answer draft open Row 2 answer draft open
After close: answer draft gone After close: answer draft gone
s7-leave-edit-discards-drafts.webm

View mode

Step Screenshot
Show answers off Show answers off
Show answers on Show answers on
Stacked, answers off Stacked, answers off
Stacked, answers on Stacked, answers on

Responsive layout

Step Screenshot
Large: open row stays side by side Large: open row stays side by side
Medium: all closed Medium: all closed
Medium: open row stacks Medium: open row stacks
Small: closed Small: closed
Small: chip open Small: chip open
View at 420px View at 420px

Associate regression

Step Screenshot
Distractor draft open Distractor draft open
Distractor chip open Distractor chip open
Distractor cleared, open Distractor cleared, open
Blank pair item Blank pair item
Focus after deleting pair 2 Focus after deleting pair 2
One pair left One pair left
s9-assoc-add-edit-blank-delete.webm

AI usage

Used Claude Code to plan and implement the shared components, the match parser, validator and editor, following the associate plugin. Verified with the QTIEditor Jest suites, pre-commit, and headless-browser QA with keyboard walkthroughs and axe.

🤖 Generated with Claude Code


@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-25 23:56 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.

</div>
</ClickableRegion>

<EditableChipList

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 a distractor is repeating an answer, the answer chip should also get flagged as an error, similar to how we do for the associate interaction.

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): an answer whose content a distractor repeats shows "Distractors cannot repeat another item" with a red border.


<EditableChipList
class="row-answers"
addMode="region"

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 a chip answer is blank, let's have a placeholder text in it.

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): a blank closed answer shows "Enter an answer" through the new placeholder prop on EditableChipList.

<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 and others added 3 commits September 25, 2026 16:09
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>
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