Skip to content

Commit 089df00

Browse files
anveshmekalabenelan
authored andcommitted
fix(table-row): update selected property on calciteTabRowSelect event (#12015)
**Related Issue:** #11740 ## Summary Callback function of `calciteTabRowSelect` event reflects the accurate `selected` value of `table-row`.
1 parent 978256e commit 089df00

File tree

4 files changed

+124
-29
lines changed

4 files changed

+124
-29
lines changed

packages/calcite-components/src/components/table-row/table-row.tsx

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ export class TableRow extends LitElement implements InteractiveComponent {
4242

4343
private tableRowSlotEl = createRef<HTMLSlotElement>();
4444

45+
private userTriggered = false;
46+
47+
private _selected = false;
48+
4549
// #endregion
4650

4751
// #region Public Properties
@@ -93,7 +97,18 @@ export class TableRow extends LitElement implements InteractiveComponent {
9397
@property() scale: Scale;
9498

9599
/** When `true`, the component is selected. */
96-
@property({ reflect: true }) selected = false;
100+
@property({ reflect: true })
101+
get selected(): boolean {
102+
return this._selected;
103+
}
104+
105+
set selected(value: boolean) {
106+
const oldValue = this._selected;
107+
if (value !== oldValue) {
108+
this._selected = value;
109+
this.handleCellChanges();
110+
}
111+
}
97112

98113
/** @private */
99114
@property() selectedRowCount: number;
@@ -142,7 +157,6 @@ export class TableRow extends LitElement implements InteractiveComponent {
142157
if (
143158
changes.has("bodyRowCount") ||
144159
changes.has("scale") ||
145-
(changes.has("selected") && (this.hasUpdated || this.selected !== false)) ||
146160
changes.has("selectedRowCount") ||
147161
(changes.has("interactionMode") &&
148162
(this.hasUpdated || this.interactionMode !== "interactive"))
@@ -157,7 +171,11 @@ export class TableRow extends LitElement implements InteractiveComponent {
157171
this.handleDelayedCellChanges();
158172
}
159173

160-
if (changes.has("selected")) {
174+
if (
175+
changes.has("selected") &&
176+
(this.hasUpdated || this.selected !== false) &&
177+
!this.userTriggered
178+
) {
161179
this.calciteInternalTableRowSelect.emit();
162180
}
163181
}
@@ -327,18 +345,25 @@ export class TableRow extends LitElement implements InteractiveComponent {
327345
this.cellCount = cells?.length;
328346
}
329347

330-
private handleSelectionOfRow = (): void => {
348+
private clickHandler = (): void => {
349+
this.handleRowSelection();
350+
};
351+
352+
private async handleRowSelection(): Promise<void> {
331353
if (this.rowType === "body" || (this.rowType === "head" && this.selectionMode === "multiple")) {
354+
this.userTriggered = true;
355+
this.selected = !this.selected;
356+
await this.updateComplete;
332357
this.calciteTableRowSelect.emit();
333358
}
334-
};
359+
}
335360

336361
private handleKeyboardSelection = (event: KeyboardEvent): void => {
337362
if (isActivationKey(event.key)) {
338363
if (event.key === " ") {
339364
event.preventDefault();
340365
}
341-
this.handleSelectionOfRow();
366+
this.handleRowSelection();
342367
}
343368
};
344369

@@ -365,7 +390,7 @@ export class TableRow extends LitElement implements InteractiveComponent {
365390
alignment="center"
366391
bodyRowCount={this.bodyRowCount}
367392
key="selection-head"
368-
onClick={this.handleSelectionOfRow}
393+
onClick={this.clickHandler}
369394
onKeyDown={this.handleKeyboardSelection}
370395
parentRowAlignment={this.alignment}
371396
selectedRowCount={this.selectedRowCount}
@@ -377,7 +402,7 @@ export class TableRow extends LitElement implements InteractiveComponent {
377402
<calcite-table-cell
378403
alignment="center"
379404
key="selection-body"
380-
onClick={this.handleSelectionOfRow}
405+
onClick={this.clickHandler}
381406
onKeyDown={this.handleKeyboardSelection}
382407
parentRowAlignment={this.alignment}
383408
parentRowIsSelected={this.selected}

packages/calcite-components/src/components/table/table.e2e.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@ import { newE2EPage } from "@arcgis/lumina-compiler/puppeteerTesting";
33
import { describe, expect, it } from "vitest";
44
import { html } from "../../../support/formatting";
55
import { accessible, renders, hidden, defaults, reflects } from "../../tests/commonTests";
6-
import { createSelectedItemsAsserter, getFocusedElementProp } from "../../tests/utils/puppeteer";
6+
import {
7+
createSelectedItemsAsserter,
8+
getFocusedElementProp,
9+
createEventTimePropValuesAsserter,
10+
} from "../../tests/utils/puppeteer";
711
import { CSS } from "../table-header/resources";
812
import { CSS as PAGINATION_CSS } from "../pagination/resources";
913
import { CSS as CELL_CSS } from "../table-cell/resources";
1014
import type { TableHeader } from "../table-header/table-header";
1115
import type { TableCell } from "../table-cell/table-cell";
16+
import { TableRow } from "../table-row/table-row";
1217
import { SLOTS } from "./resources";
1318

1419
describe("calcite-table", () => {
@@ -2739,4 +2744,58 @@ describe("keyboard navigation", () => {
27392744
await page.$eval(`#${row3.id}`, (el) => el.shadowRoot?.activeElement.shadowRoot?.querySelector("td").classList),
27402745
).toEqual({ "0": CSS.selectionCell });
27412746
});
2747+
2748+
it("updates table-row's selected property correctly when calciteTabRowSelect event is emitted", async () => {
2749+
const page = await newE2EPage();
2750+
await page.setContent(
2751+
html`<calcite-table caption="Simple table" selection-mode="multiple">
2752+
<calcite-table-row slot="table-header">
2753+
<calcite-table-header heading="Heading" description="Description"></calcite-table-header>
2754+
<calcite-table-header heading="Heading" description="Description"></calcite-table-header>
2755+
</calcite-table-row>
2756+
<calcite-table-row id="row-1">
2757+
<calcite-table-cell>row1</calcite-table-cell>
2758+
<calcite-table-cell>row1</calcite-table-cell>
2759+
</calcite-table-row>
2760+
<calcite-table-row id="row-2">
2761+
<calcite-table-cell>row2</calcite-table-cell>
2762+
<calcite-table-cell>row2</calcite-table-cell>
2763+
</calcite-table-row>
2764+
</calcite-table>`,
2765+
);
2766+
2767+
async function selectRow(rowSelector: string): Promise<void> {
2768+
await page.$eval(rowSelector + " >>> calcite-table-cell:first-child", (el: TableCell["el"]) => {
2769+
el.click();
2770+
});
2771+
await page.waitForChanges();
2772+
}
2773+
2774+
const rowSelector = "calcite-table-row[id='row-1']";
2775+
const rowElement = await page.find(rowSelector);
2776+
expect(await rowElement.getProperty("selected")).toBe(false);
2777+
2778+
async function propValueAsserter(expectedPropValue: boolean): Promise<() => Promise<void>> {
2779+
return await createEventTimePropValuesAsserter<TableRow>(
2780+
page,
2781+
{
2782+
eventListenerSelector: "calcite-table",
2783+
selector: rowSelector,
2784+
eventName: "calciteTableRowSelect",
2785+
props: ["selected"],
2786+
},
2787+
async (propValues) => {
2788+
expect(propValues["selected"]).toBe(expectedPropValue);
2789+
},
2790+
);
2791+
}
2792+
2793+
const rowSelected = await propValueAsserter(true);
2794+
await selectRow(rowSelector);
2795+
await expect(rowSelected()).resolves.toBe(undefined);
2796+
2797+
const rowDeselected = await propValueAsserter(false);
2798+
await selectRow(rowSelector);
2799+
await expect(rowDeselected()).resolves.toBe(undefined);
2800+
});
27422801
});

packages/calcite-components/src/components/table/table.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,12 @@ export class Table extends LitElement {
211211
}
212212

213213
private calciteInternalTableRowSelectListener(event: CustomEvent): void {
214-
if (event.composedPath().includes(this.el)) {
215-
this.updateSelectedItems(false);
214+
if (!event.composedPath().includes(this.el)) {
215+
return;
216216
}
217+
218+
this.updateSelectedItems(false);
219+
event.stopPropagation();
217220
}
218221

219222
private calciteInternalTableRowFocusEvent(event: CustomEvent<TableRowFocusEvent>): void {
@@ -343,7 +346,7 @@ export class Table extends LitElement {
343346
});
344347
}
345348

346-
private updateSelectedItems(emit?: boolean): void {
349+
private async updateSelectedItems(emit?: boolean): Promise<void> {
347350
const selectedItems = this.bodyRows?.filter((el) => el.selected);
348351
this._selectedItems = selectedItems;
349352
this.selectedCount = selectedItems?.length;
@@ -368,8 +371,7 @@ export class Table extends LitElement {
368371
if (elToMatch?.rowType === "head") {
369372
el.selected = this.selectedCount !== this.bodyRows?.length;
370373
} else {
371-
el.selected =
372-
elToMatch === el ? !el.selected : this.selectionMode === "multiple" ? el.selected : false;
374+
el.selected = this.selectionMode === "multiple" || elToMatch === el ? el.selected : false;
373375
}
374376
});
375377
this.updateSelectedItems(true);

packages/calcite-components/src/tests/utils/puppeteer.ts

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ export async function findAll(
550550
* This util helps assert on a component's state at the time of an event firing.
551551
*
552552
* This is needed due to how Puppeteer works asynchronously, so the state at event emit-time might not be the same as the state at the time of the assertion.
553+
* When specified, the `eventListenerTarget` will be used to listen for the event instead of the component itself.
553554
*
554555
* Note: values returned can only be serializable values.
555556
*
@@ -559,25 +560,28 @@ export async function findAll(
559560
* const page = await newE2EPage();
560561
* await page.setContent(html`
561562
* <calcite-combobox>
562-
* <!-- ... -->
563+
* <calcite-combobox-item value="A">A</calcite-combobox-item>
564+
* <!-- ... -->
563565
* </calcite-combobox>
564566
* `);
565567
* const propValueAsserter = await createEventTimePropValuesAsserter<Combobox>(
566568
* page,
567569
* {
568-
* selector: "calcite-combobox",
569-
* eventName: "calciteComboboxChange",
570-
* props: ["value", "selectedItems"],
570+
* selector: "calcite-combobox-item",
571+
* eventName: "calciteComboboxItemChange",
572+
* props: ["selected"],
573+
* eventListenerSelector: "calcite-combobox",
571574
* },
572575
* async (propValues) => {
573-
* expect(propValues.value).toBe("K");
574-
* expect(propValues.selectedItems).toHaveLength(1);
576+
* expect(propValues.selected).toBe(true)
575577
* },
576578
* );
577579
* const combobox = await page.find("calcite-combobox");
578-
* await combobox.callMethod("setFocus");
579-
* await combobox.press("K");
580-
* await combobox.press("Enter");
580+
* await element.click();
581+
* await page.waitForChanges();
582+
* const comboboxItem = await page.find("calcite-combobox-item#A");
583+
* await comboboxItem.click();
584+
* await page.waitForChanges();
581585
*
582586
* await expect(propValueAsserter()).resolves.toBe(undefined);
583587
* });
@@ -587,6 +591,7 @@ export async function findAll(
587591
* @param propValuesTarget.selector
588592
* @param propValuesTarget.eventName
589593
* @param propValuesTarget.props
594+
* @param propValuesTarget.eventListenerSelector
590595
* @param onEvent
591596
*/
592597
export async function createEventTimePropValuesAsserter<
@@ -598,18 +603,21 @@ export async function createEventTimePropValuesAsserter<
598603
>(
599604
page: E2EPage,
600605
propValuesTarget: {
601-
selector: ComponentTag;
606+
selector: ComponentTag | string;
602607
eventName: Events;
603608
props: Keys[];
609+
eventListenerSelector?: string;
604610
},
605611
onEvent: (propValues: PropValues) => Promise<void>,
606612
): Promise<() => Promise<void>> {
607613
// we set this up early to we capture state as soon as the event fires
614+
const { selector, eventName, props, eventListenerSelector } = propValuesTarget;
608615
const callbackAfterEvent = page.$eval(
609-
propValuesTarget.selector,
610-
(element: El, eventName, props) => {
616+
selector,
617+
(element: El, eventName, props, eventListenerSelector) => {
618+
const targetEl = eventListenerSelector ? document.querySelector(eventListenerSelector) : element;
611619
return new Promise<PropValues>((resolve) => {
612-
element.addEventListener(
620+
targetEl.addEventListener(
613621
eventName,
614622
() => {
615623
const propValues = Object.fromEntries(props.map((prop) => [prop, element[prop]]));
@@ -619,8 +627,9 @@ export async function createEventTimePropValuesAsserter<
619627
);
620628
});
621629
},
622-
propValuesTarget.eventName,
623-
propValuesTarget.props,
630+
eventName,
631+
props,
632+
eventListenerSelector,
624633
);
625634

626635
return () => callbackAfterEvent.then(onEvent);

0 commit comments

Comments
 (0)