From 34a4c982b26f534eee21d800387761138aa0be8d Mon Sep 17 00:00:00 2001 From: arimieandreea Date: Fri, 20 Mar 2026 16:52:26 +0200 Subject: [PATCH 1/3] fix(categorize): handle drag targets correctly PD-5514 --- .../categorize/configure/src/design/index.jsx | 86 +++++++----- packages/categorize/docs/demo/generate.js | 127 ++++++++++-------- 2 files changed, 119 insertions(+), 94 deletions(-) diff --git a/packages/categorize/configure/src/design/index.jsx b/packages/categorize/configure/src/design/index.jsx index 74bfa85aa1..e87f079ad7 100644 --- a/packages/categorize/configure/src/design/index.jsx +++ b/packages/categorize/configure/src/design/index.jsx @@ -226,55 +226,69 @@ export class Design extends React.Component { }); }; - onDragEnd = (event) => { - const { active, over } = event; - + onDragEnd = ({ active, over }) => { this.setState({ activeDragItem: null }); + if (!active) return; + + const { model } = this.props; + const { allowAlternateEnabled, categories = [], choices = [] } = model; + + const activeData = active?.data?.current; + const overData = over?.data?.current; + + if (!activeData) return; - if (!over || !active) { + const choiceIndex = activeData.choiceIndex || 0; + const overType = overData?.type; + const isPreview = activeData.type === 'choice-preview'; + const isNewChoice = activeData.type === 'choice'; + + const choiceId = + activeData.choice?.id || (typeof activeData.id === 'string' ? activeData.id.split('-')[0] : activeData.id); + + if (isPreview && (!overData || overType === 'choice')) { + this.removeChoiceFromSource(activeData, choiceIndex, { allowAlternateEnabled, categories, choices }); return; } - const { model } = this.props; - const { allowAlternateEnabled } = model; - const activeData = active.data.current; - const overData = over.data.current; - - // moving a choice between categories (correct response) - if (activeData.type === 'choice-preview' && overData.type === 'category') { - // Extract original choice.id - if DraggableChoice uses the unique id in data, extract the first part - // Format: ${choice.id}-${categoryId}-${choiceIndex} or ${choice.id}-${categoryId}-${choiceIndex}-alt-${alternateResponseIndex} - const choiceId = - activeData.choice?.id || (typeof activeData.id === 'string' ? activeData.id.split('-')[0] : activeData.id); - this.moveChoice(choiceId, activeData.categoryId, overData.id, activeData.choiceIndex || 0); + if (isPreview && overType === 'category') { + return this.moveChoice(choiceId, activeData.categoryId, overData.id, choiceIndex); } - // placing a choice into a category (correct response) - if (activeData.type === 'choice' && overData.type === 'category') { - this.addChoiceToCategory({ id: activeData.id }, overData.id); + if (isNewChoice && overType === 'category') { + return this.addChoiceToCategory({ id: activeData.id }, overData.id); } - // moving a choice between categories (alternate response) - if (activeData.type === 'choice-preview' && overData.type === 'category-alternate') { - const toAlternateIndex = overData.alternateResponseIndex; - // Extract original choice.id - if DraggableChoice uses the unique id in data, extract the first part - const choiceId = - activeData.choice?.id || (typeof activeData.id === 'string' ? activeData.id.split('-')[0] : activeData.id); - this.moveChoiceInAlternate( + if (isPreview && overType === 'category-alternate') { + return this.moveChoiceInAlternate( choiceId, activeData.categoryId, overData.id, - activeData.choiceIndex || 0, - toAlternateIndex, + choiceIndex, + overData.alternateResponseIndex, ); } - // placing a choice into a category (alternate response) - if (allowAlternateEnabled && activeData.type === 'choice' && overData.type === 'category-alternate') { - const choiceId = activeData.id; - const categoryId = overData.id; - const toAlternateResponseIndex = overData.alternateResponseIndex; - this.addChoiceToAlternateCategory({ id: choiceId }, categoryId, toAlternateResponseIndex); + if (allowAlternateEnabled && isNewChoice && overType === 'category-alternate') { + return this.addChoiceToAlternateCategory({ id: activeData.id }, overData.id, overData.alternateResponseIndex); + } + }; + + removeChoiceFromSource = (activeData, choiceIndex, { allowAlternateEnabled, categories, choices }) => { + const isAlternateSource = activeData.alternateResponseIndex !== undefined; + + if (!isAlternateSource) { + this.deleteChoiceFromCategory(activeData.categoryId, activeData.choiceId, choiceIndex); + return; + } + + if (!allowAlternateEnabled) return; + + const category = categories?.find((c) => c.id === activeData.categoryId); + const choice = choices?.find((c) => c.id === activeData.choiceId); + + if (category && choice) { + this.deleteChoiceFromAlternateCategory(category, choice, choiceIndex, activeData.alternateResponseIndex); } }; @@ -299,9 +313,9 @@ export class Design extends React.Component { }); }; - deleteChoiceFromCategory = (category, choice, choiceIndex) => { + deleteChoiceFromCategory = (categoryId, choiceId, choiceIndex) => { const { model } = this.props; - const correctResponse = removeChoiceFromCategory(choice.id, category.id, choiceIndex, model.correctResponse); + const correctResponse = removeChoiceFromCategory(choiceId, categoryId, choiceIndex, model.correctResponse); this.updateModel({ correctResponse }); }; diff --git a/packages/categorize/docs/demo/generate.js b/packages/categorize/docs/demo/generate.js index b5bfa405d3..9eddfeb4a1 100644 --- a/packages/categorize/docs/demo/generate.js +++ b/packages/categorize/docs/demo/generate.js @@ -279,79 +279,90 @@ exports.model = (id, element) => ({ id, element, - 'teacherInstructions': '', - 'promptEnabled': true, - 'prompt': '

Move the equations to tell whether they are true or false.

', - 'categories': [ + promptEnabled: true, + choices: [ { - 'id': '0', - 'label': '' + id: '0', + content: + '
420\\text{ cm}=4.2\\text{ meters}
', + categoryCount: 0, + correctResponseCount: 1, }, { - 'id': '1', - 'label': '' - } - ], - 'categoriesPerRow': 2, - 'allowMaxChoicesPerCategory': false, - 'maxChoicesPerCategory': 0, - 'rowLabels': [], - 'choices': [ + id: '1', + content: + '
3.4\\text{ kg}=340\\text{ g}
', + categoryCount: 0, + correctResponseCount: 1, + }, { - 'id': '0', - 'content': '\\(6+3=9-2\\)', - 'categoryCount': 1, - 'correctResponseCount': 1 + id: '2', + content: + '
1\\text{,}800\\text{ mL}=180\\text{ L}
', + categoryCount: 0, + correctResponseCount: 0, }, { - 'id': '1', - 'content': '\\(10-4=9-5\\)', - 'categoryCount': 1, - 'correctResponseCount': 1 + id: '3', + content: + '
3.5\\text{ meters}=350\\text{ cm}
', + categoryCount: 0, + correctResponseCount: 1, + }, + { + id: '4', + content: + '
4\\text{,}800\\text{ g}=0.48\\text{ kg}
', + categoryCount: 0, + correctResponseCount: 1, }, { - 'id': '2', - 'content': '\\(17-9=9+17\\)', - 'categoryCount': 1, - 'correctResponseCount': 1 + id: '5', + content: + '
250\\text{ mL}=0.25\\text{ L}
', + categoryCount: 0, + correctResponseCount: 0, }, + ], + categories: [ { - 'id': '3', - 'content': '\\(11+9=10+10\\)', - 'categoryCount': 1, - 'correctResponseCount': 1 + id: '0', + label: '
Equivalent
', }, { - 'id': '4', - 'content': '\\(14-4=5+5\\)', - 'categoryCount': 1, - 'correctResponseCount': 1 - } + id: '1', + label: '
NOT equivalent
', + }, ], - 'choicesPosition': 'above', - 'choicesLabel': '', - 'lockChoiceOrder': true, - 'allowMultiplePlacementsEnabled': 'No', - 'correctResponse': [ + rowLabels: ['
'], + correctResponse: [ { - 'category': '0', - 'choices': [ - '3', - '4' - ] + category: '0', + choices: ['0', '3'], }, { - 'category': '1', - 'choices': [ - '0', - '1', - '2' - ] - } + category: '1', + choices: ['4', '1'], + }, ], - 'allowAlternateEnabled': false, - 'alternates': [], - 'partialScoring': true, - 'minRowHeight': '175px', - 'fontSizeFactor': 1.3 + allowAlternateEnabled: true, + allowMaxChoicesPerCategory: false, + allowMultiplePlacementsEnabled: 'Yes', + alternates: [], + categoriesPerRow: 2, + choicesLabel: '', + choicesPosition: 'below', + feedbackEnabled: false, + lockChoiceOrder: true, + maxAnswerChoices: 6, + maxChoicesPerCategory: 0, + partialScoring: true, + rationaleEnabled: true, + studentInstructionsEnabled: true, + teacherInstructionsEnabled: true, + toolbarEditorPosition: 'bottom', + minRowHeight: '80px', + teacherInstructions: '
', + prompt: '
', + rationale: '
', }); From d08c28fdc1aead7a4ed58a79309177467f1cacd0 Mon Sep 17 00:00:00 2001 From: arimieandreea Date: Tue, 24 Mar 2026 10:13:36 +0200 Subject: [PATCH 2/3] fix(categorize): set back old model from docs PD-5514 --- packages/categorize/docs/demo/generate.js | 127 ++++++++++------------ 1 file changed, 58 insertions(+), 69 deletions(-) diff --git a/packages/categorize/docs/demo/generate.js b/packages/categorize/docs/demo/generate.js index 9eddfeb4a1..b5bfa405d3 100644 --- a/packages/categorize/docs/demo/generate.js +++ b/packages/categorize/docs/demo/generate.js @@ -279,90 +279,79 @@ exports.model = (id, element) => ({ id, element, - promptEnabled: true, - choices: [ - { - id: '0', - content: - '
420\\text{ cm}=4.2\\text{ meters}
', - categoryCount: 0, - correctResponseCount: 1, - }, + 'teacherInstructions': '', + 'promptEnabled': true, + 'prompt': '

Move the equations to tell whether they are true or false.

', + 'categories': [ { - id: '1', - content: - '
3.4\\text{ kg}=340\\text{ g}
', - categoryCount: 0, - correctResponseCount: 1, + 'id': '0', + 'label': '' }, { - id: '2', - content: - '
1\\text{,}800\\text{ mL}=180\\text{ L}
', - categoryCount: 0, - correctResponseCount: 0, - }, + 'id': '1', + 'label': '' + } + ], + 'categoriesPerRow': 2, + 'allowMaxChoicesPerCategory': false, + 'maxChoicesPerCategory': 0, + 'rowLabels': [], + 'choices': [ { - id: '3', - content: - '
3.5\\text{ meters}=350\\text{ cm}
', - categoryCount: 0, - correctResponseCount: 1, + 'id': '0', + 'content': '\\(6+3=9-2\\)', + 'categoryCount': 1, + 'correctResponseCount': 1 }, { - id: '4', - content: - '
4\\text{,}800\\text{ g}=0.48\\text{ kg}
', - categoryCount: 0, - correctResponseCount: 1, + 'id': '1', + 'content': '\\(10-4=9-5\\)', + 'categoryCount': 1, + 'correctResponseCount': 1 }, { - id: '5', - content: - '
250\\text{ mL}=0.25\\text{ L}
', - categoryCount: 0, - correctResponseCount: 0, + 'id': '2', + 'content': '\\(17-9=9+17\\)', + 'categoryCount': 1, + 'correctResponseCount': 1 }, - ], - categories: [ { - id: '0', - label: '
Equivalent
', + 'id': '3', + 'content': '\\(11+9=10+10\\)', + 'categoryCount': 1, + 'correctResponseCount': 1 }, { - id: '1', - label: '
NOT equivalent
', - }, + 'id': '4', + 'content': '\\(14-4=5+5\\)', + 'categoryCount': 1, + 'correctResponseCount': 1 + } ], - rowLabels: ['
'], - correctResponse: [ + 'choicesPosition': 'above', + 'choicesLabel': '', + 'lockChoiceOrder': true, + 'allowMultiplePlacementsEnabled': 'No', + 'correctResponse': [ { - category: '0', - choices: ['0', '3'], + 'category': '0', + 'choices': [ + '3', + '4' + ] }, { - category: '1', - choices: ['4', '1'], - }, + 'category': '1', + 'choices': [ + '0', + '1', + '2' + ] + } ], - allowAlternateEnabled: true, - allowMaxChoicesPerCategory: false, - allowMultiplePlacementsEnabled: 'Yes', - alternates: [], - categoriesPerRow: 2, - choicesLabel: '', - choicesPosition: 'below', - feedbackEnabled: false, - lockChoiceOrder: true, - maxAnswerChoices: 6, - maxChoicesPerCategory: 0, - partialScoring: true, - rationaleEnabled: true, - studentInstructionsEnabled: true, - teacherInstructionsEnabled: true, - toolbarEditorPosition: 'bottom', - minRowHeight: '80px', - teacherInstructions: '
', - prompt: '
', - rationale: '
', + 'allowAlternateEnabled': false, + 'alternates': [], + 'partialScoring': true, + 'minRowHeight': '175px', + 'fontSizeFactor': 1.3 }); From 6b6d151a73d93199e57a08690ff4dfefe0189bc2 Mon Sep 17 00:00:00 2001 From: arimieandreea Date: Tue, 24 Mar 2026 11:16:36 +0200 Subject: [PATCH 3/3] fix(categorize): add tests for new onDrag extension, handle invalid target student PD-5514 --- .../src/design/__tests__/index.test.jsx | 114 ++++++++++++++++++ .../src/categorize/__tests__/index.test.jsx | 101 +++++++++++++++- packages/categorize/src/categorize/index.jsx | 63 +++++----- 3 files changed, 246 insertions(+), 32 deletions(-) diff --git a/packages/categorize/configure/src/design/__tests__/index.test.jsx b/packages/categorize/configure/src/design/__tests__/index.test.jsx index 80b9784d21..5e772dcdaa 100644 --- a/packages/categorize/configure/src/design/__tests__/index.test.jsx +++ b/packages/categorize/configure/src/design/__tests__/index.test.jsx @@ -105,4 +105,118 @@ describe('Design', () => { expect(container).toBeInTheDocument(); }); }); + + describe('onDragEnd', () => { + const createInstance = (modelExtras = {}) => { + const instance = new Design({ + model: model({ + allowAlternateEnabled: true, + ...modelExtras, + }), + onChange, + }); + + instance.setState = jest.fn(); + instance.removeChoiceFromSource = jest.fn(); + instance.moveChoice = jest.fn(); + instance.addChoiceToCategory = jest.fn(); + instance.moveChoiceInAlternate = jest.fn(); + instance.addChoiceToAlternateCategory = jest.fn(); + + return instance; + }; + + const eventFor = ({ activeData, overData }) => ({ + active: activeData ? { data: { current: activeData } } : null, + over: overData ? { data: { current: overData } } : null, + }); + + it('routes choice-preview with no target to removeChoiceFromSource', () => { + const instance = createInstance(); + const activeData = { + type: 'choice-preview', + id: 'c1-cat1-0', + categoryId: '1', + choiceIndex: 0, + }; + + instance.onDragEnd(eventFor({ activeData })); + + expect(instance.removeChoiceFromSource).toHaveBeenCalledWith( + activeData, + 0, + expect.objectContaining({ + allowAlternateEnabled: true, + categories: expect.any(Array), + choices: expect.any(Array), + }), + ); + }); + + it('routes choice-preview dropped on choice pool to removeChoiceFromSource', () => { + const instance = createInstance(); + const activeData = { + type: 'choice-preview', + id: 'c1-cat1-0', + categoryId: '1', + choiceIndex: 0, + }; + const overData = { type: 'choice' }; + + instance.onDragEnd(eventFor({ activeData, overData })); + + expect(instance.removeChoiceFromSource).toHaveBeenCalled(); + expect(instance.moveChoice).not.toHaveBeenCalled(); + }); + + it('routes choice-preview dropped on category to moveChoice', () => { + const instance = createInstance(); + const activeData = { + type: 'choice-preview', + id: 'c1-cat1-0', + categoryId: '1', + choiceIndex: 2, + }; + const overData = { type: 'category', id: '2' }; + + instance.onDragEnd(eventFor({ activeData, overData })); + + expect(instance.moveChoice).toHaveBeenCalledWith('c1', '1', '2', 2); + }); + + it('routes new choice dropped on category to addChoiceToCategory', () => { + const instance = createInstance(); + const activeData = { type: 'choice', id: '9' }; + const overData = { type: 'category', id: '2' }; + + instance.onDragEnd(eventFor({ activeData, overData })); + + expect(instance.addChoiceToCategory).toHaveBeenCalledWith({ id: '9' }, '2'); + }); + + it('routes choice-preview dropped on category-alternate to moveChoiceInAlternate', () => { + const instance = createInstance(); + const activeData = { + type: 'choice-preview', + id: 'c1-cat1-0', + categoryId: '1', + choiceIndex: 1, + }; + const overData = { type: 'category-alternate', id: '2', alternateResponseIndex: 3 }; + + instance.onDragEnd(eventFor({ activeData, overData })); + + expect(instance.moveChoiceInAlternate).toHaveBeenCalledWith('c1', '1', '2', 1, 3); + }); + + it('routes new choice dropped on category-alternate to addChoiceToAlternateCategory', () => { + const instance = createInstance({ allowAlternateEnabled: true }); + const activeData = { type: 'choice', id: '11' }; + const overData = { type: 'category-alternate', id: '2', alternateResponseIndex: 4 }; + + instance.onDragEnd(eventFor({ activeData, overData })); + + expect(instance.addChoiceToAlternateCategory).toHaveBeenCalledWith({ id: '11' }, '2', 4); + }); + }); }); diff --git a/packages/categorize/src/categorize/__tests__/index.test.jsx b/packages/categorize/src/categorize/__tests__/index.test.jsx index 3cc68a909a..6f44ca901e 100644 --- a/packages/categorize/src/categorize/__tests__/index.test.jsx +++ b/packages/categorize/src/categorize/__tests__/index.test.jsx @@ -2,6 +2,7 @@ import React from 'react'; import { render } from '@testing-library/react'; import { ThemeProvider, createTheme } from '@mui/material/styles'; import { Categorize } from '../index'; +import CategorizeProvider from '../index'; jest.mock('@pie-lib/drag', () => ({ uid: { @@ -98,7 +99,7 @@ describe('categorize', () => { return render( - + , ); }; @@ -137,4 +138,102 @@ describe('categorize', () => { expect(container).toBeInTheDocument(); }); }); + + describe('provider onDragEnd', () => { + const createProvider = (extras = {}) => { + const instance = new CategorizeProvider({ + ...defaultProps, + onAnswersChange: jest.fn(), + onShowCorrectToggle: jest.fn(), + resumeMathObserver: jest.fn(), + ...extras, + }); + + instance.setState = jest.fn(); + instance.categorizeRef = { + removeChoice: jest.fn(), + dropChoice: jest.fn(), + }; + + return instance; + }; + + const dndEvent = ({ activeData, overId, overData }) => ({ + active: activeData ? { data: { current: activeData } } : null, + over: overId || overData ? { id: overId, data: { current: overData } } : null, + }); + + it('removes choice from source when dropped outside valid target', () => { + const provider = createProvider(); + const event = dndEvent({ + activeData: { + id: 'c1', + type: 'choice', + categoryId: 'cat-1', + choiceIndex: 0, + value: 'v1', + itemType: 'categorize', + }, + }); + + provider.onDragEnd(event); + + expect(provider.categorizeRef.removeChoice).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'c1', + categoryId: 'cat-1', + choiceIndex: 0, + }), + ); + expect(provider.categorizeRef.dropChoice).not.toHaveBeenCalled(); + }); + + it('removes choice from source when dropped on choices-board', () => { + const provider = createProvider(); + const event = dndEvent({ + overId: 'choices-board', + overData: { itemType: 'categorize' }, + activeData: { + id: 'c2', + type: 'choice', + categoryId: 'cat-2', + choiceIndex: 1, + value: 'v2', + itemType: 'categorize', + }, + }); + + provider.onDragEnd(event); + + expect(provider.categorizeRef.removeChoice).toHaveBeenCalled(); + expect(provider.categorizeRef.dropChoice).not.toHaveBeenCalled(); + }); + + it('drops choice into category when valid category target exists', () => { + const provider = createProvider(); + const event = dndEvent({ + overId: 'cat-target', + overData: { itemType: 'categorize' }, + activeData: { + id: 'c3', + type: 'choice', + categoryId: 'cat-source', + choiceIndex: 2, + value: 'v3', + itemType: 'categorize', + }, + }); + + provider.onDragEnd(event); + + expect(provider.categorizeRef.dropChoice).toHaveBeenCalledWith( + 'cat-target', + expect.objectContaining({ + id: 'c3', + categoryId: 'cat-source', + choiceIndex: 2, + }), + ); + }); + }); }); diff --git a/packages/categorize/src/categorize/index.jsx b/packages/categorize/src/categorize/index.jsx index 5bdec56fcc..e250da6c1a 100644 --- a/packages/categorize/src/categorize/index.jsx +++ b/packages/categorize/src/categorize/index.jsx @@ -21,7 +21,7 @@ class DragPreviewWrapper extends React.Component { static propTypes = { children: PropTypes.node, }; - + containerRef = React.createRef(); componentDidMount() { @@ -371,15 +371,10 @@ class CategorizeProvider extends React.Component { // Check if drop is valid const draggedItem = active?.data?.current; const overData = over?.data?.current; - const isValidDrop = - over && - active && - draggedItem && - draggedItem.type === 'choice' && - overData && - overData.itemType === 'categorize'; - - this.setState({ + const isValidDrop = + over && active && draggedItem && draggedItem.type === 'choice' && overData && overData.itemType === 'categorize'; + + this.setState({ activeDragItem: null, isValidDrop: isValidDrop, }); @@ -388,30 +383,36 @@ class CategorizeProvider extends React.Component { resumeMathObserver(); } - if (!over || !active) { + if (!active || !draggedItem || draggedItem.type !== 'choice') { + return; + } + + const choiceData = { + id: draggedItem.id, + categoryId: draggedItem.categoryId, + choiceIndex: draggedItem.choiceIndex, + value: draggedItem.value, + itemType: draggedItem.itemType, + }; + + // Dropped outside a valid/known target: remove from source category, + // which returns the choice to the choices pool. + if (!over) { + if (this.categorizeRef && this.categorizeRef.removeChoice && draggedItem.categoryId) { + this.categorizeRef.removeChoice(choiceData); + } return; } - if (draggedItem && draggedItem.type === 'choice') { - const choiceData = { - id: draggedItem.id, - categoryId: draggedItem.categoryId, - choiceIndex: draggedItem.choiceIndex, - value: draggedItem.value, - itemType: draggedItem.itemType, - }; - - if (over.id === 'choices-board') { - if (this.categorizeRef && this.categorizeRef.removeChoice && draggedItem.categoryId) { - this.categorizeRef.removeChoice(choiceData); - } - } else { - const categoryId = over.id; - - if (this.categorizeRef && this.categorizeRef.dropChoice) { - this.categorizeRef.dropChoice(categoryId, choiceData); - } + if (over.id === 'choices-board') { + if (this.categorizeRef && this.categorizeRef.removeChoice && draggedItem.categoryId) { + this.categorizeRef.removeChoice(choiceData); } + return; + } + + if (this.categorizeRef && this.categorizeRef.dropChoice) { + this.categorizeRef.dropChoice(over.id, choiceData); } }; @@ -436,7 +437,7 @@ class CategorizeProvider extends React.Component { // Disable drop animation for valid drops to prevent visual snap-back // Keep default animation for invalid drops to show visual feedback const dropAnimation = isValidDrop ? null : undefined; - + return (