Skip to content

Drag-and-drop reordering in the QTI choice and ordering editors - #6109

Merged
AlexVelezLl merged 9 commits into
learningequality:unstablefrom
rtibblesbot:issue-6106-95187d
Sep 1, 2026
Merged

Drag-and-drop reordering in the QTI choice and ordering editors#6109
AlexVelezLl merged 9 commits into
learningequality:unstablefrom
rtibblesbot:issue-6106-95187d

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Reordering a choice or ordering row took one chevron click per position, from a toolbar at the row's end. Both editors now reorder by dragging a handle at the row's start, backed by a copy of Kolibri's draggable package as shared/views/dragSort/. The choice editor hides the handle when shuffle is on. Delete stays as its own icon button at the end, so neither editor uses CollapsibleToolbar.

The copy keeps Kolibri's filenames and layout so it stays diffable. Edits to it: Studio's i18n, strings and logging; a color prop on DragSortWidget; opacity: 0 on the dragged row's ghost, since visibility: hidden alone left the TipTap toolbar painted over the list. sortablejs is new in package.json, at Kolibri's version.

Review follow-up also lines the handle, selection control and error icon up on one 24px box, moves an invalid choice's error icon into the selection control's place and turns the invalid ordering row's badge red, drops the row icons to grey.v_700 (which is why TextEntryEditor is in the diff), and makes a row opaque while it is dragged.

References

Fixes #6106. Source: learningequality/kolibri packages/kolibri-common/components/draggable/ at e5eb4ddbcc85e61ec03075e1a7b7d477455c182c.

Reviewer guidance

Open a channel for editing, go to /channels/<channel_id>/#/qti-demo. Questions 1-2 use the choice editor, question 6 the ordering editor.

  1. Drag a row by its grip — the list settles there, badges renumber, and the row does not open for editing.
  2. Tab to a grip — chevrons replace it. Move the row; focus follows it. First row has no move-up, last no move-down.
  3. With a screen reader, move a row — "Choice 2 moved to position 3 of 4".
  4. Switch to Arabic, repeat step 1 — grip at the right edge, delete at the left.

Worth questioning:

  • The grip has no label of its own; the accessible name comes from DragSortWidget's two move buttons, as in Kolibri. Labelling both announces the affordance twice.
  • shared/views/dragSort/__tests__/ uses @vue/test-utils, deprecated per AGENTS.md. Copied with the package, and the only cover over the SortableJS reconciliation.
  • The demo page does not persist, so reorder-then-reload is unexercised. setChoiceOrder/setItemOrder are asserted to emit bodyXml byte-identical to the equivalent chevron move.

Screenshots

Editor Before After
Choice Choice before Choice after
Ordering Ordering before Ordering after
Choice, RTL Choice RTL
Choice editor Ordering editor

axe-core AA reports one violation with both editors open: contrast on the untouched AddListItemButton.

AI usage

Used Claude Code to port the package, wire both editors, and write the tests. Verified with the Jest suite, pre-commit run --all-files, a file-for-file diff against Kolibri, and browser QA of drag, keyboard move, announcement and RTL.


@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-01 13:01 UTC

From learningequality/kolibri packages/kolibri-common/components/draggable
at e5eb4ddbcc85e61ec03075e1a7b7d477455c182c. Filenames and layout kept so the
copy stays diffable against the source.

Deviations, all module swaps:
- dragSortStrings.js: createTranslator from shared/i18n
- renderSlotRoot.js: shared/logging, which exposes only error()
- DragSortWidget/index.vue: shared/strings/commonStrings, shared/i18n

The four direction labels are copied verbatim from Kolibri's
commonCoreStrings so both stay translatable in step.

shared/views/draggable/ is Studio's older, unrelated system and shares three
filenames with this package — hence the separate directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rtibblesbot
rtibblesbot force-pushed the issue-6106-95187d branch 2 times, most recently from 9417a85 to 2665786 Compare August 26, 2026 21:45
@rtibblesbot
rtibblesbot marked this pull request as ready for review August 26, 2026 21:55
@rtibblesbot
rtibblesbot force-pushed the issue-6106-95187d branch 3 times, most recently from c1ce11d to 8c320a4 Compare August 26, 2026 22:43
@AlexVelezLl

Copy link
Copy Markdown
Member

@rtibblesbot, from the screenshots you added, I can see the drag handler icon is not properly vertically aligned with the radio button and the remove button

@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 implementation overall, just found a couple of minor details.

interaction: block(CHOICE_SINGLE_SELECT_XML),
questionType: QuestionType.SINGLE_SELECT,
});
await fireEvent.click(screen.getByRole('button', { name: moveDownName(1) }));

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

Switched to userEvent.setup(). This PR adds two fireEvent.click calls — this one and the ordering equivalent — and both are converted; the new error-icon test below also uses userEvent. The remaining fireEvent calls in these files predate the PR, so I left them.

interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML),
questionType: QuestionType.ORDERING,
});
await fireEvent.click(screen.getByRole('button', { name: moveDownName(1) }));

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.

idem

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, same as the choice one.

<div
class="position-badge"
:style="{
backgroundColor: $themePalette.green.v_100,

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 a palette.red.v_100 background if there is an error, and remove the color style, please?

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 — red.v_100 background when the item is invalid, and the color override is gone so the badge inherits the row text colour.

'is-open': isChoiceOpen(choice.id),
'small-screen': windowIsSmall,
}"
<div class="choice-group">

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 a surface background for these choices so that it has a better constrast when dragging and hovering on top of other options?

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. Applied as background-color: $themeTokens.surface on .choice-border in the style block rather than inline, so the :hover rule still wins (verified in the browser: hovering a row still paints fineLine over it). Ordering rows have the same overlap problem while dragging, so .item-border gets it too.

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

Almost there!

Comment on lines +25 to +31
// `visibility` alone is not enough: a descendant can set `visibility: visible` on
// itself and stay painted over the list — KListWithOverflow does exactly that for
// the TipTap toolbar buttons. `opacity` cannot be overridden from inside.
.draggable-item--ghost {
visibility: hidden;
opacity: 0;
}

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 open a PR in Kolibri to add this (and the color attribute) to its implementation?

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.

This account has no push access to learningequality/kolibri, so I filed learningequality/kolibri#15255 there instead — it covers both the ghost opacity and the color prop. (Its issue type is unset: UpdateIssueIssueType is denied for this account.)

}"
>
<!-- `@click.stop` so using the handle does not open the choice for editing -->
<DraggableHandle v-if="mode === 'edit'">

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 shuffle is true, we should also remove the handler because it does not make sense to reorder the items.

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 handle and the region's sortable now both come off isReorderable (mode === 'edit' && !state.shuffle), so shuffling drops the grip and the keyboard move buttons together.

Searched the branch for every DraggableHandle and :sortable binding: two, this editor and OrderingInteractionEditor. Changed this one only. The ordering editor's shuffle is always true and has no author toggle — its list is the answer key, which is what "Learners will see these shuffled" above it says — so dropping its handle would remove reordering outright.

rtibblesbot and others added 3 commits September 1, 2026 05:48
The grip and chevrons take KIcon's default otherwise, which is darker than
the radio and checkbox the QTI editor rows put them next to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`.draggable-item--ghost` relied on `visibility: hidden`, which KListWithOverflow
undoes for the toolbar buttons it sizes — they stayed painted over the list while
the row was dragged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DraggableRegion emits the whole reordered array; the editors reduce it to an
id order so no readonly state proxy travels back into the composable and the
reorder stays expressible as a permutation of what already exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rtibblesbot and others added 4 commits September 1, 2026 05:56
The move-up/move-down chevrons go; DragSortWidget's own move buttons, revealed
when the handle takes focus, keep reordering keyboard-operable. Delete becomes
a standalone icon button, so the row no longer needs CollapsibleToolbar.

Shuffled choices reach the learner in a random order, so with shuffle on the
row drops its handle and the region stops sorting.

DraggableRegion is keyed on the select mode: single-select renders the list as
a KRadioButtonGroup and multi-select as a plain div, and SortableJS binds to
that element once, on mount.

The card gets a surface background so a dragged row stays opaque over the rows
beneath it, and the selection control is pulled onto the row's centre line so
it sits level with the handle.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same shape as the choice editor: drag handle at the start of the row,
DragSortWidget's move buttons for the keyboard, delete as a standalone icon
button in place of CollapsibleToolbar. The card gets a surface background so a
dragged row stays opaque over the rows beneath it.

windowIsSmall was read only by the toolbar's collapse rule, so
useKResponsiveWindow goes with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The choice editor puts the error icon where the radio or checkbox would be,
rather than beside it; the ordering editor's position badge turns red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches the other QTI editors' row icons.

Co-Authored-By: Claude Opus 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.

LGTM!

@AlexVelezLl
AlexVelezLl merged commit 655e57e into learningequality:unstable Sep 1, 2026
14 checks passed
@rtibblesbot
rtibblesbot deleted the issue-6106-95187d branch September 1, 2026 14:52
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] Replace the reorder chevrons with drag and drop in the choice and ordering interaction editors

2 participants