Disable tree item #2279
Open
Disable tree item #2279
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 40cfbe0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Collaborator
Author
|
/gemini summary |
Contributor
Summary of ChangesThis pull request implements the functionality to disable individual items within the 'ix-tree' component. This feature provides a clear visual indication and prevents user interaction with specific tree nodes, enhancing the control and clarity of hierarchical data displays. The changes span across the core component, its styling, and its integrations with Angular, React, and Vue, ensuring consistent behavior and appearance. Highlights
Changelog
Activity
|
Collaborator
Author
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a disabled property for the ix-tree-item component, making it available across Angular, React, and Vue frameworks. The changes include updating component interfaces, properties, and rendering logic to reflect the disabled state. Styling was added to visually indicate disabled items with reduced opacity and a 'not-allowed' cursor, and pointer-events: none; was applied to prevent mouse interactions. Event handlers for toggling and clicking tree items were updated to prevent actions on disabled items. New regression tests were added to verify the functionality and styling of disabled tree items. Review comments suggest adding a clarifying comment regarding the onClick checks for disabled items, as pointer-events: none; already prevents mouse events, though the checks remain valuable for programmatic or keyboard interactions. Additionally, it was recommended to refactor the repetitive test setup logic into a helper function for improved maintainability.
Comment on lines
+467
to
+623
| regressionTest( | ||
| 'disabled item should have disabled class', | ||
| async ({ mount, page }) => { | ||
| await mount(` | ||
| <div style="height: 20rem; width: 100%;"> | ||
| <ix-tree root="root"></ix-tree> | ||
| </div> | ||
| `); | ||
|
|
||
| const tree = page.locator('ix-tree'); | ||
|
|
||
| const modelWithDisabled = { | ||
| ...defaultModel, | ||
| 'sample-child-1': { | ||
| ...defaultModel['sample-child-1'], | ||
| disabled: true, | ||
| }, | ||
| }; | ||
|
|
||
| await tree.evaluate( | ||
| (element: HTMLIxTreeElement, [model]) => { | ||
| element.model = model; | ||
| }, | ||
| [modelWithDisabled] | ||
| ); | ||
|
|
||
| await expect(tree).toHaveClass(/hydrated/); | ||
|
|
||
| const sampleItem = tree.locator('ix-tree-item').first(); | ||
| await sampleItem.locator('ix-icon').click(); | ||
|
|
||
| const disabledItem = tree.locator('ix-tree-item', { | ||
| hasText: 'Sample Child 1', | ||
| }); | ||
|
|
||
| await expect(disabledItem).toBeVisible(); | ||
| await expect(disabledItem).toHaveClass(/disabled/); | ||
| } | ||
| ); | ||
|
|
||
| regressionTest( | ||
| 'disabled item should have reduced opacity', | ||
| async ({ mount, page }) => { | ||
| await mount(` | ||
| <div style="height: 20rem; width: 100%;"> | ||
| <ix-tree root="root"></ix-tree> | ||
| </div> | ||
| `); | ||
|
|
||
| const tree = page.locator('ix-tree'); | ||
|
|
||
| const modelWithDisabled = { | ||
| ...defaultModel, | ||
| 'sample-child-1': { | ||
| ...defaultModel['sample-child-1'], | ||
| disabled: true, | ||
| }, | ||
| }; | ||
|
|
||
| await tree.evaluate( | ||
| (element: HTMLIxTreeElement, [model]) => { | ||
| element.model = model; | ||
| }, | ||
| [modelWithDisabled] | ||
| ); | ||
|
|
||
| const sampleItem = tree.locator('ix-tree-item').first(); | ||
| await sampleItem.locator('ix-icon').click(); | ||
|
|
||
| const disabledItem = tree.locator('ix-tree-item', { | ||
| hasText: 'Sample Child 1', | ||
| }); | ||
|
|
||
| await expect(disabledItem).toBeVisible(); | ||
| await expect(disabledItem).toHaveCSS('opacity', '0.5'); | ||
| } | ||
| ); | ||
|
|
||
| regressionTest( | ||
| 'disabled item should have not-allowed cursor', | ||
| async ({ mount, page }) => { | ||
| await mount(` | ||
| <div style="height: 20rem; width: 100%;"> | ||
| <ix-tree root="root"></ix-tree> | ||
| </div> | ||
| `); | ||
|
|
||
| const tree = page.locator('ix-tree'); | ||
|
|
||
| const modelWithDisabled = { | ||
| ...defaultModel, | ||
| 'sample-child-1': { | ||
| ...defaultModel['sample-child-1'], | ||
| disabled: true, | ||
| }, | ||
| }; | ||
|
|
||
| await tree.evaluate( | ||
| (element: HTMLIxTreeElement, [model]) => { | ||
| element.model = model; | ||
| }, | ||
| [modelWithDisabled] | ||
| ); | ||
|
|
||
| const sampleItem = tree.locator('ix-tree-item').first(); | ||
| await sampleItem.locator('ix-icon').click(); | ||
|
|
||
| const disabledItem = tree.locator('ix-tree-item', { | ||
| hasText: 'Sample Child 1', | ||
| }); | ||
|
|
||
| await expect(disabledItem).toBeVisible(); | ||
| await expect(disabledItem).toHaveCSS('cursor', 'not-allowed'); | ||
| } | ||
| ); | ||
|
|
||
| regressionTest( | ||
| 'enabled items should still be selectable when other items are disabled', | ||
| async ({ mount, page }) => { | ||
| await mount(` | ||
| <div style="height: 20rem; width: 100%;"> | ||
| <ix-tree root="root"></ix-tree> | ||
| </div> | ||
| `); | ||
|
|
||
| const tree = page.locator('ix-tree'); | ||
|
|
||
| const modelWithDisabled = { | ||
| ...defaultModel, | ||
| 'sample-child-1': { | ||
| ...defaultModel['sample-child-1'], | ||
| disabled: true, | ||
| }, | ||
| }; | ||
|
|
||
| await tree.evaluate( | ||
| (element: HTMLIxTreeElement, [model]) => { | ||
| element.model = model; | ||
| }, | ||
| [modelWithDisabled] | ||
| ); | ||
|
|
||
| const sampleItem = tree.locator('ix-tree-item').first(); | ||
| await sampleItem.locator('ix-icon').click(); | ||
|
|
||
| const enabledItem = tree.locator('ix-tree-item', { | ||
| hasText: 'Sample Child 2', | ||
| }); | ||
|
|
||
| await expect(enabledItem).toBeVisible(); | ||
| await expect(enabledItem).not.toHaveClass(/disabled/); | ||
|
|
||
| await enabledItem.click(); | ||
|
|
||
| await expect(enabledItem).toHaveClass(/selected/); | ||
| } | ||
| ); |
Contributor
There was a problem hiding this comment.
To improve maintainability and reduce code duplication, you could extract the test setup logic into a helper function. All the new tests share the same setup code.
Here's an example of how you could refactor it:
const setupTreeWithDisabledItem = async (mount: Mount, page: Page) => {
await mount(`
<div style="height: 20rem; width: 100%;">
<ix-tree root="root"></ix-tree>
</div>
`);
const tree = page.locator('ix-tree');
const modelWithDisabled = {
...defaultModel,
'sample-child-1': {
...defaultModel['sample-child-1'],
disabled: true,
},
};
await tree.evaluate(
(element: HTMLIxTreeElement, [model]) => {
element.model = model;
},
[modelWithDisabled]
);
await expect(tree).toHaveClass(/hydrated/);
const sampleItem = tree.locator('ix-tree-item').first();
await sampleItem.locator('ix-icon').click();
const disabledItem = tree.locator('ix-tree-item', {
hasText: 'Sample Child 1',
});
await expect(disabledItem).toBeVisible();
return { tree, disabledItem };
};
regressionTest(
'disabled item should have disabled class',
async ({ mount, page }) => {
const { disabledItem } = await setupTreeWithDisabledItem(mount, page);
await expect(disabledItem).toHaveClass(/disabled/);
}
);This would make the tests cleaner and easier to maintain.
Add property **disabled** to **ix-tree-item**.
nuke-ellington
requested changes
Jan 26, 2026
|
|
||
| :host(.disabled), | ||
| :host(:disabled) { | ||
| cursor: not-allowed; |
Collaborator
There was a problem hiding this comment.
Suggested change
| cursor: not-allowed; |
| @Prop() context?: TreeItemContext; | ||
|
|
||
| /** | ||
| * Disable tree item |
| ); | ||
|
|
||
| regressionTest( | ||
| 'disabled item should have reduced opacity', |
Collaborator
There was a problem hiding this comment.
This test case should be part of visual regression
Comment on lines
+545
to
+582
| regressionTest( | ||
| 'disabled item should have not-allowed cursor', | ||
| async ({ mount, page }) => { | ||
| await mount(` | ||
| <div style="height: 20rem; width: 100%;"> | ||
| <ix-tree root="root"></ix-tree> | ||
| </div> | ||
| `); | ||
|
|
||
| const tree = page.locator('ix-tree'); | ||
|
|
||
| const modelWithDisabled = { | ||
| ...defaultModel, | ||
| 'sample-child-1': { | ||
| ...defaultModel['sample-child-1'], | ||
| disabled: true, | ||
| }, | ||
| }; | ||
|
|
||
| await tree.evaluate( | ||
| (element: HTMLIxTreeElement, [model]) => { | ||
| element.model = model; | ||
| }, | ||
| [modelWithDisabled] | ||
| ); | ||
|
|
||
| const sampleItem = tree.locator('ix-tree-item').first(); | ||
| await sampleItem.locator('ix-icon').click(); | ||
|
|
||
| const disabledItem = tree.locator('ix-tree-item', { | ||
| hasText: 'Sample Child 1', | ||
| }); | ||
|
|
||
| await expect(disabledItem).toBeVisible(); | ||
| await expect(disabledItem).toHaveCSS('cursor', 'not-allowed'); | ||
| } | ||
| ); | ||
|
|
Collaborator
There was a problem hiding this comment.
Suggested change
| regressionTest( | |
| 'disabled item should have not-allowed cursor', | |
| async ({ mount, page }) => { | |
| await mount(` | |
| <div style="height: 20rem; width: 100%;"> | |
| <ix-tree root="root"></ix-tree> | |
| </div> | |
| `); | |
| const tree = page.locator('ix-tree'); | |
| const modelWithDisabled = { | |
| ...defaultModel, | |
| 'sample-child-1': { | |
| ...defaultModel['sample-child-1'], | |
| disabled: true, | |
| }, | |
| }; | |
| await tree.evaluate( | |
| (element: HTMLIxTreeElement, [model]) => { | |
| element.model = model; | |
| }, | |
| [modelWithDisabled] | |
| ); | |
| const sampleItem = tree.locator('ix-tree-item').first(); | |
| await sampleItem.locator('ix-icon').click(); | |
| const disabledItem = tree.locator('ix-tree-item', { | |
| hasText: 'Sample Child 1', | |
| }); | |
| await expect(disabledItem).toBeVisible(); | |
| await expect(disabledItem).toHaveCSS('cursor', 'not-allowed'); | |
| } | |
| ); |
Comment on lines
+583
to
+623
| regressionTest( | ||
| 'enabled items should still be selectable when other items are disabled', | ||
| async ({ mount, page }) => { | ||
| await mount(` | ||
| <div style="height: 20rem; width: 100%;"> | ||
| <ix-tree root="root"></ix-tree> | ||
| </div> | ||
| `); | ||
|
|
||
| const tree = page.locator('ix-tree'); | ||
|
|
||
| const modelWithDisabled = { | ||
| ...defaultModel, | ||
| 'sample-child-1': { | ||
| ...defaultModel['sample-child-1'], | ||
| disabled: true, | ||
| }, | ||
| }; | ||
|
|
||
| await tree.evaluate( | ||
| (element: HTMLIxTreeElement, [model]) => { | ||
| element.model = model; | ||
| }, | ||
| [modelWithDisabled] | ||
| ); | ||
|
|
||
| const sampleItem = tree.locator('ix-tree-item').first(); | ||
| await sampleItem.locator('ix-icon').click(); | ||
|
|
||
| const enabledItem = tree.locator('ix-tree-item', { | ||
| hasText: 'Sample Child 2', | ||
| }); | ||
|
|
||
| await expect(enabledItem).toBeVisible(); | ||
| await expect(enabledItem).not.toHaveClass(/disabled/); | ||
|
|
||
| await enabledItem.click(); | ||
|
|
||
| await expect(enabledItem).toHaveClass(/selected/); | ||
| } | ||
| ); |
Collaborator
There was a problem hiding this comment.
Suggested change
| regressionTest( | |
| 'enabled items should still be selectable when other items are disabled', | |
| async ({ mount, page }) => { | |
| await mount(` | |
| <div style="height: 20rem; width: 100%;"> | |
| <ix-tree root="root"></ix-tree> | |
| </div> | |
| `); | |
| const tree = page.locator('ix-tree'); | |
| const modelWithDisabled = { | |
| ...defaultModel, | |
| 'sample-child-1': { | |
| ...defaultModel['sample-child-1'], | |
| disabled: true, | |
| }, | |
| }; | |
| await tree.evaluate( | |
| (element: HTMLIxTreeElement, [model]) => { | |
| element.model = model; | |
| }, | |
| [modelWithDisabled] | |
| ); | |
| const sampleItem = tree.locator('ix-tree-item').first(); | |
| await sampleItem.locator('ix-icon').click(); | |
| const enabledItem = tree.locator('ix-tree-item', { | |
| hasText: 'Sample Child 2', | |
| }); | |
| await expect(enabledItem).toBeVisible(); | |
| await expect(enabledItem).not.toHaveClass(/disabled/); | |
| await enabledItem.click(); | |
| await expect(enabledItem).toHaveClass(/selected/); | |
| } | |
| ); |
|
Collaborator
There was a problem hiding this comment.
A11y support is missing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
💡 What is the current behavior?
GitHub Issue Number: #2091
🆕 What is the new behavior?
🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support