TreeView: provide a way to control ability to select nodes - change disabled behavior#32512
TreeView: provide a way to control ability to select nodes - change disabled behavior#32512dmlvr wants to merge 28 commits intoDevExpress:26_1from
Conversation
| assert.ok(isNodeExpanded($node2), 'second node is expanded'); | ||
| assert.equal(getNodeItemId($node2), 11, 'id for second node'); | ||
|
|
||
| assert.notOk(isNodeExpanded($node3), 'third node is expanded'); | ||
| assert.ok(isNodeExpanded($node3), 'third node is expanded'); | ||
| assert.equal(getNodeItemId($node3), 111, 'id for third node'); |
There was a problem hiding this comment.
If you see such things don't hesitate to refactor them to strictEqual since we decided to use this syntax to make assertions a bit more readable and predictable:
assert.strictEqual(isNodeExpanded($node2), true, ...
assert.strictEqual(getNodeItemId($node2), 11, ...There was a problem hiding this comment.
fixed all equal to strictEqual in the affected tests
07f8213 to
84a35e7
Compare
There was a problem hiding this comment.
Pull request overview
This PR changes how disabled TreeView nodes behave to provide better accessibility and user experience. The core change moves the disabled state from the item frame to the item content, allowing users to still navigate, expand, and collapse disabled nodes while maintaining a visual disabled state.
Changes:
- Modified disabled TreeView node behavior: disabled nodes can now be expanded/collapsed/focused, with disabled visual state applied only to content
- Updated all test assertions from
assert.equaltoassert.strictEqualfor stricter type checking - Added SCSS styling for disabled item content across all themes (generic, material, fluent)
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts | Moved disabled class and aria-disabled from item frame to item content; removed disabled state checks for expand/collapse operations |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/treeViewParts/optionChanged.js | Updated test assertions to check item content for disabled class; upgraded assert.equal to assert.strictEqual |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/treeViewParts/keyboardNavigation.js | Updated tests to reflect that disabled items can now be focused and navigated; upgraded assert.equal to assert.strictEqual |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.virtualMode.tests.js | Updated test assertions for disabled item expansion; upgraded assert.equal to assert.strictEqual |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.markup.tests.js | Updated markup tests to check item content for disabled class; upgraded assert.equal to assert.strictEqual |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.expanded.tests.js | Updated tests to reflect disabled items can now expand/collapse; upgraded assert.equal to assert.strictEqual |
| packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.accessibility.tests.js | Removed aria-disabled check from node element as it's now on content |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/columnChooser.integration.tests.js | Updated selector to check item content for disabled class |
| packages/testcafe-models/dataGrid/columnChooser.ts | Updated TestCafe model to check item content for disabled class |
| packages/devextreme-scss/scss/widgets/material/treeView/_index.scss | Added disabled state styling for item content |
| packages/devextreme-scss/scss/widgets/material/treeView/_colors.scss | Added disabled color variable |
| packages/devextreme-scss/scss/widgets/generic/treeView/_index.scss | Added disabled state styling for item content |
| packages/devextreme-scss/scss/widgets/generic/treeView/_colors.scss | Added disabled color variables for all generic themes |
| packages/devextreme-scss/scss/widgets/fluent/treeView/_index.scss | Added disabled state styling for item content and disabled checkbox focus states |
| packages/devextreme-scss/scss/widgets/fluent/treeView/_colors.scss | Added disabled color and checkbox variables |
| packages/devextreme-scss/scss/widgets/base/treeView/_common.scss | Removed disabled opacity and cursor styles from item frame and toggle icons |
packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts:1580
- The code assumes
$itemContentwill always exist by using.eq(0). If for some reason the item content is not rendered, this could cause silent failures when toggling disabled state. Consider adding a check:if ($itemContent.length === 0) return;before attempting to toggle classes to ensure robustness.
const $itemContent = $node.find(`.${ITEM_CONTENT_CLASS}`).eq(0);
this._dataAdapter.toggleNodeDisabledState(node.internalFields.key, state);
$itemContent.toggleClass(DISABLED_STATE_CLASS, !!state);
if (this._showCheckboxes()) {
const checkbox = this._getCheckBoxInstance($node);
checkbox.option('disabled', !!state);
}
}
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeViewParts/optionChanged.js
Outdated
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeViewParts/optionChanged.js
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.expanded.tests.js:265
- The test "expanded disabled item should not collapse on click" (line 251-265) appears to be outdated and should be updated to reflect the new behavior. With the removal of the disabled check in
_toggleExpandedState(tree_view.base.ts line 1151-1160), expanded disabled items with standard toggle icons should now be able to collapse, just like disabled items with custom expander icons. This test still expects disabled items NOT to collapse when clicking the standard toggle icon, which contradicts the implementation changes and the updated test for custom expander icons at line 295.
test('expanded disabled item should not collapse on click', function(assert) {
const data = $.extend(true, [], DATA[5]);
data[0].expanded = true;
data[0].disabled = true;
const $treeView = initTree({
items: data
});
const treeView = $treeView.dxTreeView('instance');
const $firstItem = $treeView.find(`.${TREEVIEW_ITEM_CLASS}`).eq(0);
const $icon = $firstItem.parent().find(`> .${TREEVIEW_TOGGLE_ITEM_VISIBILITY_CLASS}`);
$icon.trigger('dxclick');
assert.ok(treeView.option('items')[0].expanded, 'disabled item was not expanded');
});
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.expanded.tests.js
Outdated
Show resolved
Hide resolved
1a4f5ec to
44bd409
Compare
packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts
Outdated
Show resolved
Hide resolved
packages/devextreme-scss/scss/widgets/material/treeView/_colors.scss
Outdated
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.ui.widgets/fileManagerParts/editing.tests.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.expanded.tests.js:249
- There's an inconsistency in the expected behavior for disabled items. The test at line 220-233 expects that disabled items with default expander icons CAN expand, while this test expects that disabled items with custom expander icons CANNOT expand. Both types use the same
_renderToggleItemVisibilityIconClickmethod (tree_view.base.ts lines 1214, 1232) which calls_toggleExpandedState, and the disabled check was removed from_toggleExpandedState(tree_view.base.ts lines 1149-1181). This suggests both should behave the same way - either both should expand or neither should. Please verify if this test expectation is correct or if it should be updated to align with the new behavior where disabled items can expand.
test('disabled item with custom expander icons should not expand on click', function(assert) {
const data = $.extend(true, [], DATA[5]);
data[0].disabled = true;
const $treeView = initTree({
items: data,
expandIcon: 'add',
collapseIcon: 'add',
});
const treeView = $treeView.dxTreeView('instance');
const $icon = $(`.${TREEVIEW_CUSTOM_EXPAND_ICON_CLASS}`);
$icon.trigger('dxclick');
assert.notOk(treeView.option('items')[0].expanded, 'disabled item was not expanded');
});
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.markup.tests.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
packages/devextreme/testing/tests/DevExpress.ui.widgets/treeView.expanded.tests.js:249
- This test expects disabled items with custom expander icons NOT to expand on click, which contradicts the new behavior implemented in this PR. With the removal of the disabled check in
_toggleExpandedState(tree_view.base.ts lines 1149-1181), disabled items should now be able to expand/collapse regardless of whether they use custom or default expander icons. This test should either be updated to expect expansion to succeed, or additional logic should be added to prevent expansion for disabled items with custom icons (if that's the intended behavior). The current state creates an inconsistency: line 220-233 shows disabled items with default icons CAN expand, but this test expects disabled items with custom icons NOT to expand.
test('disabled item with custom expander icons should not expand on click', function(assert) {
const data = $.extend(true, [], DATA[5]);
data[0].disabled = true;
const $treeView = initTree({
items: data,
expandIcon: 'add',
collapseIcon: 'add',
});
const treeView = $treeView.dxTreeView('instance');
const $icon = $(`.${TREEVIEW_CUSTOM_EXPAND_ICON_CLASS}`);
$icon.trigger('dxclick');
assert.notOk(treeView.option('items')[0].expanded, 'disabled item was not expanded');
});
packages/devextreme-scss/scss/widgets/fluent/treeView/_colors.scss
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Dmitry Lavrinovich <52966626+dmlvr@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/ui/tree_view/tree_view.base.ts:2111
_getClosestNonDisabledNodeuses$el.find(...)to detect disabled state. Becausefindsearches all descendants, a parent node can be treated as “disabled” just because it contains any disabled child node, which can break focus/collapse navigation in mixed enabled/disabled trees. This check should be limited to the current node’s own item (e.g. direct child.dx-treeview-item > .dx-treeview-item-content.dx-state-disabled).
_getClosestNonDisabledNode($node: dxElementWrapper): dxElementWrapper {
const isNodeDisabled = ($el: dxElementWrapper): boolean => $el.find(`.${ITEM_CLASS} .${ITEM_CONTENT_CLASS}.${DISABLED_STATE_CLASS}`).length > 0;
let currentNode = $node;
do {
currentNode = currentNode.parent().closest(`.${NODE_CLASS}`);
} while (currentNode.length && isNodeDisabled(currentNode));
packages/devextreme-scss/scss/widgets/fluent/treeView/_colors.scss
Outdated
Show resolved
Hide resolved
…thub.com:dmlvr/DevExtreme into 3249_26_1_tree_view_control_ability_select_nodes
No description provided.