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.

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

Table component: adding click event on expandable table rows #2127

Merged
merged 13 commits into from
Aug 7, 2023
Merged
6 changes: 6 additions & 0 deletions .changeset/heavy-pandas-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@navikt/ds-css": minor
"@navikt/ds-react": minor
---

Adding click event on table row
KenAJoh marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 5 additions & 1 deletion @navikt/core/css/table.css
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@
display: table-row;
}

.navds-table__body .navds-table__row--shade-on-hover:hover {
.navds-table__body .navds-table__row--shade-on-hover:not(.navds-table__expandable-row--expansion-disabled):hover {
background-color: var(--ac-table-row-hover, var(--a-bg-subtle));
}

@@ -154,6 +154,10 @@
transition: border-bottom-color 190ms cubic-bezier(0.6, 0.04, 0.98, 0.335);
}

.navds-table__expandable-row:not(.navds-table__expandable-row--expansion-disabled):hover {
cursor: pointer;
}

.navds-table__expandable-row--open .navds-table__data-cell {
border-bottom-color: transparent;
}
20 changes: 13 additions & 7 deletions @navikt/core/react/src/table/ExpandableRow.tsx
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ export interface ExpandableRowProps extends Omit<RowProps, "content"> {
*/
onOpenChange?: (open: boolean) => void;
/**
* Disable expansio
* Disable expansion. shadeOnHover will not be visible.
* @default false
*/
expansionDisabled?: boolean;
@@ -68,14 +68,25 @@ export const ExpandableRow: ExpandableRowType = forwardRef(

const isOpen = open ?? internalOpen;

const clickHandler = (e) => {
onOpenChange?.(!isOpen);
if (open === undefined) {
setInternalOpen((open) => !open);
}
e.stopPropagation();
};

return (
<>
<Row
{...rest}
ref={ref}
className={cl("navds-table__expandable-row", className, {
"navds-table__expandable-row--open": isOpen,
"navds-table__expandable-row--expansion-disabled":
expansionDisabled,
})}
onClick={!expansionDisabled ? clickHandler : undefined}
>
{togglePlacement === "right" && children}
<DataCell
@@ -89,12 +100,7 @@ export const ExpandableRow: ExpandableRowType = forwardRef(
className="navds-table__toggle-expand-button"
aria-controls={id}
aria-expanded={isOpen}
onClick={() => {
onOpenChange?.(!isOpen);
if (open === undefined) {
setInternalOpen((open) => !open);
}
}}
onClick={clickHandler}
>
<ChevronDownIcon
className="navds-table__expandable-icon"
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { useState } from "react";
import { Table } from "..";
import { Link } from "../..";
import { Button, Checkbox, Link } from "../..";
import { within, userEvent } from "@storybook/testing-library";
import { expect } from "@storybook/jest";

export default {
title: "ds-react/Table",
@@ -37,6 +39,7 @@ export const Expandable = () => {
};

export const ExpandableSmall = () => {
// eslint-disable-next-line react-hooks/rules-of-hooks
const [open, setOpen] = useState(false);
return (
<Table size="small">
@@ -227,3 +230,72 @@ export const ExpandableOpen = () => {
</Table>
);
};

export const ClickableRow = {
render: () => {
// eslint-disable-next-line react-hooks/rules-of-hooks
const [row1, setOpen1] = useState(false);
// eslint-disable-next-line react-hooks/rules-of-hooks
const [row2, setOpen2] = useState(false);

return (
<Table zebraStripes>
<Table.Header>
<Table.Row>
<Table.HeaderCell>Navn</Table.HeaderCell>
<Table.HeaderCell>Info</Table.HeaderCell>
<Table.HeaderCell aria-hidden />
</Table.Row>
</Table.Header>
<Table.Body>
<Table.ExpandableRow
content={<div>placeholder</div>}
togglePlacement="right"
onOpenChange={setOpen1}
data-testid="row1"
open={row1}
>
<Table.DataCell>Ola</Table.DataCell>
<Table.DataCell>
<Button size="xsmall">Mer info</Button>
</Table.DataCell>
</Table.ExpandableRow>
<Table.ExpandableRow
content={<div>placeholder</div>}
togglePlacement="right"
onOpenChange={setOpen2}
data-testid="row2"
open={row2}
>
<Table.DataCell>Hans</Table.DataCell>
<Table.DataCell>
<Checkbox hideLabel size="small">
Sett
</Checkbox>
</Table.DataCell>
</Table.ExpandableRow>
</Table.Body>
</Table>
);
},
play: async ({ canvasElement, step }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Laget en interaktiv demo (kan ses i storybook) for et av problemene som har stoppet oss fra denne oppdateringen tidligere:

I tilfeller der man har andre interaktive elementer som eks lenker, checkboxer eller knapper ønsker vi at man ikke skal åpne raden ved klikk på dem. Dette kan kanskje også videreføres til at raden ikke skal åpnes hvis man ønsker å eks kopiere noe tekst, men ikke sikker på den.

Arbeidet stoppet litt opp med å finne en god løsning på dette, men kan hende alt man trenger er å sjekke document.activeElement 🤞 Vært helt mega å finne en god løsning på dette da jeg vet mange ønsker seg denne featuren 🙌

Copy link
Author

Choose a reason for hiding this comment

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

Takk for eksemplet.
Eventet vil per default spre seg videre ja. Dersom de interaktive elementene skal hindre eventet til å gå videre til raden må det ansvaret være på dem. Jeg la til en event.stopPropagation i click-handlere på både Button og Checkbox i eksempelet.
Jeg kommenterte også ut "play" bitten slik at alerts ikke kjører med en gang.

Copy link
Author

@ghost ghost Aug 3, 2023

Choose a reason for hiding this comment

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

🐡 Update: Har lagt til en ny commit igjen nå, hvor jeg fjerner stopPropagation fra elementene i storyen, og istedet legger til logikk på selve ExpandableRow for å kun ekspande ved behov (Dvs når den registrerer klikk på selve raden, eller ekspandable-knappen, men ikke ellers). Se linje 73-89.

🍄 Det kan være en mer robust løsning som gjør at konsumentene ikke behøver å forholde seg til propagation i det hele tatt.

const canvas = within(canvasElement);
const button = canvas.getByText("Mer info");
const checkbox = canvas.getByText("Sett");

await step("click elements", async () => {
userEvent.click(button);
userEvent.click(checkbox);
});

const row1 = canvas.getByTestId("row1");
const row2 = canvas.getByTestId("row2");

expect(
row1.className.includes("navds-table__expandable-row--open")
).toBeFalsy();
expect(
row2.className.includes("navds-table__expandable-row--open")
).toBeFalsy();
},
};