T1320189 - List - TypeError: Cannot read properties of null (reading 'get') occurs when using Shift + Up Arrow on the first item #32334
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a list keyboard-reordering edge case where using Shift+Arrow on boundary items could cause a runtime error, and adds regression tests to cover the behavior.
Changes:
- Guard
ListEdit’s keyboard-based reordering logic against missing destination items so Shift+Arrow on out-of-range indices exits safely without throwing. - Add QUnit tests to verify that Shift+ArrowUp on the first item and Shift+ArrowDown on the last item do not throw and keep the item order unchanged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/devextreme/js/__internal/ui/list/list.edit.ts |
Adds a null check for the next item element before computing group boundaries and calling reorderItem, preventing a TypeError when trying to move beyond list bounds. |
packages/devextreme/testing/tests/DevExpress.ui.widgets/listParts/editingTests.js |
Introduces two keyboard navigation tests that reproduce the Shift+ArrowUp/Down boundary scenarios and assert that the items order remains unchanged (regression coverage for T1320189). |
| assert.deepEqual(list.option('items'), items, 'items were reordered'); | ||
| }); | ||
|
|
||
| QUnit.test('shift+arrowUp on first item should not throw error', function(assert) { |
There was a problem hiding this comment.
I don't see the test that would check if error isn't thrown, could you please add it to make the test title and its body connected? As well as for the second test, to make that I suppose you can use sinon methods smth like:
const consoleSpy = sinon.spy(console, 'error');
...
assert.strictEqual(consoleSpy.callCount, 0, 'no error messages in console');
because currently it seems that old behaviour would pass these tests as well since before items order wasn't changed too, isn't it?
also you can actually combine them in one using forEach syntax
or
if this behaviour actually throws an error it could be handled by standard QUnit methods
2 participants
No description provided.