Skip to content

Add the QTI associate interaction plugin - #6113

Open
rtibblesbot wants to merge 7 commits into
learningequality:unstablefrom
rtibblesbot:issue-6101-e6feda
Open

rtibblesbot wants to merge 7 commits into
learningequality:unstablefrom
rtibblesbot:issue-6101-e6feda

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Associate questions could not be authored: an item whose body is <qti-associate-interaction> had no plugin, so it fell back to the choice descriptor — the header read "Multiple Choice" and the editor rendered no choices at all, because an associate pool holds no <qti-simple-choice>. This adds the plugin end to end — the flat choice pool parses into correct pairs plus distractors, serializes back to a cardinality="multiple" base-type="pair" declaration, validates, and renders in an editor.

A long formula is one unbreakable inline node and painted over the prompt card's border. Fixing that was requested in review; the bound went into TipTapEditor's own .editor-content rather than per interaction editor, so every TipTap editor clips — the legacy exercise editors included.

References

Fixes #6101.

Reviewer guidance

The 746 QTI editor Jest tests pass. Both browser flows start from pnpm devsetup sample data: open any channel, go to #/qti-demo, and edit question 7, "Match each country with its capital city" — three pairs and two distractors.

  1. Open pair 2's card and delete that row with its editor still open — the editor closes rather than landing on the row that shifted up, and the rows below renumber. Distractors and pairs delete down to one pair, whose delete control is then disabled.
  2. Narrow the window to ~900px and open a pair card — only that row stacks and its toolbar fits inside the card, while the rows beside it stay side by side. Below ~840px every row stacks, open or not.

Worth questioning, both following from the issue's XML rules 8 and 9:

  • buildXML collapses two choices with equal text into one pool entry and bumps its match-max, so a player renders one option matchable twice where the author drew two. Blank content is exempt, otherwise a freshly added pair could not round-trip.
  • A distractor is derived from match-max minus the choice's appearances in the correct response, so hand-written XML with a match-max larger than intended silently gains distractors on import.

Screenshots

State Screenshot
Edit mode Edit
View mode, answers hidden View
View mode, answers shown View with answers
Validation Validation
Small screen, editing a pair Mobile

axe-core (WCAG 2 AA) on the demo page reports one violation: colour contrast on AddListItemButton's label. It reproduces on the unchanged ordering editor, which uses the same shared component.

Deviations from the issue spec

Criterion Why not
A new distractor is saved from the editor opened below the pool Committed when that editor closes instead, with no Save button, per review — matches the other RTEs in the editor

AI usage

Used Claude Code to implement the plugin test-first against a written plan, following the ordering interaction as the reference. Verified with the Jest suite, pre-commit, manual QA on the QTI demo page, and an axe-core audit.


@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-16 16:30 UTC

@rtibblesbot
rtibblesbot force-pushed the issue-6101-e6feda branch 3 times, most recently from 6e5a1f4 to 2c9fff3 Compare August 31, 2026 16:53
@rtibblesbot
rtibblesbot marked this pull request as ready for review August 31, 2026 16:54

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

@rtibblesbot, from the screenshots, I can see that the padding of the chips is not the same as in the specs, and the colors of the pair rows are not the same as in the specs. Take a closer look at the images in the issue and check that everything is correctly aligned.

@rtibblesbot

Copy link
Copy Markdown
Contributor Author

@AlexVelezLl Fixed, and I swept every element in the editor against the six mockups rather than only the two you named — eight divergences, all changed:

  • Pair rowsgrey.v_100grey.v_50 with a fineLine border; cards now sit on surface instead of inheriting the row tint.
  • Pair cards — 40px tall with an 8px text inset; were 50px at the editor's default 24px inset.
  • Chipspadding: 4px 12px with an 8px gap before the delete control; were 0 4px 0 12px, so the × sat flush against the border. Dropping the editor's paragraph margins takes them from 50px to 34px.
  • Chip fill — a chip inverts against its container: tinted in the distractor pool, white in the shuffled pool. Both were white.
  • Shuffled pool boxgrey.v_100grey.v_50.
  • Correct-answer chip bordergreen.v_500green.v_600, matching the mockup and the choice/ordering editors.
  • Pair validation message — aligned to the row's left edge; it was indented under the cards.
  • CopyMatching pairs, Distractors (optional), Response pool (shuffled), Add distractor and both sublabels, verbatim from the mockups. The branch shipped the plan's provisional wording (Correct pairs, Additional options, Options (shuffled), Add option) — which is also why the labels in the PR body's screenshots don't match what the branch renders. The four below are current.

Two mockup details I left out, both more than styling — say the word and I'll add them:

  • the SAVE button on the new-distractor editor (the sibling editors commit on type and close via the minimize control)
  • the Answer placeholder in an empty pair card (TipTapEditor has no placeholder prop)
State Screenshot
Edit Edit
View, answers shown View
Validation Validation
Small screen Mobile

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

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

Just few findings from a high-level overview.

},

associateLabel: {
message: 'Connect pairs',

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 translate it to "Associate" 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 Associate.

message: 'Connect pairs',
context: 'Display name for an associate question type shown in the question type selector',
},
associateDescription: {

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.

Must associate pairs of items, or something similar

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.

Now Learners must associate pairs of items.

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 add what we added for choice interaction, and let's prevent the removal of the last pair, also let's disable the remove button if its the last pair.

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.

removePair is a no-op at one pair, matching removeChoice, and the delete button is disabled there. Spec gained that case plus a max-associations block mirroring choice's max-choices one. Checked the branch's other list mutators: removeDistractor is the only sibling, and distractors are optional, so it keeps no floor.


it('appends a third pair when the add pair button is clicked', async () => {
renderEditor();
await fireEvent.click(screen.getByRole('button', { name: tr.$tr('addPairBtn') }));

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 userEvent.setup() 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.

Whole editor spec is on userEvent.setup() now — 43 interactions, no fireEvent left under interactions/associate/. It was the only spec on the branch using it; the parse, validate and composable specs call the units directly.

Comment on lines +83 to +89
it('appends a second distractor when the add distractor button is clicked', async () => {
renderEditor();
await fireEvent.click(screen.getByRole('button', { name: tr.$tr('addDistractorBtn') }));
expect(
screen.getByRole('button', { name: tr.$tr('deleteDistractorBtn', { number: 2 }) }),
).toBeInTheDocument();
});

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.

This behavior is incorrect. When the "Add distractor" button is clicked, it should show a TipTapEditor below the distractors pool, and it should be saved when the Save button is clicked.

Image

If focus is lost and tiptap is closed, let's add a small "new distractor" label so that users can come back and edit it until they hit save. Once they hit save, then we can show the add distractor button again.

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.

Reworked. Add distractor opens a TipTap editor below the pool with a Save button; the draft lives outside state, so nothing joins the pool and nothing is emitted until Save. Closing the editor (by clicking a pair, say) leaves a small New distractor chip that reopens it with the written content still there; Save turns it into a chip and brings the Add distractor button back.

Verified in the running editor, not only in jsdom — worth it, because the first version passed its tests and was dead in the browser: the click that opens the draft finishes bubbling after TipTap has mounted its outside-click listener, so the editor closed itself immediately. Add pair had the same bug (the new pair's editor never stayed open). Both add buttons now stop the click.

expect(contentsOf(parse(ASSOCIATE_XML, [decl]).pairs)).toEqual([['Capulet', 'Montague']]);
});

it('drops a pair naming identifiers that only exist on Object.prototype', () => {

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 not clear to me what this tests for, is because these choices are not declared on ASSOCIATE_XML?

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.

Right — constructor/toString are not declared in ASSOCIATE_XML, and the point is that the pool lookup must not resolve them off Object.prototype (it is a Map, not a plain object). Renamed the test to say that and added the reason as a comment on both prototype cases.

Comment on lines +267 to +296
it('counts a distractor repeat of paired content towards match-max', () => {
const distractors = [{ id: 'choice_zzz00000', content: 'Antonio' }];
const root = parseXmlString(build({ ...baseState, distractors }).bodyXml);
const antonio = choicesOf(root).filter(el => el.textContent === 'Antonio');
expect(antonio).toHaveLength(1);
expect(antonio[0].getAttribute('match-max')).toBe('2');
});

it('reassigns the id of a later choice that reuses an id with different content', () => {
const pairs = [
baseState.pairs[0],
[
{ id: 'choice_aaa11111', content: 'Capulet' },
{ id: 'choice_ddd44444', content: 'Montague' },
],
];
const { bodyXml, responseDeclarations } = build({
...baseState,
pairs,
distractors: [],
});
const [capulet] = choicesOf(parseXmlString(bodyXml)).filter(
el => el.textContent === 'Capulet',
);
expect(capulet.getAttribute('identifier')).toMatch(/^choice_/);
expect(capulet.getAttribute('identifier')).not.toBe('choice_aaa11111');
expect(valuesOf(parseXmlString(responseDeclarations[0]))[1]).toBe(
`${capulet.getAttribute('identifier')} choice_ddd44444`,
);
});

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 make some permutations of these to test what happens if: the first appearance is repeated, then a following has the same id but different content, etc. Also, lets test that the first appearance keep the id.

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 an id normalization block with four permutations: a repeat carrying a different id (the first id wins, the second never appears), a later choice reusing an id with different content (the first keeps the id), a repeat followed by an id conflict on the same choice (match-max="2" plus a fresh id for the conflict), and a third choice repeating already-reassigned content. Each asserts the pool and the <qti-value> list.

Comment on lines +128 to +133
describe('TOO_FEW_PAIRS', () => {
it('returns error when there are no pairs at all', () => {
expect(errorCodes(validateAssociateInteraction(makeState({ pairs: [] })))).toContain(
ValidationError.TOO_FEW_PAIRS,
);
});

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.

This should test also valid pairs, if there is a pair, but its invalid, then show the too few valid pairs error.

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: the only pair blank, the only pair holding the same content twice, every pair invalid for a different reason, and one valid pair among invalid ones (no error).

pairedCount.set(id, (pairedCount.get(id) || 0) + 1);
}

const distractors = pool.flatMap(({ id, content, matchMax }) =>

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 use 'lodash/flatMap' 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.

Done, and the rest of the branch with it — 6 sites: parse.js (3), validate.js (1), the editor (2). No native flat/flatMap left in the associate plugin.

Comment on lines +298 to +305
function shuffled(items) {
const result = [...items];
for (let i = result.length - 1; i > 0; i--) {
const j = Math.floor(Math.random() * (i + 1));
[result[i], result[j]] = [result[j], result[i]];
}
return result;
}

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 use lodash/shuffle 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.

Done — the hand-rolled Fisher–Yates is gone.

@AlexVelezLl AlexVelezLl self-assigned this Sep 8, 2026
Comment on lines +265 to +272
<KIconButton
icon="close"
size="mini"
:ariaLabel="deleteDistractorBtn$({ number: index + 1 })"
:tooltip="deleteDistractorBtn$({ number: index + 1 })"
:color="$themePalette.grey.v_800"
@click="onRemoveDistractor(index)"
/>

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.

Lets use size="small" and v_700 for the color

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 — size="small", $themePalette.grey.v_700.

Searched the editor for other KIconButtons: one more, the pair delete, which your other comment puts at base/gray/400 — also now size="small".

Comment on lines +290 to +331
<div
v-if="isDraftOpen"
class="draft-editor item-border"
:style="{ borderColor: $themeTokens.fineLine }"
>
<TipTapEditor
:value="draft.content"
mode="edit"
format="html"
:minHeight="'48px'"
autofocus
:imageProcessor="EditorImageProcessor"
:tabindex="-1"
class="editor"
@update="setDraftContent"
@minimize="closeOpenTarget"
/>
</div>
<ClickableRegion
v-else
class="chip draft-chip is-tinted"
:style="{ borderColor: $themeTokens.fineLine }"
:aria-label="editNewDistractorLabel$()"
@click="openDraft"
>
{{ newDistractorLabel$() }}
</ClickableRegion>
<KButton
primary
:text="saveDistractorBtn$()"
@click="saveDraft"
/>
</div>
<div
v-else
@click.stop
>
<AddListItemButton
:label="addDistractorBtn$()"
@click="onAddDistractor"
/>
</div>

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 make some UX changes here so that it's consistent with other RTEs where we don't need a save button.

  1. Let's remove the save button. When the RTE is blurred or closed, it will automatically be added to the list (if it was new) or updated. If the content is empty, then remove it from the list.
  2. The add distractor will always be present.
  3. With this, no "new distractor" placeholder chip should be present at any point.
  4. The remove button should always be present on the current editable row, no matter if it's a new distractor; this will be the "cancel" button in case the user regrets it after hitting the add distractor button.

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.

Reworked, all four points:

  1. Save gone. Closing the editor commits: a written draft joins the pool, and an existing distractor emptied then closed leaves it. Verified in the browser that TipTap's blur-time update lands before the click that closed the editor, so the committed content is the typed content.
  2. Add distractor is always rendered.
  3. newDistractorLabel / editNewDistractorLabel / saveDistractorBtn deleted.
  4. The draft row carries the same remove button as a chip, which is also how you abandon 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.

New distractor, running locally — no Save, Add distractor still there, remove button on the row:

new distractor

/>
</div>

<ValidationMessage

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 set the RTE border to red if it has an error? If it's an "Answers within a pair cannot be the same" error, then both RTEs should have a red border.

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. pairItemStyle turns a card's border red for a blank member, for both members when the pair duplicates itself, and for a member a distractor repeats (your validate.js comment).

Swept the other bordered editors in the file for the same gap: distractor chips get the same treatment via distractorStyle; the prompt already had it.

Comment on lines +245 to +272
<ClickableRegion
class="chip-region"
:class="{ 'is-clickable': !isDistractorOpen(index) }"
:suppressed="isDistractorOpen(index)"
:aria-label="editDistractorLabel$({ number: index + 1 })"
@click="openDistractor(index)"
>
<TipTapEditor
:value="choice.content"
:mode="isDistractorOpen(index) ? 'edit' : 'view'"
format="html"
:minHeight="'48px'"
:autofocus="isDistractorOpen(index)"
:imageProcessor="EditorImageProcessor"
:tabindex="-1"
class="editor"
@update="html => setDistractorContent(index, html)"
@minimize="closeOpenTarget"
/>
</ClickableRegion>
<KIconButton
icon="close"
size="mini"
:ariaLabel="deleteDistractorBtn$({ number: index + 1 })"
:tooltip="deleteDistractorBtn$({ number: index + 1 })"
:color="$themePalette.grey.v_800"
@click="onRemoveDistractor(index)"
/>

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.

We need a small gap between the remove button and the RTE content.

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.

Lets also remove the li's padding, and the border color if the distractor is in edit mode.

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.

Both done. The gap was only missing while editing — the chip class carried gap: 8px, the editing row had no rule of its own. It now shares gap: 8px with the chip, and drops the padding and the border colour while open, since the editor draws its own.

:disabled="state.pairs.length <= 1"
:ariaLabel="deletePairBtn$({ number: index + 1 })"
:tooltip="deletePairBtn$({ number: index + 1 })"
:color="$themePalette.grey.v_800"

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 base/gray/400 as per the specs and size="small".

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 — $themePalette.grey.v_400, size="small".

padding: 4px 12px;
background-color: v-bind('$themeTokens.surface');
border: 1px solid;
border-radius: 4px;

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.

Radius of all chips is 8px

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 — 8px on .chip and on .chip-region, the two rules that round a chip. The pool box, pair rows and pair cards are cards rather than chips, so they stay at 4px; say if the specs put those at 8px too.

Comment on lines +597 to +599
const byId = new Map(
[...flatten(state.value.pairs), ...state.value.distractors].map(c => [c.id, c]),
);

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.

There may be repeated IDs that are still valid, right? This Map will squash them. However, if the same response appears in two different pairs, they will see the response twice in the chips list.

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.

Right, the Map was wrong. It now folds by content instead of by id, which is what buildAssociateInteractionXML does — same-content choices become one qti-simple-associable-choice with a match-max. So the preview shows exactly the options delivery renders: a response used in two pairs is one chip that can be used twice, not two chips.

If you meant it should show twice, say so and I will key it by position instead — the change is one line.

>
<li
v-for="choice in shuffledPool"
:key="choice.id"

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.

We cannot use choice.id if id may be repeated across several choices.

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 — keyed by position.

Swept the branch for the same assumption. Three :key on choices: the pair cards and the distractor chips already carried -${position} / -${index}, so only the pool needed changing. Three id-keyed lookups had the defect and are now matched on content: the blank-choice error set, the paired-ids set behind the green border in showAnswers, and the pool dedup above.

}

function openDraft() {
if (props.mode === 'edit') openTarget.value = { kind: '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.

Could we have a small constant for these kinds?

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 — module-level frozen OpenTarget with PROMPT / PAIR / DISTRACTOR / DRAFT, used by every read and write of openTarget.kind. No other bare-string discriminators in the branch.

} else if (state.value.pairs.length > 0) {
openPairItem(0, 0);
}
emit('update:interaction', workingInteraction.value);

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.

Why? This is different from what we have on other interaction editors, right?

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.

No change — this is the ordering editor's pattern, OrderingInteractionEditor.vue:309 and :318-321, line for line. TextEntryEditor.vue:345 gates the same way.

The gate is deliberate: a mode="view" preview must not write back to the parent, so the entry emit moved into the mode watcher and the ongoing one is gated. ChoiceInteractionEditor is the odd one out — it emits in view mode too. Covered by the does not emit update:interaction in view mode test.

Happy to align all four either way if you want one rule.

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 always emit only in edit mode, and only if the content changed, not just because the mode changed.

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. The mode watcher no longer emits; workingInteraction is now compared with isEqual, so an emit needs a real change to bodyXml or responseDeclarations — reopening an editor or retyping the same text stays silent.

Tests follow: mount in edit mode asserts no emit, plus a case for moving the open editor between pair items.

Searched every update:interaction emit under QTIEditor/ — four editors, three others match the pattern: ChoiceInteractionEditor.vue:382 (immediate, no mode gate), OrderingInteractionEditor.vue:309 (mode watcher), TextEntryEditor.vue:349 (immediate). All three are outside this diff, so I filed #6136 rather than widening the PR.

class="editor"
/>
</div>
</template>

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.

When an item within a pair has duplicated content with distractors, the error message does not appear in the pair row.

Image

We should have another message below the other pairErrorMessage.

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 — the pair row now stacks every applicable message instead of showing the first match.

Audited the class rather than the line: all five codes the associate validator emits, against every control the editor reddens (prompt wrapper, pair card, distractor chip).

  • One gap: pairItemStyle reddened a pair card for DUPLICATE_DISTRACTOR_CONTENT, but pairErrorMessage never named it — and it returned one message where two can apply.
  • No gap on distractors: a blank distractor has no text to duplicate, so EMPTY_CHOICE_CONTENT and DUPLICATE_DISTRACTOR_CONTENT are mutually exclusive there.

pairErrorMessage(index) is now a pairErrorMessages computed returning an array; the row renders one ValidationMessage per entry. Two new tests: the message reaches the pair row the distractor repeats, and both messages stack on a pair that repeats itself and a distractor.

pair row with both messages

Comment on lines +26 to +37
<TipTapEditor
:value="state.prompt"
:mode="isPromptOpen ? mode : 'view'"
format="html"
:minHeight="'80px'"
:autofocus="isPromptOpen"
:imageProcessor="EditorImageProcessor"
:tabindex="-1"
class="editor"
@update="setPrompt"
@minimize="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.

When we have a long formula, its content overflows

Image Image Image

This does not happen on choice interaction

Image

But it does happen in the prompt editor of the text entry and ordering interactions.

Could you fix them all within the scope of this PR?

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 all three. .editor-content now clips at the card's inner edge — the bound .choice-content already gives choice's prompt. The toolbar and popovers are siblings of that box, so they stay whole.

Searched every TipTapEditor in the QTI interaction editors — 11 across choice, ordering, text entry and associate:

  • Matched: ordering's prompt, text entry's prompt, and all six in associate.
  • Already bounded: choice's two (.choice-content) and ordering's list items (.item-content).

Associate needed one more thing: its chips and distractor rows are flex items sized to content, so a wide formula grew the chip past the pool box rather than overflowing it. max-width: 100% on .chip / .distractor caps them.

Verified on the demo page with an oversized formula in the associate prompt, a pair item and a distractor, plus the ordering and text entry prompts.

Before:

before

After:

after

@rtibblesbot
rtibblesbot force-pushed the issue-6101-e6feda branch 2 times, most recently from f0e8e7f to 72ed32a Compare September 15, 2026 22:07
Comment on lines +615 to +617
::v-deep .editor-content {
overflow: hidden;
}

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 a default style within the TipTapEditor instead? Does anyone benefit from having it unbounded?

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.

Agreed — moved to EditorContentWrapper's own .editor-content rule (770fabe). Toolbars and popovers are siblings of that element, so clipping there leaves them whole.

Searched the branch for the same class — every consumer-side copy of a style that belongs to TipTapEditor. grep -rn "editor-content" frontend/ gave 7 hits across 4 files: 3 were this identical ::v-deep .editor-content { overflow: hidden } override (ordering, text entry, associate), all now deleted. The other 2 associate hits set per-chip padding, which is genuinely that card's layout, so they stay.

Kept min-width: 0 on .editor in those three: that one is about the editor being a flex item in their row, not about the editor itself.

Choice was never in the set — it bounds at .choice-content, a level above, which also clips the non-editor parts of the card. Left alone.

The five legacy exercise editors (AnswersEditor, HintsEditor, AssessmentItemEditor, AssessmentItemPreview, ResourceDrawer) pick up the bound for free; none of them reaches into .editor-content, so nothing there relied on the overflow.

Comment on lines +61 to +76
<!-- Keyed by position: a choice id may repeat across the pool. -->
<li
v-for="(choice, index) in shuffledPool"
:key="index"
class="chip"
:style="poolChipStyle(choice)"
>
<TipTapEditor
:value="choice.content"
mode="view"
format="html"
:imageProcessor="EditorImageProcessor"
:tabindex="-1"
class="editor"
/>
</li>

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'm still getting these de-duplicated. For example, I have these pairs. Test 2 and Test 1 should appear two times in the pool, but they only appear once.

Image 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 — the pool now renders one chip per occurrence it walks, so a choice used in two pairs shows twice, and so does a distractor carrying match-max="2". It was keying the pool into a Map by content first. The count of chips now equals the sum of the emitted match-max values.

Searched the branch for the same content-keyed collapse: three other sites key on choiceTextpairedTexts (green highlight in showAnswers), the duplicate-distractor error set, and the blank check. All three are membership tests that never decide what is rendered, so none changed. buildXML still folds equal content into one choice, which is exactly what the repeated chips represent.

Covered by a view-mode test on a fixture whose two pairs share one choice.

</div>

<ol
class="pairs-list"

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.

Some screen sizes show the TipTapEditor toolbar overflowing the container. Could you show the stacked view for screen sizes !windowIsLarge only when the editor of that row is opened? Let's first test how that feels, if it feels bad, we can have all of these stacked for !windowIsLarge.

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 — below windowIsLarge a pair row stacks only while its own editor is open; the rest of the rows stay side by side, and nothing stacks on a large screen. The row was previously stacked on windowIsSmall regardless of what was open, which is what left the toolbar overflowing between the two breakpoints.

Searched the other responsive layout switches in the QTI editor: four files (QuestionTypeSelector, TextEntryEditor, ChoiceInteractionEditor, QTIEditor/index.vue) branch on windowIsSmall. None of them puts two editors side by side in one row, so none has this failure mode and none changed.

Two tests cover it: stacked row follows the open editor, and no row stacks when large.

:class="{ 'chip is-tinted': !isDistractorOpen(index) }"
:style="distractorStyle(index, choice)"
>
<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.

Could we not set the border radius to the TipTapEditor? It makes the focus outline look too round.

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.

Removed — the chip's 8px radius now stops at the chip. The region wrapping the editor is square, so the focus ring it draws is square too.

Checked all eight ClickableRegions in the QTI interaction editors. The other seven wrap a card whose own visible border carries the 4px radius, so their focus ring traces a border that is actually there. This one was the only region whose radius existed solely to round the ring.

rtibblesbot and others added 5 commits September 16, 2026 07:14
Splits the single flat pool of <qti-simple-associable-choice> elements into
authoring state: `pairs` from the correct response, `distractors` from the
match-max capacity the correct response does not consume. buildXML re-merges
them, normalizing ids so equal content shares one pool entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registering the descriptor does not populate QUESTION_TYPE_LABELS, so
QTIItemEditor gets an explicit ASSOCIATE entry — without it every associate
item's view-mode header reads "Unknown type".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A formula or a long URL is one unbreakable inline node, so it paints over
whatever bounds the editor. The clip goes on the editor's own content wrapper,
so every editor embedding it is bounded — the legacy exercise editors included.

`.editor` needs `min-width: 0` to shrink below that content in a flex row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
// Two cards side by side leave an open editor too narrow to hold its
// toolbar on anything but a large screen, so the row being edited stacks.
function isPairRowStacked(index) {
return !windowIsLarge.value && isPairRowOpen(index);

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 windowIsSmall these should always be stacked.

rtibblesbot and others added 2 commits September 16, 2026 08:59
grey.v_400 on the pair row's #fafafa is 2.73:1, under the 3:1 WCAG 1.4.11
minimum for a UI component. The distractor delete alongside it and the
ordering editor's both use grey.v_700 (5.50:1).

Follow the ordering editor the rest of the way: textDisabled when it is the
only pair, behind an isOnlyPair computed that also carries :disabled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two cards side by side leave each too narrow to read below 600px, so the
row stacks whether or not its editor is open. A stacked view-mode chip
keeps its content width instead of stretching to the row.

Co-Authored-By: Claude Opus 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 Associate Interaction editor

2 participants