From 723a5fdbb56f53822be34fe298894f01e7da23a9 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 26 Aug 2026 14:01:44 -0700 Subject: [PATCH 1/9] Copy Kolibri's draggable package into shared/views/dragSort/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../frontend/shared/strings/commonStrings.js | 16 + .../views/dragSort/DragSortWidget/index.vue | 288 +++++++++++ .../shared/views/dragSort/DraggableHandle.vue | 15 + .../shared/views/dragSort/DraggableItem.vue | 27 ++ .../shared/views/dragSort/DraggableRegion.vue | 65 +++ .../views/dragSort/DraggableUniverse.vue | 39 ++ .../dragSort/__tests__/DragSortWidget.spec.js | 68 +++ .../dragSort/__tests__/DraggableItem.spec.js | 64 +++ .../__tests__/DraggableRegion.spec.js | 459 ++++++++++++++++++ .../__tests__/DraggableUniverse.spec.js | 80 +++ .../shared/views/dragSort/classDefinitions.js | 16 + .../shared/views/dragSort/domUtils.js | 23 + .../shared/views/dragSort/dragSortStrings.js | 34 ++ .../shared/views/dragSort/draggable.scss | 55 +++ .../shared/views/dragSort/renderSlotRoot.js | 34 ++ .../views/dragSort/useDraggableRegion.js | 249 ++++++++++ .../views/dragSort/useDraggableUniverse.js | 103 ++++ package.json | 1 + pnpm-lock.yaml | 8 + 19 files changed, 1644 insertions(+) create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableHandle.vue create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableItem.vue create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableRegion.vue create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableUniverse.vue create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DragSortWidget.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableItem.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableRegion.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableUniverse.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/classDefinitions.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/domUtils.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/dragSortStrings.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/renderSlotRoot.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableRegion.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableUniverse.js diff --git a/contentcuration/contentcuration/frontend/shared/strings/commonStrings.js b/contentcuration/contentcuration/frontend/shared/strings/commonStrings.js index 0a1fb9161b..8df5ae95c6 100644 --- a/contentcuration/contentcuration/frontend/shared/strings/commonStrings.js +++ b/contentcuration/contentcuration/frontend/shared/strings/commonStrings.js @@ -82,4 +82,20 @@ export const commonStrings = createTranslator('CommonStrings', { message: 'Removed {label}', context: 'Announced when an option is removed. {label} is the name of the option', }, + moveUpLabel: { + message: 'Move up', + context: 'Label for the button that moves a resource up in the list. Not visible in the UI.', + }, + moveDownLabel: { + message: 'Move down', + context: 'Label for the button that moves a resource down in the list. Not visible in the UI.', + }, + moveLeftLabel: { + message: 'Move left', + context: 'Label for the button that moves a resource left in the list. Not visible in the UI.', + }, + moveRightLabel: { + message: 'Move right', + context: 'Label for the button that moves a resource right in the list. Not visible in the UI.', + }, }); diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue b/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue new file mode 100644 index 0000000000..b1371487f4 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue @@ -0,0 +1,288 @@ + + + + + + + diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableHandle.vue b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableHandle.vue new file mode 100644 index 0000000000..c51c704fd4 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableHandle.vue @@ -0,0 +1,15 @@ + diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableItem.vue b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableItem.vue new file mode 100644 index 0000000000..9c1be951af --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableItem.vue @@ -0,0 +1,27 @@ + diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableRegion.vue b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableRegion.vue new file mode 100644 index 0000000000..9a7cec23a7 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableRegion.vue @@ -0,0 +1,65 @@ + + + + + diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableUniverse.vue b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableUniverse.vue new file mode 100644 index 0000000000..9bc9faea33 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/DraggableUniverse.vue @@ -0,0 +1,39 @@ + diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DragSortWidget.spec.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DragSortWidget.spec.js new file mode 100644 index 0000000000..82de028047 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DragSortWidget.spec.js @@ -0,0 +1,68 @@ +import { mount } from '@vue/test-utils'; +import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion'; +import DragSortWidget from '../DragSortWidget/index.vue'; + +jest.mock('kolibri-design-system/lib/composables/useKLiveRegion'); + +describe('DragSortWidget', () => { + let sendPoliteMessage; + + beforeEach(() => { + sendPoliteMessage = jest.fn(); + useKLiveRegion.mockReturnValue({ sendPoliteMessage }); + }); + + function makeWrapper(propsData = {}) { + return mount(DragSortWidget, { + propsData: { + isFirst: false, + isLast: false, + ...propsData, + }, + }); + } + + describe('move-button announcements', () => { + it('announces the item and its new position on move up', async () => { + const wrapper = makeWrapper({ itemLabel: 'Rubens Barrichello', position: 2, total: 3 }); + await wrapper.findComponent({ ref: 'upBtn' }).vm.$emit('click'); + expect(sendPoliteMessage).toHaveBeenCalledWith('Rubens Barrichello moved to position 1 of 3'); + }); + + it('announces the item and its new position on move down', async () => { + const wrapper = makeWrapper({ itemLabel: 'Rubens Barrichello', position: 2, total: 3 }); + await wrapper.findComponent({ ref: 'dnBtn' }).vm.$emit('click'); + expect(sendPoliteMessage).toHaveBeenCalledWith('Rubens Barrichello moved to position 3 of 3'); + }); + + it('does not announce when itemLabel is not provided', async () => { + const wrapper = makeWrapper({ position: 2, total: 3 }); + await wrapper.findComponent({ ref: 'upBtn' }).vm.$emit('click'); + expect(sendPoliteMessage).not.toHaveBeenCalled(); + }); + + it('does not announce when total is not provided', async () => { + const wrapper = makeWrapper({ itemLabel: 'Rubens Barrichello', position: 2 }); + await wrapper.findComponent({ ref: 'upBtn' }).vm.$emit('click'); + expect(sendPoliteMessage).not.toHaveBeenCalled(); + }); + }); + + describe('item-specific aria-labels', () => { + it('uses the item label in vertical mode', () => { + const wrapper = makeWrapper({ itemLabel: 'Rubens Barrichello', position: 2, total: 3 }); + expect(wrapper.findComponent({ ref: 'upBtn' }).props('ariaLabel')).toBe( + 'Move Rubens Barrichello up', + ); + expect(wrapper.findComponent({ ref: 'dnBtn' }).props('ariaLabel')).toBe( + 'Move Rubens Barrichello down', + ); + }); + + it('falls back to the generic label when no itemLabel is given', () => { + const wrapper = makeWrapper(); + expect(wrapper.findComponent({ ref: 'upBtn' }).props('ariaLabel')).toBe('Move up'); + expect(wrapper.findComponent({ ref: 'dnBtn' }).props('ariaLabel')).toBe('Move down'); + }); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableItem.spec.js new file mode 100644 index 0000000000..e458ebacb8 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableItem.spec.js @@ -0,0 +1,64 @@ +import { mount } from '@vue/test-utils'; +import DraggableItem from '../DraggableItem.vue'; +import DraggableHandle from '../DraggableHandle.vue'; +import { ITEM_CLASS, HANDLE_CLASS, DISABLED_CLASS } from '../classDefinitions'; + +// The slot's own element is what gets marked, so every mount supplies one. +const slots = { default: '
' }; + +describe('DraggableItem', () => { + it('marks the element the consumer wrote instead of wrapping it', () => { + const wrapper = mount(DraggableItem, { slots: { default: '
  • row
    • ' } }); + expect(wrapper.element.tagName).toBe('LI'); + expect(wrapper.classes()).toContain(ITEM_CLASS); + }); + + it('keeps the classes that element already had', () => { + const wrapper = mount(DraggableItem, { slots: { default: '
    ' } }); + expect(wrapper.classes()).toContain(ITEM_CLASS); + expect(wrapper.classes()).toContain('my-row'); + }); + + it('merges a class bound on the component onto that same element', () => { + const host = mount({ + components: { DraggableItem }, + template: `
    `, + }); + const item = host.findComponent(DraggableItem); + expect(item.classes()).toContain(ITEM_CLASS); + expect(item.classes()).toContain('my-row'); + }); + + it('adds the disabled class only when disabled', () => { + expect(mount(DraggableItem, { slots }).classes()).not.toContain(DISABLED_CLASS); + expect(mount(DraggableItem, { propsData: { disabled: true }, slots }).classes()).toContain( + DISABLED_CLASS, + ); + }); + + it('forwards attributes to that element', () => { + const wrapper = mount(DraggableItem, { attrs: { tabindex: '-1' }, slots }); + expect(wrapper.attributes('tabindex')).toBe('-1'); + }); + + it('renders its slot content', () => { + const wrapper = mount(DraggableItem, { slots: { default: '
    hello
    ' } }); + expect(wrapper.text()).toBe('hello'); + }); +}); + +describe('DraggableHandle', () => { + it('marks the element the consumer wrote instead of wrapping it', () => { + const wrapper = mount(DraggableHandle, { slots: { default: 'grip' } }); + expect(wrapper.element.tagName).toBe('SPAN'); + expect(wrapper.classes()).toContain(HANDLE_CLASS); + }); + + it('marks the root element of a component in its slot', () => { + const Grip = { name: 'Grip', template: '' }; + const wrapper = mount(DraggableHandle, { slots: { default: Grip } }); + expect(wrapper.element.tagName).toBe('BUTTON'); + expect(wrapper.classes()).toContain(HANDLE_CLASS); + expect(wrapper.classes()).toContain('grip'); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableRegion.spec.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableRegion.spec.js new file mode 100644 index 0000000000..762c97d470 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableRegion.spec.js @@ -0,0 +1,459 @@ +import { mount } from '@vue/test-utils'; +import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion'; +import DraggableUniverse from '../DraggableUniverse.vue'; +import DraggableRegion from '../DraggableRegion.vue'; +import { ITEM_CLASS } from '../classDefinitions'; + +jest.mock('kolibri-design-system/lib/composables/useKLiveRegion'); + +// Real SortableJS drives pointer events jsdom can't produce; we only need the +// options object it's constructed with, so we can drive the region's own +// lifecycle callbacks (onStart / onEnd / group.put) against real jsdom nodes. +let mockInstances; +jest.mock('sortablejs', () => + jest.fn().mockImplementation((el, options) => { + const instance = { el, options, option: jest.fn(), destroy: jest.fn() }; + mockInstances.push(instance); + return instance; + }), +); + +// A row element carrying the draggable marker class, so insertNodeAt has real +// children to index into. +function row(text) { + const el = document.createElement('div'); + el.className = ITEM_CLASS; + el.textContent = text; + return el; +} + +describe('DraggableRegion', () => { + let sendPoliteMessage; + + beforeEach(() => { + mockInstances = []; + sendPoliteMessage = jest.fn(); + useKLiveRegion.mockReturnValue({ sendPoliteMessage }); + document.hasFocus = jest.fn(() => true); + }); + + // Mounts a lone region and returns its captured Sortable options. The region marks + // the element written inside it rather than rendering one of its own. + async function mountRegion(propsData = {}, mountOptions = {}) { + const wrapper = mount(DraggableRegion, { + propsData: { items: [{ id: 'a' }, { id: 'b' }, { id: 'c' }], ...propsData }, + slots: { default: '
    ' }, + ...mountOptions, + }); + await wrapper.vm.$nextTick(); + const instance = mockInstances[mockInstances.length - 1]; + return { wrapper, options: instance.options, sortable: instance }; + } + + it('sorts the element written inside it, rather than one of its own', async () => { + const { wrapper, sortable } = await mountRegion({}, { slots: { default: '
      ' } }); + expect(wrapper.element.tagName).toBe('UL'); + expect(sortable.el).toBe(wrapper.element); + }); + + it('confines the press-and-hold delay to touch, so a mouse drag is not swallowed', async () => { + const { options } = await mountRegion(); + expect(options.delayOnTouchOnly).toBe(true); + }); + + describe('capacity (group.put)', () => { + it('accepts a drop while below capacity and rejects it once full', async () => { + const { options } = await mountRegion({ items: [{ id: 'a' }], capacity: 2 }); + expect(options.group.put()).toBe(true); + const { options: full } = await mountRegion({ + items: [{ id: 'a' }, { id: 'b' }], + capacity: 2, + }); + expect(full.group.put()).toBe(false); + }); + + it('never rejects when capacity is null (unlimited)', async () => { + const { options } = await mountRegion({ items: [{ id: 'a' }, { id: 'b' }], capacity: null }); + expect(options.group.put()).toBe(true); + }); + + it('rejects every drop when disabled', async () => { + const { options } = await mountRegion({ items: [], capacity: 5, disabled: true }); + expect(options.group.put()).toBe(false); + }); + + it('rejects when the accepts predicate returns false, even below capacity', async () => { + const { options } = await mountRegion({ + items: [{ id: 'a' }], + capacity: 5, + accepts: () => false, + }); + expect(options.group.put()).toBe(false); + }); + }); + + describe('options that change after mount', () => { + it('pushes a flipped sortable prop into the SortableJS instance', async () => { + const { wrapper, sortable } = await mountRegion({ sortable: true }); + await wrapper.setProps({ sortable: false }); + expect(sortable.option).toHaveBeenCalledWith('sort', false); + }); + + it('pushes a flipped clone prop into the SortableJS instance', async () => { + const { wrapper, sortable } = await mountRegion({ clone: false }); + await wrapper.setProps({ clone: true }); + expect(sortable.option).toHaveBeenCalledWith( + 'group', + expect.objectContaining({ pull: 'clone' }), + ); + }); + + it("pushes the universe's changed delay into the SortableJS instance", async () => { + const wrapper = mount({ + components: { DraggableUniverse, DraggableRegion }, + data() { + return { delay: 250, items: [{ id: 'a' }] }; + }, + template: ` + +
      +
      +
      + + `, + }); + await wrapper.vm.$nextTick(); + const sortable = mockInstances[mockInstances.length - 1]; + expect(sortable.options.delay).toBe(250); + + await wrapper.setData({ delay: 0 }); + expect(sortable.option).toHaveBeenCalledWith('delay', 0); + }); + }); + + describe('reorder within a region', () => { + it('emits the reordered array on a same-region move', async () => { + const items = [{ id: 'a' }, { id: 'b' }, { id: 'c' }]; + const { wrapper, options } = await mountRegion({ items }); + const from = wrapper.element; + [row('a'), row('b'), row('c')].forEach(r => from.appendChild(r)); + const item = from.children[0]; + + options.onEnd({ + item, + from, + to: from, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 2, + }); + + const emitted = wrapper.emitted('update:items'); + expect(emitted).toHaveLength(1); + expect(emitted[0][0].map(i => i.id)).toEqual(['b', 'c', 'a']); + }); + + it('emits nothing for a no-op drag (same index)', async () => { + const { wrapper, options } = await mountRegion(); + const from = wrapper.element; + from.appendChild(row('a')); + options.onEnd({ + item: from.children[0], + from, + to: from, + oldIndex: 0, + oldDraggableIndex: 1, + newDraggableIndex: 1, + }); + expect(wrapper.emitted('update:items')).toBeUndefined(); + }); + + it('reverts the DOM so the moved node is back under its source at its old index', async () => { + const { wrapper, options } = await mountRegion(); + const from = wrapper.element; + [row('a'), row('b')].forEach(r => from.appendChild(r)); + const item = from.children[0]; + options.onEnd({ + item, + from, + to: from, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 1, + }); + expect(item.parentElement).toBe(from); + expect(from.children[0]).toBe(item); + }); + }); + + // Two regions inside one universe so they share a SortableJS group and registry. + async function mountUniverse(targetProps = {}, { sourceProps = {}, sourceItems } = {}) { + const wrapper = mount({ + components: { DraggableUniverse, DraggableRegion }, + data() { + return { + source: sourceItems || [{ id: 'a' }, { id: 'b' }], + target: [{ id: 'x' }], + sourceProps, + targetProps, + }; + }, + template: ` + +
      + +
      + + +
      + +
      + + `, + }); + await wrapper.vm.$nextTick(); + const regions = wrapper.findAllComponents({ name: 'DraggableRegion' }); + return { + wrapper, + sourceRegion: regions.at(0), + targetRegion: regions.at(1), + sourceOptions: mockInstances[0].options, + sourceEl: regions.at(0).element, + targetEl: regions.at(1).element, + }; + } + + describe('cross-region move', () => { + it('moves an item: source loses it, target gains it at the drop index', async () => { + const { sourceRegion, targetRegion, sourceOptions, sourceEl, targetEl } = + await mountUniverse(); + sourceEl.appendChild(row('a')); + sourceEl.appendChild(row('b')); + const item = sourceEl.children[0]; + + sourceOptions.onStart({ oldDraggableIndex: 0 }); + sourceOptions.onEnd({ + item, + from: sourceEl, + to: targetEl, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 1, + }); + + expect(sourceRegion.emitted('update:items')[0][0].map(i => i.id)).toEqual(['b']); + expect(targetRegion.emitted('update:items')[0][0].map(i => i.id)).toEqual(['x', 'a']); + }); + + // SortableJS builds a clone node for every drag, whether or not it is shown + it('removes the clone node SortableJS left in the source region', async () => { + const { sourceOptions, sourceEl, targetEl } = await mountUniverse(); + sourceEl.appendChild(row('a')); + sourceEl.appendChild(row('b')); + const item = sourceEl.children[0]; + const clone = row('a-clone'); + sourceEl.appendChild(clone); + + sourceOptions.onStart({ oldDraggableIndex: 0 }); + sourceOptions.onEnd({ + item, + clone, + from: sourceEl, + to: targetEl, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 0, + }); + + expect(clone.parentNode).toBeNull(); + }); + + it('announces the drop when the target region has a label', async () => { + const { sourceOptions, sourceEl, targetEl } = await mountUniverse({ label: 'Gap 1' }); + sourceEl.appendChild(row('a')); + sourceEl.appendChild(row('b')); + sourceOptions.onStart({ oldDraggableIndex: 0 }); + sourceOptions.onEnd({ + item: sourceEl.children[0], + from: sourceEl, + to: targetEl, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 0, + }); + expect(sendPoliteMessage).toHaveBeenCalledWith('Moved to Gap 1'); + }); + + it('leaves data untouched when dropped outside the universe', async () => { + const { sourceRegion, sourceOptions, sourceEl } = await mountUniverse(); + sourceEl.appendChild(row('a')); + sourceEl.appendChild(row('b')); + const stray = document.createElement('div'); + sourceOptions.onStart({ oldDraggableIndex: 0 }); + sourceOptions.onEnd({ + item: sourceEl.children[0], + from: sourceEl, + to: stray, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 0, + }); + expect(sourceRegion.emitted('update:items')).toBeUndefined(); + }); + }); + + describe('cross-region clone', () => { + // Drags the first source item onto the front of the target region, as a clone. + async function dropClone({ sourceProps, sourceItems } = {}) { + const context = await mountUniverse( + {}, + { sourceProps: sourceProps || { clone: true }, sourceItems }, + ); + const { sourceOptions, sourceEl, targetEl } = context; + sourceEl.appendChild(row('a')); + sourceEl.appendChild(row('b')); + sourceOptions.onStart({ oldDraggableIndex: 0 }); + sourceOptions.onEnd({ + item: sourceEl.children[0], + from: sourceEl, + to: targetEl, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: 0, + pullMode: 'clone', + }); + return context; + } + + it('sets pull to clone when the clone prop is set', async () => { + const { options } = await mountRegion({ clone: true }); + expect(options.group.pull).toBe('clone'); + }); + + it('leaves the source array untouched', async () => { + const { sourceRegion } = await dropClone(); + expect(sourceRegion.emitted('update:items')).toBeUndefined(); + }); + + it('inserts a copy, not the source object itself', async () => { + const { sourceRegion, targetRegion } = await dropClone(); + const inserted = targetRegion.emitted('update:items')[0][0][0]; + const original = sourceRegion.props('items')[0]; + expect(inserted).not.toBe(original); + expect(inserted).toEqual(original); + }); + + it('does not mutate the source item when the clone is changed', async () => { + const { sourceRegion, targetRegion } = await dropClone(); + const inserted = targetRegion.emitted('update:items')[0][0][0]; + inserted.id = 'changed'; + inserted.matched = true; + expect(sourceRegion.props('items')[0]).toEqual({ id: 'a' }); + }); + + it('keeps the identifier field the source data uses', async () => { + const { targetRegion } = await dropClone({ + sourceItems: [ + { identifier: 'CHOICE_A', text: 'Alpha' }, + { identifier: 'CHOICE_B', text: 'Beta' }, + ], + }); + expect(targetRegion.emitted('update:items')[0][0][0]).toEqual({ + identifier: 'CHOICE_A', + text: 'Alpha', + }); + }); + + it('uses a transform function passed as the clone prop', async () => { + let next = 0; + const { targetRegion } = await dropClone({ + sourceProps: { clone: original => ({ ...original, uid: `copy-${++next}` }) }, + }); + expect(targetRegion.emitted('update:items')[0][0][0]).toEqual({ id: 'a', uid: 'copy-1' }); + }); + }); + + describe('full-order announcement on focus-exit', () => { + // The announcement waits a frame to see where focus settled. + function nextFrame() { + return new Promise(resolve => requestAnimationFrame(() => resolve())); + } + + // A region in the document, so document.activeElement can be inside it. + async function mountAttachedRegion() { + const { wrapper } = await mountRegion({}, { attachTo: document.body }); + const outside = document.createElement('button'); + document.body.appendChild(outside); + return { wrapper, outside }; + } + + it('announces the current order when focus leaves the region', async () => { + const { wrapper, outside } = await mountAttachedRegion(); + // Simulate DragSortWidget registrations via the provided callbacks. + const provided = wrapper.vm._provided; + provided.registerSortItem(0, 'First', 1); + provided.registerSortItem(1, 'Second', 2); + provided.registerSortItem(2, 'Third', 3); + + await wrapper.trigger('focusout', { relatedTarget: outside }); + outside.focus(); + await nextFrame(); + + expect(sendPoliteMessage).toHaveBeenCalledWith( + 'Current order: 1. First, 2. Second, 3. Third', + ); + document.body.removeChild(outside); + wrapper.destroy(); + }); + + it('does not announce when no items are registered', async () => { + const { wrapper, outside } = await mountAttachedRegion(); + await wrapper.trigger('focusout', { relatedTarget: outside }); + outside.focus(); + await nextFrame(); + expect(sendPoliteMessage).not.toHaveBeenCalled(); + document.body.removeChild(outside); + wrapper.destroy(); + }); + + it('does not announce anything for items that have been unregistered', async () => { + const { wrapper } = await mountRegion(); + const provided = wrapper.vm._provided; + provided.registerSortItem(0, 'First', 1); + provided.registerSortItem(1, 'Second', 2); + provided.registerSortItem(2, 'Third', 3); + [0, 1, 2].forEach(uid => provided.unregisterSortItem(uid)); + + const outside = document.createElement('button'); + document.body.appendChild(outside); + await wrapper.trigger('focusout', { relatedTarget: outside }); + + expect(sendPoliteMessage).not.toHaveBeenCalled(); + document.body.removeChild(outside); + }); + + it('does not announce on window blur (document not focused)', async () => { + document.hasFocus = jest.fn(() => false); + const { wrapper } = await mountRegion(); + wrapper.vm._provided.registerSortItem(0, 'First', 1); + await wrapper.trigger('focusout', { relatedTarget: null }); + await nextFrame(); + expect(sendPoliteMessage).not.toHaveBeenCalled(); + }); + }); + + describe('no announcement on row-to-row focus movement', () => { + it('does not announce when focus moves to another row inside the region', async () => { + const { wrapper } = await mountRegion(); + const provided = wrapper.vm._provided; + provided.registerSortItem(0, 'First', 1); + provided.registerSortItem(1, 'Second', 2); + const secondRow = row('Second'); + wrapper.element.appendChild(secondRow); + + await wrapper.trigger('focusout', { relatedTarget: secondRow }); + + expect(sendPoliteMessage).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableUniverse.spec.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableUniverse.spec.js new file mode 100644 index 0000000000..aa3add5030 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/__tests__/DraggableUniverse.spec.js @@ -0,0 +1,80 @@ +import { mount } from '@vue/test-utils'; +import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion'; +import DraggableUniverse from '../DraggableUniverse.vue'; +import { createDraggableUniverse, injectDraggableUniverse } from '../useDraggableUniverse'; + +jest.mock('kolibri-design-system/lib/composables/useKLiveRegion'); + +describe('useDraggableUniverse', () => { + beforeEach(() => { + useKLiveRegion.mockReturnValue({ sendPoliteMessage: jest.fn() }); + }); + + it('gives separate universes distinct group names', () => { + const a = createDraggableUniverse(); + const b = createDraggableUniverse(); + expect(a.groupName).not.toEqual(b.groupName); + }); + + it('uses an explicit name when provided', () => { + expect(createDraggableUniverse({ name: 'gaps' }).groupName).toBe('gaps'); + }); + + it('honours a custom delay', () => { + expect(createDraggableUniverse({ delay: 0 }).delay.value).toBe(0); + expect(createDraggableUniverse().delay.value).toBe(250); + }); + + it('tracks later changes to the delay prop', async () => { + let injected = null; + const Child = { + render: () => null, + setup() { + injected = injectDraggableUniverse(); + }, + }; + const wrapper = mount(DraggableUniverse, { + propsData: { delay: 250 }, + slots: { default: Child }, + }); + await wrapper.setProps({ delay: 0 }); + expect(injected.delay.value).toBe(0); + }); + + it('resolves a registered region element back to its API', () => { + const universe = createDraggableUniverse(); + const el = document.createElement('div'); + const api = { insertAt: jest.fn() }; + universe.registerRegion(el, api); + expect(universe.getRegion(el)).toBe(api); + universe.unregisterRegion(el); + expect(universe.getRegion(el)).toBeUndefined(); + }); + + it('provides the context to descendants that inject it', () => { + let injected = null; + const Child = { + render: () => null, + setup() { + injected = injectDraggableUniverse(); + }, + }; + mount(DraggableUniverse, { + propsData: { name: 'shared' }, + slots: { default: Child }, + }); + expect(injected).not.toBeNull(); + expect(injected.groupName).toBe('shared'); + }); + + it('injects null when there is no universe ancestor', () => { + let injected = 'unset'; + mount({ + render: () => null, + setup() { + injected = injectDraggableUniverse(); + }, + }); + expect(injected).toBeNull(); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/classDefinitions.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/classDefinitions.js new file mode 100644 index 0000000000..3aced79995 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/classDefinitions.js @@ -0,0 +1,16 @@ +// CSS classes shared between the draggable composables +// SortableJS draws the drag affordances itself and only needs the class names + +export const ITEM_CLASS = 'draggable-item'; +export const HANDLE_CLASS = 'draggable-handle'; +export const DISABLED_CLASS = 'draggable-item--disabled'; +// The clone that follows the pointer (SortableJS fallbackClass). +export const MIRROR_CLASS = 'draggable-item--mirror'; +// The placeholder left in the list showing where the item will land (ghostClass). +export const GHOST_CLASS = 'draggable-item--ghost'; +// The item being dragged, still in its source list (chosenClass). +export const CHOSEN_CLASS = 'draggable-item--chosen'; +// The copy under the cursor in native-drag mode (dragClass). +export const DRAG_CLASS = 'draggable-item--drag'; +// Hand-rolled drop "bounce"; SortableJS has no equivalent. +export const PLACED_CLASS = 'draggable-item--placed'; diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/domUtils.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/domUtils.js new file mode 100644 index 0000000000..1e1a65194d --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/domUtils.js @@ -0,0 +1,23 @@ +// SortableJS reorders the DOM directly. To keep Vue's virtual DOM the single +// source of truth we revert that mutation and then drive the change through data, + +/** + * Remove a node from its parent, if it has one. + * @param {HTMLElement} node - the node to detach from the DOM + */ +export function removeNode(node) { + if (node.parentElement !== null) { + node.parentElement.removeChild(node); + } +} + +/** + * Insert a node into a parent at a given child position. + * @param {HTMLElement} parent - the element to insert into + * @param {HTMLElement} node - the node to insert + * @param {number} position - the child index the node should occupy + */ +export function insertNodeAt(parent, node, position) { + const refNode = position === 0 ? parent.children[0] : parent.children[position - 1].nextSibling; + parent.insertBefore(node, refNode); +} diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/dragSortStrings.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/dragSortStrings.js new file mode 100644 index 0000000000..9a8eb58340 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/dragSortStrings.js @@ -0,0 +1,34 @@ +import { createTranslator } from 'shared/i18n'; + +export const dragSortStrings = createTranslator('DragSortStrings', { + moveItemUpLabel: { + message: 'Move {item} up', + context: 'Button label to move a specific item up in a vertical reorderable list', + }, + moveItemDownLabel: { + message: 'Move {item} down', + context: 'Button label to move a specific item down in a vertical reorderable list', + }, + moveItemLeftLabel: { + message: 'Move {item} left', + context: 'Button label to move a specific item left in a horizontal reorderable list', + }, + moveItemRightLabel: { + message: 'Move {item} right', + context: 'Button label to move a specific item right in a horizontal reorderable list', + }, + itemMovedToPosition: { + message: '{item} moved to position {position, number} of {total, number}', + context: 'Live region announcement after moving an item in a reorderable list', + }, + currentOrder: { + message: 'Current order: {order}', + context: + 'Live region announcement of the full list order after focus leaves the reorderable list', + }, + itemMovedToRegion: { + message: 'Moved to {region}', + context: + 'Live region announcement after an item is dragged into a named drop zone (e.g. a gap)', + }, +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss b/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss new file mode 100644 index 0000000000..0313eb15ae --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss @@ -0,0 +1,55 @@ +@import '~kolibri-design-system/lib/styles/definitions'; + +// here rather than in DraggableHandle because the handle marks an element the +// consumer owns, which is outside the reach of a scoped style there +.draggable-handle { + cursor: grab; +} + +// Content inside a drag item offers no expand affordance: `display: none` takes +// it out of the tab order and the accessibility tree with no JS involved. +// Read by `.expand-overlay` in SafeHtmlImage.vue; AccordionItem.vue's `.content` +// opts back in. +.draggable-item { + --content-affordance-display: none; +} + +.draggable-item--mirror { + @extend %dropshadow-6dp; + + z-index: 8; + cursor: grabbing; + border-radius: $radius; +} + +.draggable-item--ghost { + visibility: hidden; +} + +.draggable-item--placed { + animation-name: bounce-in; + animation-duration: $core-time; +} + +@keyframes bounce-in { + 0% { + transform: scale3d(1.05, 1.05, 1.05); + animation-timing-function: cubic-bezier(0.215, 0.61, 0.355, 1); + } + + 50% { + transform: scale3d(0.98, 0.98, 0.98); + animation-timing-function: cubic-bezier(0.215, 0.61, 0.355, 1); + } + + 100% { + transform: scale3d(1, 1, 1); + animation-timing-function: cubic-bezier(0.215, 0.61, 0.355, 1); + } +} + +@media (prefers-reduced-motion: reduce) { + .draggable-item--placed { + animation: none; + } +} diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/renderSlotRoot.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/renderSlotRoot.js new file mode 100644 index 0000000000..2536ee006e --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/renderSlotRoot.js @@ -0,0 +1,34 @@ +import logging from 'shared/logging'; + +/** + * Render a draggable component's slot content as it was written, adding the SortableJS + * marker classes to the consumer's own element rather than wrapping it in one. The + * consumer therefore chooses the element — ``, `
        `, ``, `
      • ` — by + * writing it, and its class, style, and attribute bindings stay on it. + * @param {import('vue').default} vm - the component instance rendering its slot + * @param {string|object|Array} [classes] - classes to add to the rendered element, in + * any form Vue's class binding accepts + * @returns {?import('vue').VNode} the slot's root node, or null when the slot is empty + */ +export default function renderSlotRoot(vm, classes) { + // whitespace text and `v-if` placeholders carry no tag + const nodes = (vm.$slots.default || []).filter(node => node.tag); + if (!nodes.length) { + return null; + } + if (nodes.length > 1) { + logging.error( + new Error( + `<${vm.$options.name}> renders a single root element; the rest of its slot is ignored`, + ), + ); + } + const [root] = nodes; + if (classes) { + // The parent re-renders whenever these components do, so this is applied to a + // fresh vnode each time rather than accumulating on one. + const data = root.data || (root.data = {}); + data.class = [data.class, classes]; + } + return root; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableRegion.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableRegion.js new file mode 100644 index 0000000000..99ee174087 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableRegion.js @@ -0,0 +1,249 @@ +import Sortable from 'sortablejs'; +import { onMounted, onBeforeUnmount, provide, watch } from 'vue'; +import { injectDraggableUniverse, createDraggableUniverse } from './useDraggableUniverse'; +import { DISABLED_CLASS, PLACED_CLASS } from './classDefinitions'; +import { removeNode, insertNodeAt } from './domUtils'; +import { dragSortStrings } from './dragSortStrings'; + +// Default `clone` transform: a copy that is a distinct object but keeps every field +// of the original, identifiers included. Consumers whose clones need independent +// identities pass their own transform. +const shallowClone = original => ({ ...original }); + +/** + * Wire up the SortableJS instance and reconciliation for one region. Call from the + * `setup()` of DraggableRegion. + * @param {object} props - the DraggableRegion props (reactive) + * @param {(event: string, ...args: unknown[]) => void} emit - the component's emit + * @param {() => HTMLElement} getRootEl - returns the region's root element; called once + * the component is mounted, since the region renders its consumer's element rather than + * one of its own + * @returns {{ handleStart: Function, handleEnd: Function, canAccept: Function }} the + * drag lifecycle callbacks, exposed for unit tests + */ +export default function useDraggableRegion(props, emit, getRootEl) { + // Regions grouped for cross-region drops share a + const universe = injectDraggableUniverse() || createDraggableUniverse(); + + const { currentOrder$, itemMovedToRegion$ } = dragSortStrings; + + let sortable = null; + + let rootEl = null; + + // frame handle for the deferred focus-exit announcement, null when none is queued + let pendingAnnouncement = null; + + // used only for the full-order announcement when focus leaves the region. + const registeredItems = {}; + + // This region's API, registered with the universe so a *source* region can hand + // this region an item on a cross-region drop. + const regionApi = { + get items() { + return props.items; + }, + get label() { + return props.label; + }, + insertAt(item, index) { + const next = [...props.items]; + next.splice(index, 0, item); + emit('update:items', next); + }, + }; + + function reordered(list, fromIndex, toIndex) { + const next = [...list]; + const [moved] = next.splice(fromIndex, 1); + next.splice(toIndex, 0, moved); + return next; + } + + function addBounce(node) { + node.classList.add(PLACED_CLASS); + node.addEventListener('animationend', () => node.classList.remove(PLACED_CLASS), { + once: true, + }); + } + + function handleStart(evt) { + universe.isDragging.value = true; + universe.activeRegion.value = regionApi; + universe.draggedItem.value = props.items[evt.oldDraggableIndex]; + emit('dragstart'); + } + + function handleEnd(evt) { + universe.isDragging.value = false; + universe.activeRegion.value = null; + universe.draggedItem.value = null; + emit('dragend'); + + const { item, clone, from, to, oldIndex, oldDraggableIndex, newDraggableIndex, pullMode } = evt; + + // exit early if the item was dropped back in its original position + if (from === to && oldDraggableIndex === newDraggableIndex) { + return; + } + + // 1. Undo SortableJS's DOM mutation + removeNode(item); + if (clone && clone.parentNode) { + removeNode(clone); + } + insertNodeAt(from, item, oldIndex); + + // 2. Apply the change to our sorable data. + if (to === from) { + emit('update:items', reordered(props.items, oldDraggableIndex, newDraggableIndex)); + addBounce(item); + return; + } + + const target = universe.getRegion(to); + if (!target) { + // Dropped outside this universe + return; + } + const movedItem = props.items[oldDraggableIndex]; + if (pullMode === 'clone') { + // a copy, so the two regions never share a reference to the same item + target.insertAt(cloneItem(movedItem), newDraggableIndex); + } else { + target.insertAt(movedItem, newDraggableIndex); + emit( + 'update:items', + props.items.filter((_, i) => i !== oldDraggableIndex), + ); + } + if (target.label) { + universe.sendPoliteMessage(itemMovedToRegion$({ region: target.label })); + } + } + + function canAccept() { + if (props.disabled) { + return false; + } + if (props.capacity != null && props.items.length >= props.capacity) { + return false; + } + return props.accepts(universe.draggedItem.value, universe.activeRegion.value); + } + + function cloneItem(original) { + return typeof props.clone === 'function' ? props.clone(original) : shallowClone(original); + } + + function groupOption() { + return { + name: universe.groupName, + pull: props.clone ? 'clone' : true, + // a closure, so capacity/disabled/accepts are re-read on every drop check + put: canAccept, + }; + } + + // SortableJS copies its options at construction, so anything reactive has to be + // pushed into the instance when it changes (see the watchers below). + function updateOption(name, value) { + if (sortable) { + sortable.option(name, value); + } + } + + watch( + () => props.sortable, + sort => updateOption('sort', sort), + ); + // on the pull mode rather than on `clone` itself, so an inline transform function + // being a new identity on each render does not churn the instance + watch( + () => Boolean(props.clone), + () => updateOption('group', groupOption()), + ); + watch(universe.delay, delay => updateOption('delay', delay)); + + function announceOrder() { + const entries = Object.values(registeredItems); + if (!entries.length) { + return; + } + const order = entries + .sort((a, b) => a.position - b.position) + .map((entry, index) => `${index + 1}. ${entry.label}`) + .join(', '); + universe.sendPoliteMessage(currentOrder$({ order })); + } + + function handleFocusOut(event) { + // window/tab blur: relatedTarget is null but focus hasn't actually left + if (!document.hasFocus()) { + return; + } + // focus moved to another row inside this region: not a list-exit, don't announce + if (event.relatedTarget && rootEl.contains(event.relatedTarget)) { + return; + } + // A keyboard move re-renders the region, which detaches the moved row and blurs + // the move button with a null relatedTarget before focus is restored to it. Wait + // a frame and look at where focus actually settled, so a move does not get + // reported as a list-exit — and so the order we read is the post-move one. + cancelPendingAnnouncement(); + pendingAnnouncement = requestAnimationFrame(() => { + pendingAnnouncement = null; + if (!rootEl || !document.hasFocus() || rootEl.contains(document.activeElement)) { + return; + } + announceOrder(); + }); + } + + function cancelPendingAnnouncement() { + if (pendingAnnouncement !== null) { + cancelAnimationFrame(pendingAnnouncement); + pendingAnnouncement = null; + } + } + + // Provided for the a11y move buttons + provide('registerSortItem', (uid, label, position) => { + registeredItems[uid] = { label, position }; + }); + provide('unregisterSortItem', uid => { + delete registeredItems[uid]; + }); + + onMounted(() => { + rootEl = getRootEl(); + universe.registerRegion(rootEl, regionApi); + rootEl.addEventListener('focusout', handleFocusOut); + + sortable = new Sortable(rootEl, { + ...universe.sortableDefaults, + delay: universe.delay.value, + sort: props.sortable, + filter: `.${DISABLED_CLASS}`, + group: groupOption(), + onStart: handleStart, + onEnd: handleEnd, + }); + }); + + onBeforeUnmount(() => { + cancelPendingAnnouncement(); + if (sortable) { + sortable.destroy(); + sortable = null; + } + if (rootEl) { + rootEl.removeEventListener('focusout', handleFocusOut); + universe.unregisterRegion(rootEl); + rootEl = null; + } + }); + + // Exposed for unit tests + return { handleStart, handleEnd, canAccept }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableUniverse.js b/contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableUniverse.js new file mode 100644 index 0000000000..f673f29e19 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/useDraggableUniverse.js @@ -0,0 +1,103 @@ +import { ref, provide, inject } from 'vue'; +import { v4 as uuidv4 } from 'uuid'; +import useKLiveRegion from 'kolibri-design-system/lib/composables/useKLiveRegion'; +import { + ITEM_CLASS, + HANDLE_CLASS, + MIRROR_CLASS, + GHOST_CLASS, + CHOSEN_CLASS, + DRAG_CLASS, +} from './classDefinitions'; + +const DraggableUniverseSymbol = Symbol('draggableUniverse'); + +/** + * Build a universe context. Kept separate from `provide` so a region with no + * `` ancestor can create its own standalone context. + * @param {object} [options] - universe configuration + * @param {string} [options.name] - explicit group name, the way to deliberately share + * one group across separate component trees; defaults to a generated unique id. + * Intentionally initial-value-only: it is read once here, and a universe keeps the + * group name it was created with for its whole lifetime. Regions cannot be moved + * between groups after mount. + * @param {number} [options.delay] - initial press-and-hold delay (ms) before a drag + * begins, touch input only; set `delay.value` on the returned context to change it + * afterwards + * @returns {object} the universe context + */ +export function createDraggableUniverse({ name, delay } = {}) { + const groupName = name || `draggable-universe-${uuidv4()}`; + + // each region's root element -> its API + const regions = new Map(); + + // drag state + const isDragging = ref(false); + const activeRegion = ref(null); + const draggedItem = ref(null); + + const { sendPoliteMessage } = useKLiveRegion(); + + // Reactive so a universe can change the press-and-hold delay after its regions + // have mounted; each region watches this and updates its SortableJS instance. + const dragDelay = ref(delay == null ? 250 : delay); + + const prefersReducedMotion = + window.matchMedia?.('(prefers-reduced-motion: reduce)').matches ?? false; + + const sortableDefaults = { + forceFallback: true, + fallbackOnBody: false, // keep the clone inside the region subtree so overrides still match + draggable: `.${ITEM_CLASS}`, + handle: `.${HANDLE_CLASS}`, + // A delayed mouse drag is cancelled when the pointer moves, so the delay must + // only apply to touch input. + delayOnTouchOnly: true, + fallbackClass: MIRROR_CLASS, + ghostClass: GHOST_CLASS, + chosenClass: CHOSEN_CLASS, + dragClass: DRAG_CLASS, + animation: prefersReducedMotion ? 0 : 150, + }; + + return { + groupName, + sortableDefaults, + delay: dragDelay, + isDragging, + activeRegion, + draggedItem, + sendPoliteMessage, + registerRegion(el, api) { + regions.set(el, api); + }, + unregisterRegion(el) { + regions.delete(el); + }, + getRegion(el) { + return regions.get(el); + }, + }; +} + +/** + * Create a universe context and provide it to descendant regions. Call from the + * `setup()` of a component that wraps several regions meant to share items. + * @param {object} [options] - name/delay options, see {@link createDraggableUniverse} + * @returns {object} the universe context + */ +export default function useDraggableUniverse(options = {}) { + const context = createDraggableUniverse(options); + provide(DraggableUniverseSymbol, context); + return context; +} + +/** + * Inject the nearest universe context, or `null` when a region has no + * `` ancestor. + * @returns {?object} the universe context + */ +export function injectDraggableUniverse() { + return inject(DraggableUniverseSymbol, null); +} diff --git a/package.json b/package.json index 9a43d729b4..853a684855 100644 --- a/package.json +++ b/package.json @@ -90,6 +90,7 @@ "pdfjs-dist": "^2.16.105", "qs": "^6.15.2", "regenerator-runtime": "^0.14.1", + "sortablejs": "1.15.7", "spark-md5": "^3.0.0", "store2": "^2.14.4", "string-strip-html": "8.3.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4154e0a794..45e4e2aab0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -122,6 +122,9 @@ importers: regenerator-runtime: specifier: ^0.14.1 version: 0.14.1 + sortablejs: + specifier: 1.15.7 + version: 1.15.7 spark-md5: specifier: ^3.0.0 version: 3.0.2 @@ -6875,6 +6878,9 @@ packages: resolution: {integrity: sha512-iF+tNDQla22geJdTyJB1wM/qrX9DMRwWrciEPwWLPRWAUEM8sQiyxgckLxWT1f7+9VabJS0jTGGr4QgBuvi6Ww==} engines: {node: '>= 10.0.0', npm: '>= 3.0.0'} + sortablejs@1.15.7: + resolution: {integrity: sha512-Kk8wLQPlS+yi1ZEf48a4+fzHa4yxjC30M/Sr2AnQu+f/MPwvvX9XjZ6OWejiz8crBsLwSq8GHqaxaET7u6ux0A==} + source-list-map@2.0.1: resolution: {integrity: sha512-qnQ7gVMxGNxsiL4lEuJwe/To8UnK7fAnmbGEEH8RpLouuKbeEm0lhbQVFIrNSuB+G7tVrAlVsZgETT5nljf+Iw==} @@ -15871,6 +15877,8 @@ snapshots: ip-address: 9.0.5 smart-buffer: 4.2.0 + sortablejs@1.15.7: {} + source-list-map@2.0.1: {} source-map-js@1.2.1: {} From 0181f599a2352da5ff86a42aa4c8d6934fbb5e46 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Tue, 1 Sep 2026 05:48:50 -0700 Subject: [PATCH 2/9] Let DragSortWidget take an icon colour 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) --- .../shared/views/dragSort/DragSortWidget/index.vue | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue b/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue index b1371487f4..b2e0441445 100644 --- a/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/DragSortWidget/index.vue @@ -16,6 +16,7 @@ class="btn up" size="mini" :ariaLabel="moveUpAriaLabel" + :color="color" :class="{ visuallyhidden: !hasFocus && !horizontal }" @click="clickUp" @keyup.space="clickUp" @@ -28,6 +29,7 @@ v-if="!horizontal" icon="dragHorizontal" class="grip" + :color="color" style="top: 0; width: 24px; height: 24px" /> Date: Mon, 31 Aug 2026 16:03:45 -0700 Subject: [PATCH 3/9] Hide the whole dragged row, TipTap toolbar included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.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) --- .../frontend/shared/views/dragSort/draggable.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss b/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss index 0313eb15ae..a5039ea132 100644 --- a/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss +++ b/contentcuration/contentcuration/frontend/shared/views/dragSort/draggable.scss @@ -22,8 +22,12 @@ border-radius: $radius; } +// `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; } .draggable-item--placed { From 6cf9372f70ad2c2fe845a6ca23caaebc525645a2 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 26 Aug 2026 14:01:58 -0700 Subject: [PATCH 4/9] Add reorder-by-id to the QTI choice and ordering composables 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) --- .../__tests__/useChoiceInteraction.spec.js | 41 +++++++++++++++++++ .../__tests__/useOrderingInteraction.spec.js | 31 ++++++++++++++ .../composables/useChoiceInteraction.js | 9 ++++ .../composables/useOrderingInteraction.js | 9 ++++ 4 files changed, 90 insertions(+) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useChoiceInteraction.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useChoiceInteraction.spec.js index 05fa5419f4..17cc9e46bd 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useChoiceInteraction.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useChoiceInteraction.spec.js @@ -112,6 +112,47 @@ describe('useChoiceInteraction', () => { }); }); + describe('setChoiceOrder()', () => { + it('applies the given order', () => { + const { state, setChoiceOrder } = setup([ + makeAnswer({ id: 'a' }), + makeAnswer({ id: 'b' }), + makeAnswer({ id: 'c' }), + ]); + setChoiceOrder(['c', 'a', 'b']); + expect(state.value.choices.map(a => a.id)).toEqual(['c', 'a', 'b']); + }); + + it('is a no-op when the given order is not a permutation of the current ids', () => { + const { state, setChoiceOrder } = setup([ + makeAnswer({ id: 'a' }), + makeAnswer({ id: 'b' }), + makeAnswer({ id: 'c' }), + ]); + + setChoiceOrder(['c', 'a']); + expect(state.value.choices.map(a => a.id)).toEqual(['a', 'b', 'c']); + + setChoiceOrder(['c', 'a', 'zzz']); + expect(state.value.choices.map(a => a.id)).toEqual(['a', 'b', 'c']); + }); + + it('produces the same bodyXml as moveChoiceUp for the equivalent move', () => { + const choices = [ + makeAnswer({ id: 'a', content: 'A' }), + makeAnswer({ id: 'b', content: 'B' }), + makeAnswer({ id: 'c', content: 'C' }), + ]; + const { moveChoiceUp, bodyXml: chevronBodyXml } = setup(choices); + moveChoiceUp('c'); + + const { setChoiceOrder, bodyXml: dragBodyXml } = setup(choices); + setChoiceOrder(['a', 'c', 'b']); + + expect(dragBodyXml.value).toBe(chevronBodyXml.value); + }); + }); + describe('toggleCorrectChoice()', () => { it('singleSelect: sets only the target as correct and clears others', () => { const { state, toggleCorrectChoice, questionTypeRef } = setup([ diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useOrderingInteraction.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useOrderingInteraction.spec.js index f46199f9a6..296b5a5331 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useOrderingInteraction.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useOrderingInteraction.spec.js @@ -102,6 +102,37 @@ describe('useOrderingInteraction', () => { }); }); + describe('setItemOrder()', () => { + it('applies the given order', () => { + const { state, setItemOrder } = setup(); + const [firstId, secondId, thirdId] = state.value.items.map(i => i.id); + setItemOrder([thirdId, firstId, secondId]); + expect(state.value.items.map(i => i.id)).toEqual([thirdId, firstId, secondId]); + }); + + it('is a no-op when the given order is not a permutation of the current ids', () => { + const { state, setItemOrder } = setup(); + const [firstId, secondId, thirdId] = state.value.items.map(i => i.id); + + setItemOrder([thirdId, firstId]); + expect(state.value.items.map(i => i.id)).toEqual([firstId, secondId, thirdId]); + + setItemOrder([thirdId, firstId, 'order_zzzzzzzz']); + expect(state.value.items.map(i => i.id)).toEqual([firstId, secondId, thirdId]); + }); + + it('produces the same bodyXml as moveItemUp for the equivalent move', () => { + const { state, moveItemUp, bodyXml: chevronBodyXml } = setup(); + const [firstId, secondId, thirdId] = state.value.items.map(i => i.id); + moveItemUp(secondId); + + const { setItemOrder, bodyXml: dragBodyXml } = setup(); + setItemOrder([secondId, firstId, thirdId]); + + expect(dragBodyXml.value).toBe(chevronBodyXml.value); + }); + }); + describe('setItemContent()', () => { it('updates only the targeted item content', () => { const { state, setItemContent } = setup(); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useChoiceInteraction.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useChoiceInteraction.js index d030f693d2..dff8864cb7 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useChoiceInteraction.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useChoiceInteraction.js @@ -53,6 +53,14 @@ export function useChoiceInteraction(interactionBlock, questionType) { state.value = { ...state.value, choices }; } + /** Apply a permutation of the current choice ids; anything else is ignored. */ + function setChoiceOrder(orderedIds) { + const byId = new Map(state.value.choices.map(c => [c.id, c])); + const choices = [...new Set(orderedIds)].map(id => byId.get(id)); + if (choices.length !== byId.size || choices.includes(undefined)) return; + state.value = { ...state.value, choices }; + } + /** * Toggle the correct flag for a single choice. * @@ -98,6 +106,7 @@ export function useChoiceInteraction(interactionBlock, questionType) { removeChoice, moveChoiceUp, moveChoiceDown, + setChoiceOrder, toggleCorrectChoice, setPrompt, setChoiceContent, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useOrderingInteraction.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useOrderingInteraction.js index 9590207c3b..67922513c2 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useOrderingInteraction.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useOrderingInteraction.js @@ -44,6 +44,14 @@ export function useOrderingInteraction(interactionBlock, questionType) { state.value = { ...state.value, items }; } + /** Apply a permutation of the current item ids; anything else is ignored. */ + function setItemOrder(orderedIds) { + const byId = new Map(state.value.items.map(item => [item.id, item])); + const items = [...new Set(orderedIds)].map(id => byId.get(id)); + if (items.length !== byId.size || items.includes(undefined)) return; + state.value = { ...state.value, items }; + } + function setItemContent(id, html) { state.value = { ...state.value, @@ -62,6 +70,7 @@ export function useOrderingInteraction(interactionBlock, questionType) { removeItem, moveItemUp, moveItemDown, + setItemOrder, setItemContent, setPrompt, }; From bbfc2ba2baf17dbc0e13c3acdb886aa768aeba4e Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 26 Aug 2026 14:02:17 -0700 Subject: [PATCH 5/9] Reorder choices by dragging in the choice interaction editor 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) --- .../choice/ChoiceInteractionEditor.vue | 339 ++++++++++-------- .../__tests__/ChoiceInteractionEditor.spec.js | 107 +++++- .../views/QTIEditor/qtiEditorStrings.js | 11 +- 3 files changed, 295 insertions(+), 162 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue index ff72914d66..4b87a347f4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue @@ -81,115 +81,155 @@ {{ answersDescription }}
      - + -
      - - -
      -
      + + -
      - - - - +
      + + +
      + +
      +       + + +
      + + + + +
      + +
      + +
      + +
      + +
      +
      - -
      - -
      - - -
      + - -
      -
      + {{ errorEmptyChoiceContent$() }} + + + {{ errorDuplicateChoiceContent$() }} + +
      - - - {{ errorEmptyChoiceContent$() }} - - - {{ errorDuplicateChoiceContent$() }} - -       -
      -       + + +       state.value.choices.find(a => a.correct)?.id ?? null); + const isOnlyChoice = computed(() => state.value.choices.length <= 1); + + // Shuffled choices reach the learner in a random order, so an authored + // order has nothing to say. + const isReorderable = computed(() => props.mode === 'edit' && !state.value.shuffle); + + const listTag = computed(() => (isSingleSelect.value ? 'KRadioButtonGroup' : 'div')); + const errorCodes = computed(() => errors.value.map(e => e.code)); const emptyChoiceIds = computed( () => @@ -395,33 +449,8 @@ } } - function getChoiceRowActions(answerId, index) { - return [ - { - id: 'up', - icon: 'chevronUp', - label: moveChoiceUpBtn$(), - disabled: index === 0, - handler: () => moveChoiceUp(answerId), - collapsed: windowIsSmall.value, - }, - { - id: 'down', - icon: 'chevronDown', - label: moveChoiceDownBtn$(), - disabled: index === state.value.choices.length - 1, - handler: () => moveChoiceDown(answerId), - collapsed: windowIsSmall.value, - }, - { - id: 'delete', - icon: 'close', - label: deleteChoiceBtn$(), - disabled: state.value.choices.length <= 1, - handler: () => onRemoveChoice(answerId), - collapsed: windowIsSmall.value, - }, - ]; + function onReorderChoices(nextChoices) { + setChoiceOrder(nextChoices.map(c => c.id)); } const isPromptEditing = computed(() => props.mode === 'edit' && isQuestionOpen.value); @@ -496,6 +525,9 @@ closeQuestion, closeChoice, correctChoiceId, + isOnlyChoice, + isReorderable, + listTag, questionHasError, noCorrectAnswerError, tooManyCorrectError, @@ -508,7 +540,10 @@ setShowAnswerCount, onToggleCorrect, onAddChoice, - getChoiceRowActions, + onRemoveChoice, + onReorderChoices, + moveChoiceUp, + moveChoiceDown, isChoiceClosed, isChoiceOpen, getChoiceClasses, @@ -516,6 +551,8 @@ handlePromptClick, handleChoiceClick, addChoiceBtn$, + deleteChoiceBtn$, + choiceItemLabel$, markCorrectLabel$, errorPromptRequired$, errorNoCorrectAnswer$, @@ -603,8 +640,10 @@ flex-direction: column; } + /* Opaque, so a row dragged over the rows beneath it stays readable */ .choice-border { position: relative; + background-color: v-bind('$themeTokens.surface'); border: 1px solid; border-radius: 4px; transition: background-color 0.3s; @@ -628,7 +667,7 @@ padding: 7.5px; } - /* Flex row: [selection] [content] [actions] */ + /* Flex row: [drag] [selection] [content] [actions] */ .choice-layout { display: flex; align-items: center; @@ -643,30 +682,48 @@ flex-wrap: wrap; align-items: center; - .choice-selection { - flex: 0 0 auto; - order: 0; - margin-bottom: 4px; - } - + .choice-drag, + .choice-selection, .choice-actions { flex: 0 0 auto; - order: 1; margin-bottom: 4px; } .choice-content { flex: 0 0 100%; - order: 2; + order: 1; min-width: 0; } } } } + .choice-drag { + display: flex; + flex-shrink: 0; + align-items: center; + margin-right: 8px; + + .small-screen & { + margin-right: 4px; + } + } + + /* KRadioButton/KCheckbox wrap their 24px control in a table with 8px block margins, + and the inline icon adds descender space under it. Both push the control off the + row centre, out of line with the drag handle. */ .choice-selection { + display: flex; flex-shrink: 0; + align-items: center; margin-right: 16px; + line-height: 0; + + ::v-deep .k-radio-button-container, + ::v-deep .k-checkbox-container { + margin-top: 0; + margin-bottom: 0; + } .small-screen & { margin-right: 6px; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/__tests__/ChoiceInteractionEditor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/__tests__/ChoiceInteractionEditor.spec.js index 032cf2885e..a2bb020e4c 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/__tests__/ChoiceInteractionEditor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/__tests__/ChoiceInteractionEditor.spec.js @@ -1,4 +1,5 @@ import { render, screen, fireEvent, within } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; import { nextTick } from 'vue'; import VueRouter from 'vue-router'; import ChoiceInteractionEditor from '../ChoiceInteractionEditor.vue'; @@ -14,8 +15,19 @@ import { } from '../../../utils/testingFixtures'; import { QuestionType } from '../../../constants'; import { qtiEditorStrings as tr } from '../../../qtiEditorStrings'; +import { dragSortStrings as dragTr } from 'shared/views/dragSort/dragSortStrings'; jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor'); +// jsdom cannot produce the pointer events real SortableJS listens for, so the tests +// call the captured `onEnd` instead. +let mockSortableInstances; +jest.mock('sortablejs', () => + jest.fn().mockImplementation((el, options) => { + const instance = { el, options, option: jest.fn(), destroy: jest.fn() }; + mockSortableInstances.push(instance); + return instance; + }), +); jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => { const { ref } = require('vue'); return { @@ -27,6 +39,7 @@ jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => { let teleportContainer; beforeEach(() => { + mockSortableInstances = []; teleportContainer = document.createElement('div'); teleportContainer.id = 'test-settings-target'; document.body.appendChild(teleportContainer); @@ -38,12 +51,29 @@ afterEach(() => { } }); +const choiceLabel = number => tr.$tr('choiceItemLabel', { number }); +const moveUpName = number => dragTr.$tr('moveItemUpLabel', { item: choiceLabel(number) }); +const moveDownName = number => dragTr.$tr('moveItemDownLabel', { item: choiceLabel(number) }); + const renderEditor = (props = {}) => render(ChoiceInteractionEditor, { props: { mode: 'edit', teleportTargetId: 'test-settings-target', ...props }, routes: new VueRouter(), }); +const dragFirstRowToLast = async () => { + const { options, el } = mockSortableInstances[mockSortableInstances.length - 1]; + options.onEnd({ + item: el.children[0], + from: el, + to: el, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: el.children.length - 1, + }); + await nextTick(); +}; + describe('ChoiceInteractionEditor', () => { describe('prompt rendering', () => { it('renders the prompt text from the XML', () => { @@ -182,34 +212,84 @@ describe('ChoiceInteractionEditor', () => { expect(screen.getAllByRole('radio')).toHaveLength(4); }); - it('renders move-up, move-down, and delete buttons for each non-fixed choice', () => { + it('gives every choice row its own delete button', () => { renderEditor({ interaction: block(CHOICE_SINGLE_SELECT_XML), questionType: QuestionType.SINGLE_SELECT, }); - expect(screen.getAllByRole('button', { name: tr.$tr('moveChoiceUpBtn') })).toHaveLength(3); - expect(screen.getAllByRole('button', { name: tr.$tr('moveChoiceDownBtn') })).toHaveLength(3); expect(screen.getAllByRole('button', { name: tr.$tr('deleteChoiceBtn') })).toHaveLength(3); }); - it('disables move-up on the first choice', () => { + it('hides move-up on the first choice and move-down on the last', () => { renderEditor({ interaction: block(CHOICE_SINGLE_SELECT_XML), questionType: QuestionType.SINGLE_SELECT, }); - const moveUpBtns = screen.getAllByRole('button', { name: tr.$tr('moveChoiceUpBtn') }); - expect(moveUpBtns[0]).toBeDisabled(); - expect(moveUpBtns[1]).toBeEnabled(); + // `v-show` hides the out-of-range control, dropping it out of the accessibility tree + expect(screen.queryByRole('button', { name: moveUpName(1) })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: moveUpName(2) })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: moveUpName(3) })).toBeInTheDocument(); + + expect(screen.getByRole('button', { name: moveDownName(1) })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: moveDownName(2) })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: moveDownName(3) })).not.toBeInTheDocument(); }); - it('disables move-down on the last choice', () => { + it('reorders the choices when a row is dragged to a new position', async () => { + const { emitted } = renderEditor({ + interaction: block(CHOICE_SINGLE_SELECT_XML), + questionType: QuestionType.SINGLE_SELECT, + }); + await dragFirstRowToLast(); + + const { bodyXml } = emitted()['update:interaction'].pop()[0]; + expect(bodyXml.indexOf('identifier="mercury"')).toBeGreaterThan( + bodyXml.indexOf('identifier="earth"'), + ); + }); + + it('drops the reorder controls once the choices are shuffled', async () => { + const user = userEvent.setup(); renderEditor({ interaction: block(CHOICE_SINGLE_SELECT_XML), questionType: QuestionType.SINGLE_SELECT, }); - const moveDownBtns = screen.getAllByRole('button', { name: tr.$tr('moveChoiceDownBtn') }); - expect(moveDownBtns[2]).toBeDisabled(); - expect(moveDownBtns[1]).toBeEnabled(); + expect(screen.getByRole('button', { name: moveDownName(1) })).toBeInTheDocument(); + + await user.click( + within(teleportContainer).getByRole('checkbox', { name: tr.$tr('shuffleAnswersLabel') }), + ); + + expect(screen.queryByRole('button', { name: moveDownName(1) })).not.toBeInTheDocument(); + const { option } = mockSortableInstances[mockSortableInstances.length - 1]; + expect(option).toHaveBeenCalledWith('sort', false); + }); + + it('binds the drag list to the rendered element after the question type changes', async () => { + // Switching question type swaps KRadioButtonGroup for a plain div. Left bound to the + // detached element, drag silently stops working, so assert on the element SortableJS + // was handed rather than on anything the editor emits. + const { updateProps } = renderEditor({ + interaction: block(CHOICE_SINGLE_SELECT_XML), + questionType: QuestionType.SINGLE_SELECT, + }); + await updateProps({ questionType: QuestionType.MULTI_SELECT }); + + const { el } = mockSortableInstances[mockSortableInstances.length - 1]; + expect(document.body).toContainElement(el); + }); + + it('reorders the choices when a row is moved down by keyboard', async () => { + const user = userEvent.setup(); + const { emitted } = renderEditor({ + interaction: block(CHOICE_SINGLE_SELECT_XML), + questionType: QuestionType.SINGLE_SELECT, + }); + await user.click(screen.getByRole('button', { name: moveDownName(1) })); + const { bodyXml } = emitted()['update:interaction'].pop()[0]; + expect(bodyXml.indexOf('identifier="venus"')).toBeLessThan( + bodyXml.indexOf('identifier="mercury"'), + ); }); it('disables delete when only one choice remains', async () => { @@ -304,6 +384,7 @@ describe('ChoiceInteractionEditor', () => { expect( screen.queryByRole('button', { name: tr.$tr('deleteChoiceBtn') }), ).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: moveDownName(1) })).not.toBeInTheDocument(); }); }); @@ -477,9 +558,7 @@ describe('ChoiceInteractionEditor', () => { interaction: block(CHOICE_SINGLE_SELECT_XML), questionType: QuestionType.SINGLE_SELECT, }); - screen - .getAllByRole('button', { name: tr.$tr('moveChoiceUpBtn') }) - .forEach(b => expect(b).toHaveAccessibleName()); + screen.getAllByRole('button').forEach(b => expect(b).toHaveAccessibleName()); }); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js index f597368a2e..8fcd7fe878 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js @@ -151,13 +151,10 @@ export const qtiEditorStrings = createTranslator('QTIEditorStrings', { message: 'Delete choice', context: 'Accessible label for the delete-choice icon button', }, - moveChoiceUpBtn: { - message: 'Move choice up', - context: 'Accessible label for the move-up icon button', - }, - moveChoiceDownBtn: { - message: 'Move choice down', - context: 'Accessible label for the move-down icon button', + choiceItemLabel: { + message: 'Choice {number}', + context: + 'Names a choice row in its reorder controls and in the screen reader announcement after it moves', }, markCorrectLabel: { message: 'Mark as correct answer', From 7660d59e345f9ef6170c071f6a2c090b9e1672ab Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 26 Aug 2026 14:02:27 -0700 Subject: [PATCH 6/9] Reorder items by dragging in the ordering interaction editor 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) --- .../ordering/OrderingInteractionEditor.vue | 240 ++++++++++-------- .../OrderingInteractionEditor.spec.js | 84 ++++-- .../views/QTIEditor/qtiEditorStrings.js | 11 +- 3 files changed, 201 insertions(+), 134 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue index 6d400f9366..5e272bc328 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue @@ -66,87 +66,119 @@
      -
        -
      1. - - -
        -
        + + - -
        - +
        + + +
        + +
        +         + + + +
        + +
        + +
        + +
        +
        - -
        + - -
        -
        -
        - - - - {{ errorEmptyItemContent$() }} - - - {{ errorDuplicateItemContent$() }} - -         -
        2. -
      + {{ errorEmptyItemContent$() }} + + + {{ errorDuplicateItemContent$() }} + + + + + +       import { computed, ref, watch } from 'vue'; - import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow'; import { themeTokens } from 'kolibri-design-system/lib/styles/theme'; import { qtiEditorStrings } from '../../qtiEditorStrings'; import { ValidationError } from '../../constants'; import { useOrderingInteraction } from '../../composables/useOrderingInteraction'; - import CollapsibleToolbar from '../../components/CollapsibleToolbar/index.vue'; import ValidationMessage from '../../components/ValidationMessage/index.vue'; import AddListItemButton from '../../components/AddListItemButton/index.vue'; import ClickableRegion from '../../components/ClickableRegion/index.vue'; + import DraggableRegion from 'shared/views/dragSort/DraggableRegion.vue'; + import DraggableItem from 'shared/views/dragSort/DraggableItem.vue'; + import DraggableHandle from 'shared/views/dragSort/DraggableHandle.vue'; + import DragSortWidget from 'shared/views/dragSort/DragSortWidget/index.vue'; import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor'; import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService'; @@ -180,14 +214,16 @@ components: { TipTapEditor, - CollapsibleToolbar, ValidationMessage, AddListItemButton, ClickableRegion, + DraggableRegion, + DraggableItem, + DraggableHandle, + DragSortWidget, }, setup(props, { emit }) { - const { windowIsSmall } = useKResponsiveWindow(); const tokens = themeTokens(); const { @@ -197,8 +233,7 @@ correctOrderDescription$, addItemBtn$, deleteItemBtn$, - moveItemUpBtn$, - moveItemDownBtn$, + orderingItemLabel$, errorTooFewChoices$, errorEmptyItemContent$, errorDuplicateItemContent$, @@ -217,6 +252,7 @@ removeItem, moveItemUp, moveItemDown, + setItemOrder, setItemContent, setPrompt, } = useOrderingInteraction(props.interaction, questionTypeRef); @@ -283,6 +319,8 @@ emit('update:interaction', newVal); }); + const isOnlyItem = computed(() => state.value.items.length <= 1); + const errorCodes = computed(() => errors.value.map(e => e.code)); const promptHasError = computed(() => @@ -353,38 +391,14 @@ if (newId) openItem(newId); } - function getItemActions(itemId, index) { - return [ - { - id: 'up', - icon: 'chevronUp', - label: moveItemUpBtn$({ number: index + 1 }), - disabled: index === 0, - handler: () => moveItemUp(itemId), - collapsed: windowIsSmall.value, - }, - { - id: 'down', - icon: 'chevronDown', - label: moveItemDownBtn$({ number: index + 1 }), - disabled: index === state.value.items.length - 1, - handler: () => moveItemDown(itemId), - collapsed: windowIsSmall.value, - }, - { - id: 'delete', - icon: 'close', - label: deleteItemBtn$({ number: index + 1 }), - disabled: state.value.items.length <= 1, - handler: () => removeItem(itemId), - collapsed: windowIsSmall.value, - }, - ]; + function onReorderItems(nextItems) { + setItemOrder(nextItems.map(item => item.id)); } return { EditorImageProcessor, state, + isOnlyItem, promptHasError, tooFewItemsError, emptyItemIds, @@ -403,12 +417,17 @@ onAddItem, setItemContent, setPrompt, - getItemActions, + moveItemUp, + moveItemDown, + removeItem, + onReorderItems, questionLabel$, errorPromptRequired$, correctOrderLabel$, correctOrderDescription$, addItemBtn$, + deleteItemBtn$, + orderingItemLabel$, errorTooFewChoices$, errorEmptyItemContent$, errorDuplicateItemContent$, @@ -481,7 +500,9 @@ font-weight: 400; } + /* Opaque, so a row dragged over the rows beneath it stays readable */ .item-border { + background-color: v-bind('$themeTokens.surface'); border: 1px solid; border-radius: 4px; transition: background-color 0.3s; @@ -542,6 +563,13 @@ } } + .item-drag { + display: flex; + flex-shrink: 0; + align-items: center; + margin-right: 8px; + } + .position-badge { display: flex; flex-shrink: 0; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/OrderingInteractionEditor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/OrderingInteractionEditor.spec.js index 6e082e94cf..fca24c360a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/OrderingInteractionEditor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/__tests__/OrderingInteractionEditor.spec.js @@ -1,4 +1,5 @@ import { render, screen, fireEvent } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; import { nextTick } from 'vue'; import VueRouter from 'vue-router'; import OrderingInteractionEditor from '../OrderingInteractionEditor.vue'; @@ -11,22 +12,47 @@ import { } from '../../../utils/testingFixtures'; import { QuestionType } from '../../../constants'; import { qtiEditorStrings as tr } from '../../../qtiEditorStrings'; +import { dragSortStrings as dragTr } from 'shared/views/dragSort/dragSortStrings'; jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor'); -jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => { - const { ref } = require('vue'); - return { - __esModule: true, - default: () => ({ windowIsSmall: ref(false) }), - }; +// jsdom cannot produce the pointer events real SortableJS listens for, so the tests +// call the captured `onEnd` instead. +let mockSortableInstances; +jest.mock('sortablejs', () => + jest.fn().mockImplementation((el, options) => { + const instance = { el, options, option: jest.fn(), destroy: jest.fn() }; + mockSortableInstances.push(instance); + return instance; + }), +); + +beforeEach(() => { + mockSortableInstances = []; }); +const itemLabel = number => tr.$tr('orderingItemLabel', { number }); +const moveUpName = number => dragTr.$tr('moveItemUpLabel', { item: itemLabel(number) }); +const moveDownName = number => dragTr.$tr('moveItemDownLabel', { item: itemLabel(number) }); + const renderEditor = (props = {}) => render(OrderingInteractionEditor, { props: { mode: 'edit', ...props }, routes: new VueRouter(), }); +const dragFirstRowToLast = async () => { + const { options, el } = mockSortableInstances[mockSortableInstances.length - 1]; + options.onEnd({ + item: el.children[0], + from: el, + to: el, + oldIndex: 0, + oldDraggableIndex: 0, + newDraggableIndex: el.children.length - 1, + }); + await nextTick(); +}; + describe('OrderingInteractionEditor', () => { describe('edit mode rendering', () => { it('renders the prompt text from the XML', () => { @@ -93,30 +119,45 @@ describe('OrderingInteractionEditor', () => { expect(screen.getByText('4')).toBeInTheDocument(); }); - it('disables move-up button for the first item', () => { + it('hides move-up on the first item and move-down on the last', () => { renderEditor({ interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML), questionType: QuestionType.ORDERING, }); - expect( - screen.getByRole('button', { name: tr.$tr('moveItemUpBtn', { number: 1 }) }), - ).toBeDisabled(); - expect( - screen.getByRole('button', { name: tr.$tr('moveItemUpBtn', { number: 2 }) }), - ).toBeEnabled(); + // `v-show` hides the out-of-range control, dropping it out of the accessibility tree + expect(screen.queryByRole('button', { name: moveUpName(1) })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: moveUpName(2) })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: moveUpName(3) })).toBeInTheDocument(); + + expect(screen.getByRole('button', { name: moveDownName(1) })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: moveDownName(2) })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: moveDownName(3) })).not.toBeInTheDocument(); }); - it('disables move-down button for the last item', () => { - renderEditor({ + it('reorders the items when a row is dragged to a new position', async () => { + const { emitted } = renderEditor({ interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML), questionType: QuestionType.ORDERING, }); - expect( - screen.getByRole('button', { name: tr.$tr('moveItemDownBtn', { number: 3 }) }), - ).toBeDisabled(); - expect( - screen.getByRole('button', { name: tr.$tr('moveItemDownBtn', { number: 1 }) }), - ).toBeEnabled(); + await dragFirstRowToLast(); + + const { bodyXml } = emitted()['update:interaction'].pop()[0]; + expect(bodyXml.indexOf('identifier="order_aaa11111"')).toBeGreaterThan( + bodyXml.indexOf('identifier="order_ccc33333"'), + ); + }); + + it('reorders the items when a row is moved down by keyboard', async () => { + const user = userEvent.setup(); + const { emitted } = renderEditor({ + interaction: blockWithDecl(ORDERING_XML, ORDERING_DECL_XML), + questionType: QuestionType.ORDERING, + }); + await user.click(screen.getByRole('button', { name: moveDownName(1) })); + const { bodyXml } = emitted()['update:interaction'].pop()[0]; + expect(bodyXml.indexOf('identifier="order_bbb22222"')).toBeLessThan( + bodyXml.indexOf('identifier="order_aaa11111"'), + ); }); it('disables delete button when only one item remains', async () => { @@ -176,6 +217,7 @@ describe('OrderingInteractionEditor', () => { expect( screen.queryByRole('button', { name: tr.$tr('deleteItemBtn', { number: 1 }) }), ).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: moveDownName(1) })).not.toBeInTheDocument(); }); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js index 8fcd7fe878..503a844152 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/qtiEditorStrings.js @@ -79,13 +79,10 @@ export const qtiEditorStrings = createTranslator('QTIEditorStrings', { message: 'Delete option {number}', context: 'Accessible label for the delete icon button next to an ordering item row', }, - moveItemUpBtn: { - message: 'Move option {number} up', - context: 'Accessible label for the move-up icon button next to an ordering item row', - }, - moveItemDownBtn: { - message: 'Move option {number} down', - context: 'Accessible label for the move-down icon button next to an ordering item row', + orderingItemLabel: { + message: 'Option {number}', + context: + 'Names an ordering row in its reorder controls and in the screen reader announcement after it moves', }, errorTooFewChoices: { message: 'At least 2 items are required for an ordering question.', From d53a9eb920a9d452a82a79b16aa95f1dc86e384c Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Tue, 1 Sep 2026 05:49:21 -0700 Subject: [PATCH 7/9] Show an invalid QTI row's error on its leading control 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) --- .../choice/ChoiceInteractionEditor.vue | 18 ++++++++++-------- .../__tests__/ChoiceInteractionEditor.spec.js | 19 +++++++++++++++++++ .../ordering/OrderingInteractionEditor.vue | 13 +++++++++---- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue index 4b87a347f4..a203e9e9fa 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/ChoiceInteractionEditor.vue @@ -142,7 +142,8 @@
      - +
      { expect(screen.getAllByRole('alert').length).toBeGreaterThan(0); }); + it('replaces the selection control with the error icon on an invalid choice', async () => { + jest.useFakeTimers(); + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + renderEditor({ + interaction: block(CHOICE_SINGLE_SELECT_XML), + questionType: QuestionType.SINGLE_SELECT, + }); + expect(screen.getAllByRole('radio')).toHaveLength(3); + // The added choice starts empty, so the debounced validation flags it. + await user.click(screen.getByRole('button', { name: /add choice/i })); + await nextTick(); + jest.advanceTimersByTime(400); + await nextTick(); + jest.useRealTimers(); + // Four rows, but the invalid one shows the error icon where its radio was + expect(screen.getAllByRole('button', { name: tr.$tr('deleteChoiceBtn') })).toHaveLength(4); + expect(screen.getAllByRole('radio')).toHaveLength(3); + }); + it('shows no-correct-choice error after toggling and running validation', async () => { jest.useFakeTimers(); renderEditor({ diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue index 5e272bc328..78ec470450 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/OrderingInteractionEditor.vue @@ -122,8 +122,9 @@