Drag-and-drop reordering in the QTI choice and ordering editors by rtibblesbot · Pull Request #6109 · learningequality/studio

rtibblesbot · 2026-08-26T21:17:49Z

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/. 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; the only edits swap in Studio's i18n, strings and logging. 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, 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.

Drag a row by its grip — the list settles there, badges renumber, and the row does not open for editing.
Tab to a grip — chevrons replace it. Move the row; focus follows it. First row has no move-up, last no move-down.
With a screen reader, move a row — "Choice 2 moved to position 3 of 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
Ordering
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

	🟡 Waiting for feedback Last updated: 2026-08-31 23:13 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>

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>

AlexVelezLl · 2026-08-26T23:18:30Z

@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

Good implementation overall, just found a couple of minor details.

AlexVelezLl · 2026-08-31T21:54:59Z

+        makeAnswer({ id: 'b', content: 'B' }),
+        makeAnswer({ id: 'c', content: 'C' }),
+      ];
+      const viaChevron = setup(choices);


Why don't we destructure here too?

Done. Grepped = setup( across both composable spec files: 4 non-destructured call sites (2 here, 2 in useOrderingInteraction.spec.js), all 4 destructured.

AlexVelezLl · 2026-08-31T21:55:24Z

+    });
+
+    it('produces the same bodyXml as moveItemUp for the equivalent move', () => {
+      const viaChevron = setup();


let's use destructuring instead

Done — destructured here and in the matching useChoiceInteraction.spec.js test; those were the only 4 non-destructured setup() calls in the two files.

AlexVelezLl · 2026-08-31T22:17:45Z

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


Lets use userEvent.setup() instead.

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.

AlexVelezLl · 2026-08-31T22:24:58Z

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


Done, same as the choice one.

AlexVelezLl · 2026-08-31T22:28:53Z

+                    <DraggableHandle v-if="mode === 'edit'">
+                      <div
+                        class="choice-drag"
+                        @click.stop
+                      >
+                        <DragSortWidget
+                          :isFirst="index === 0"
+                          :isLast="index === state.choices.length - 1"
+                          :itemLabel="choiceItemLabel$({ number: index + 1 })"
+                          :position="index + 1"
+                          :total="state.choices.length"
+                          @moveUp="moveChoiceUp(choice.id)"
+                          @moveDown="moveChoiceDown(choice.id)"
+                        />
+                      </div>
+                    </DraggableHandle>
+
+                    <!-- Selection control -->
+                    <div
+                      class="choice-selection"
+                      @click.stop
+                    >
+                      <KIcon
+                        v-if="choiceHasError(choice.id)"
+                        icon="error"
+                        :color="$themeTokens.error"
+                        :style="{
+                          width: '20px',
+                          height: '20px',
+                          marginTop: '2px',
+                          marginRight: '4px',
+                        }"
+                      />
+                      <KRadioButton
+                        v-if="isSingleSelect"
+                        :currentValue="correctChoiceId || ''"
+                        :buttonValue="choice.id"
+                        :label="markCorrectLabel$()"
+                        :showLabel="false"
+                        :disabled="mode !== 'edit'"
+                        :style="{ width: 'auto' }"
+                        :color="$themePalette.green.v_600"
+                        @change="onToggleCorrect(choice.id)"
+                      />


Could you make sure both the radio button and the drag handle are properly aligned?

Fixed. KRadioButton/KCheckbox wrap their 24px control in a table with 8px block margins, and the inline icon adds descender space below it — together that put the control 2.5px above the row centre when closed and 8px below the handle when open. .choice-selection now zeroes both, so handle, control and error icon are all one 24px box. Measured in the browser: handle/control centre offset is 0px for single-select and multi-select, in both the open and the closed row.

AlexVelezLl · 2026-08-31T22:32:03Z

+                      <KIcon
+                        v-if="choiceHasError(choice.id)"
+                        icon="error"
+                        :color="$themeTokens.error"
+                        :style="{
+                          width: '20px',
+                          height: '20px',
+                          marginTop: '2px',
+                          marginRight: '4px',
+                        }"
+                      />
+                      <KRadioButton
+                        v-if="isSingleSelect"
+                        :currentValue="correctChoiceId || ''"
+                        :buttonValue="choice.id"
+                        :label="markCorrectLabel$()"
+                        :showLabel="false"
+                        :disabled="mode !== 'edit'"
+                        :style="{ width: 'auto' }"
+                        :color="$themePalette.green.v_600"
+                        @change="onToggleCorrect(choice.id)"


One little change here, I know this isn't introduced here, but could you please make this change: If there is an error, use the error icon in the same place as the radio button/checkbox like this.

Done — the error icon now replaces the radio/checkbox rather than sitting beside it, and inherits the same 24px slot. One consequence worth flagging: while a choice is invalid it has no selection control, so it cannot be marked correct until its content is fixed. Added a test asserting the control is gone on an invalid row.

AlexVelezLl · 2026-08-31T22:32:38Z

+                        <DragSortWidget
+                          :isFirst="index === 0"
+                          :isLast="index === state.choices.length - 1"
+                          :itemLabel="choiceItemLabel$({ number: index + 1 })"
+                          :position="index + 1"
+                          :total="state.choices.length"
+                          @moveUp="moveChoiceUp(choice.id)"
+                          @moveDown="moveChoiceDown(choice.id)"
+                        />


Could we use palette.grey.v_700 for the icon used for this widget so that it matches the color we use for the radio button and checkbox? Change this color on the remove button, and also for the ordering interaction and text entry interaction.

Done. Grepped grey.v_800 across the three interaction editors: 4 hits — the remove button in choice, ordering and text entry, plus the ordering position badge. The three remove buttons are now grey.v_700; the badge's color is dropped entirely per your other comment. The two hits in CollapsibleToolbar/QTIItemEditor are the item toolbar rather than these row widgets, so I left them.

DragSortWidget gained a color prop to carry this through — it is the one behavioural deviation from the Kolibri copy, since KIcon writes fill inline and cannot be overridden from a parent stylesheet.

AlexVelezLl · 2026-08-31T22:37:23Z

+                    </div>
+
+                    <div class="choice-content">
+                      <TipTapEditor


For some reason, when we drag the option while the Tiptap editor is open, the Tiptap editor toolbar stays in the background. Could you fix this, please?

Grabacion.de.pantalla.2026-08-31.a.la.s.5.35.17.p.m.mov

(This also happens on the ordering interaction)

Fixed. .draggable-item--ghost hid the dragged row with visibility: hidden, but KListWithOverflow writes visibility: visible inline on the toolbar groups it measures, which wins over an inherited hidden. Confirmed in the browser: 43 descendants of the ghost row stayed visible, all under a div.toolbar-group with that inline style. The ghost now also sets opacity: 0, which a descendant cannot undo. One shared stylesheet, so this covers the ordering interaction too.

AlexVelezLl · 2026-08-31T22:42:31Z

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


Could we use a palette.red.v_100 background if there is an error, and remove the color style, please?

Done — red.v_100 background when the item is invalid, and the color override is gone so the badge inherits the row text colour.

AlexVelezLl · 2026-08-31T22:45:43Z

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


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?

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.

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

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>

- Handle, radio/checkbox and error icon now share one 24px box, so they line up in both the open and the closed row. - An invalid choice shows the error icon in place of its selection control. - Handles and remove buttons use grey.v_700, like the radio and checkbox. - Choice and item cards get a surface background so a dragged row is opaque. - The ordering position badge turns red.v_100 when the item is invalid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rtibblesbot and others added 2 commits August 26, 2026 14:01

rtibblesbot force-pushed the issue-6106-95187d branch 2 times, most recently from 9417a85 to 2665786 Compare August 26, 2026 21:45

rtibblesbot marked this pull request as ready for review August 26, 2026 21:55

rtibblesbot force-pushed the issue-6106-95187d branch 3 times, most recently from c1ce11d to 8c320a4 Compare August 26, 2026 22:43

AlexVelezLl requested changes Aug 31, 2026

View reviewed changes

rtibblesbot and others added 5 commits August 31, 2026 16:11

Use destructuring and userEvent.setup() in the reorder tests

da436b0

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rtibblesbot force-pushed the issue-6106-95187d branch from 22a65ca to 9814af0 Compare August 31, 2026 23:11

Uh oh!

Uh oh!

Conversation

rtibblesbot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

References

Reviewer guidance

Screenshots

AI usage

🟡 Waiting for feedback

Uh oh!

AlexVelezLl commented Aug 26, 2026

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rtibblesbot commented Aug 26, 2026 •

edited

Loading