Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tree): allow selection of parent category w/out selecting children #6926

Merged
merged 6 commits into from
May 6, 2023
Merged

fix(tree): allow selection of parent category w/out selecting children #6926

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
db0c849
fix(tree): allow selection of parent category w/out selecting children
paulcpederson May 5, 2023
9585e9e
Merge branch 'master' of github.com:Esri/calcite-components into paul…
paulcpederson May 5, 2023
b448b40
chore: use composed path to check for action click
paulcpederson May 5, 2023
15bc891
handle action keyboard events
paulcpederson May 5, 2023
ab2af5b
test: add test for keyboard activation of tree action
paulcpederson May 5, 2023
58c14f4
chore: skip failing tests, remove comment
paulcpederson May 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/components/tree-item/tree-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,11 @@ export class TreeItem
{this.iconStart ? iconStartEl : null}
{checkbox ? checkbox : defaultSlotNode}
</div>
<div class={CSS.actionsEnd} hidden={!hasEndActions}>
<div
class={CSS.actionsEnd}
hidden={!hasEndActions}
ref={(el) => (this.actionSlotWrapper = el as HTMLElement)}
>
{slotNode}
</div>
</div>
Expand Down Expand Up @@ -361,7 +365,9 @@ export class TreeItem

@Listen("click")
onClick(event: Event): void {
if (this.disabled) {
const composedPath = event.composedPath();
const isActionClick = composedPath.includes(this.actionSlotWrapper);
if (this.disabled || isActionClick) {
paulcpederson marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand Down Expand Up @@ -482,6 +488,8 @@ export class TreeItem

defaultSlotWrapper!: HTMLElement;

actionSlotWrapper!: HTMLElement;

private parentTreeItem?: HTMLCalciteTreeItemElement;

@State() hasEndActions = false;
Expand Down
25 changes: 22 additions & 3 deletions src/components/tree/tree.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,25 @@ describe("calcite-tree", () => {
expect(changeSpy).toHaveReceivedEventTimes(0);
});

it("does not emit calciteTreeSelect on click of slotted action", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-tree selection-mode="multichildren">
<calcite-tree-item>
Cables
<calcite-action slot="actions-end" text="Save" icon="360-view" scale="s"></calcite-action>
</calcite-tree-item>
</calcite-tree>
`);
const action = await page.find("calcite-action");
await action.click();

const changeSpy = await action.spyOnEvent("calciteTreeSelect");
await page.waitForChanges();

expect(changeSpy).toHaveReceivedEventTimes(0);
});

describe("has selected items in the selection event payload", () => {
it("contains current selection when selection=multiple", async () => {
const page = await newE2EPage({
Expand Down Expand Up @@ -348,15 +367,15 @@ describe("calcite-tree", () => {

await item2.click();

expect(await tree.getProperty("selectedItems")).toHaveLength(3);
expect(await tree.getProperty("selectedItems")).toHaveLength(2);

await item3.click();

expect(await tree.getProperty("selectedItems")).toHaveLength(3);
expect(await tree.getProperty("selectedItems")).toHaveLength(2);

await item4.click();

expect(await tree.getProperty("selectedItems")).toHaveLength(2);
expect(await tree.getProperty("selectedItems")).toHaveLength(3);
});
});

Expand Down
44 changes: 26 additions & 18 deletions src/components/tree/tree.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,23 +165,26 @@ export class Tree {
(target.hasChildren &&
(this.selectionMode === "children" || this.selectionMode === "multichildren")));

// deselecting a parent in multichildren should deselect all the children
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment can be dropped as the line below is expressive enough.

const shouldDeselectAllChildren = this.selectionMode === "multichildren" && target.hasChildren;

const shouldModifyToCurrentSelection =
!isNoneSelectionMode &&
event.detail.modifyCurrentSelection &&
(this.selectionMode === "multiple" || this.selectionMode === "multichildren");

const shouldSelectChildren =
this.selectionMode === "multichildren" || this.selectionMode === "children";

const shouldClearCurrentSelection =
!shouldModifyToCurrentSelection &&
(((this.selectionMode === "single" || this.selectionMode === "multiple") &&
childItems.length <= 0) ||
this.selectionMode === "children" ||
this.selectionMode === "multichildren");

const shouldExpandTarget =
this.selectionMode === "children" || this.selectionMode === "multichildren";
const shouldUpdateExpand =
["children", "multichildren"].includes(this.selectionMode) ||
(["single", "multiple"].includes(this.selectionMode) &&
target.hasChildren &&
!event.detail.forceToggle);

if (!this.child) {
const targetItems: HTMLCalciteTreeItemElement[] = [];
Expand All @@ -190,12 +193,6 @@ export class Tree {
targetItems.push(target);
}

if (shouldSelectChildren) {
childItems.forEach((treeItem) => {
targetItems.push(treeItem);
});
}

if (shouldClearCurrentSelection) {
const selectedItems = nodeListToArray(
this.el.querySelectorAll("calcite-tree-item[selected]")
Expand All @@ -208,18 +205,29 @@ export class Tree {
});
}

if (shouldExpandTarget && !event.detail.forceToggle) {
target.expanded = true;
if (shouldUpdateExpand) {
if (["single", "multiple"].includes(this.selectionMode)) {
target.expanded = !target.expanded;
} else if (this.selectionMode === "multichildren") {
target.expanded = !target.selected;
} else if (this.selectionMode === "children") {
target.expanded = target.selected ? !target.expanded : true;
}
}

if (shouldDeselectAllChildren) {
childItems.forEach((item) => {
item.selected = false;
if (item.hasChildren) {
item.expanded = false;
}
});
}

if (shouldModifyToCurrentSelection) {
window.getSelection().removeAllRanges();
}

if (
(shouldModifyToCurrentSelection && target.selected) ||
(shouldSelectChildren && event.detail.forceToggle)
) {
if ((shouldModifyToCurrentSelection && target.selected) || event.detail.forceToggle) {
targetItems.forEach((treeItem) => {
if (!treeItem.disabled) {
treeItem.selected = false;
Expand Down
71 changes: 55 additions & 16 deletions src/demos/tree.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,57 @@ <h1 style="margin: 0 auto; text-align: center">Tree</h1>
</div>
</div>

<!-- children select -->
<div class="parent">
<div class="child right-aligned-text">children select</div>

<div class="child">
<calcite-tree lines selection-mode="children" scale="s">
<calcite-tree-item> Child 1 </calcite-tree-item>

<calcite-tree-item>
Child 2

<calcite-tree slot="children">
<calcite-tree-item> Grandchild 1 </calcite-tree-item>

<calcite-tree-item>
Grandchild 2
<calcite-tree slot="children">
<calcite-tree-item> Great-Grandchild 1 </calcite-tree-item>
<calcite-tree-item> Great-Grandchild 2 </calcite-tree-item>
</calcite-tree>
</calcite-tree-item>
</calcite-tree>
</calcite-tree-item>
</calcite-tree>
</div>

<div class="child">
<calcite-tree lines selection-mode="children" scale="m">
<calcite-tree-item>
Child 1 <calcite-action slot="actions-end" text="Save" icon="360-view" scale="s"></calcite-action
></calcite-tree-item>

<calcite-tree-item>
Child 2

<calcite-tree slot="children">
<calcite-tree-item> Grandchild 1 </calcite-tree-item>

<calcite-tree-item>
Grandchild 2
<calcite-tree slot="children">
<calcite-tree-item> Great-Grandchild 1 </calcite-tree-item>
<calcite-tree-item> Great-Grandchild 2 </calcite-tree-item>
</calcite-tree>
</calcite-tree-item>
</calcite-tree>
</calcite-tree-item>
</calcite-tree>
</div>
</div>

<!-- active and expanded state -->
<div class="parent">
<div class="child right-aligned-text">active and expanded state</div>
Expand Down Expand Up @@ -475,24 +526,12 @@ <h1 style="margin: 0 auto; text-align: center">Tree</h1>
<div class="child">
<p id="tree-events-demo-result-medium">0 Selected</p>
<calcite-tree id="tree-events-demo-tree-medium" lines selection-mode="multichildren" scale="m">
<calcite-tree-item> Child 1 </calcite-tree-item>

<calcite-tree-item>
<calcite-tree-item id="1"> Child 1 </calcite-tree-item>
<calcite-tree-item id="2">
Child 2

<calcite-tree slot="children">
<calcite-tree-item> Grandchild 1 </calcite-tree-item>

<calcite-tree-item> Grandchild 2 </calcite-tree-item>

<calcite-tree-item>
Grandchild 3
<calcite-tree slot="children">
<calcite-tree-item> Great-Grandchild 1 </calcite-tree-item>
<calcite-tree-item> Great-Grandchild 2 </calcite-tree-item>
<calcite-tree-item> Great-Grandchild 3 </calcite-tree-item>
</calcite-tree>
</calcite-tree-item>
<calcite-tree-item id="3" disabled> Grandchild 1 </calcite-tree-item>
<calcite-tree-item id="4"> Grandchild 2 </calcite-tree-item>
</calcite-tree>
</calcite-tree-item>
</calcite-tree>
Expand Down