Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions src/components/IconButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,30 @@ describe("IconButton", () => {
expect(onClick).toHaveBeenCalledTimes(1);
});

it("does not propagate clicks with onClick=lambda", async () => {
const parentOnClick = jest.fn();
const iconOnClick = jest.fn();
const r = await render(
<div onClick={parentOnClick}>
<IconButton icon="trash" onClick={iconOnClick} />
</div>,
);
click(r.trash);
expect(iconOnClick).toHaveBeenCalledTimes(1);
expect(parentOnClick).toHaveBeenCalledTimes(0);
});

it("does not propagate clicks with onClick=string", async () => {
const parentOnClick = jest.fn();
const r = await render(
<div onClick={parentOnClick}>
<IconButton icon="trash" onClick="http://foo.com" />
</div>,
);
click(r.trash);
expect(parentOnClick).toHaveBeenCalledTimes(0);
});

it("applies expected properties when rendering a link with an absolute url", async () => {
const r = await render(<IconButton icon="trash" onClick="https://www.homebound.com" />, withRouter());
expect(r.trash.tagName).toBe("A");
Expand Down
10 changes: 8 additions & 2 deletions src/components/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { Icon, IconProps, maybeTooltip, navLink, resolveTooltip } from "src/comp
import { Css, Palette } from "src/Css";
import { useGetRef } from "src/hooks/useGetRef";
import { BeamButtonProps, BeamFocusableProps } from "src/interfaces";
import { noop } from "src/utils";
import { getButtonOrLink } from "src/utils/getInteractiveElement";
import { useTestIds } from "src/utils/useTestIds";

Expand Down Expand Up @@ -54,7 +53,13 @@ export function IconButton(props: IconButtonProps) {
const { buttonProps } = useButton(
{
...ariaProps,
onPress: typeof onPress === "string" ? noop : onPress,
onPress:
typeof onPress === "string"
? (e) => {
debugger;
console.log("HERE");
}
: onPress,
elementType: typeof onPress === "string" ? "a" : "button",
},
ref,
Expand Down Expand Up @@ -94,6 +99,7 @@ export function IconButton(props: IconButtonProps) {
color={color || (isDisabled ? Palette.Gray400 : iconColor)}
bgColor={bgColor}
inc={compact ? 2 : inc}
{...testIds.svg}
/>
);

Expand Down
31 changes: 27 additions & 4 deletions src/components/Table/GridTable.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { act } from "@testing-library/react";
import { MutableRefObject, useContext, useMemo, useState } from "react";
import { IconButton } from "src/components/IconButton";
import { GridDataRow } from "src/components/Table/components/Row";
import { GridTable, OnRowSelect, setRunningInJest } from "src/components/Table/GridTable";
import { GridTableApi, GridTableApiImpl, useGridTableApi } from "src/components/Table/GridTableApi";
Expand Down Expand Up @@ -1005,11 +1006,9 @@ describe("GridTable", () => {
// Given rowStyles that specify an action for each row
const rowStyles: RowStyles<Row> = {
header: {},
data: {
rowLink: () => "https://www.homebound.com",
},
data: { rowLink: () => "https://www.homebound.com" },
};
// And a table where one columns omits wrapping the action
// And a table where one column omits wrapping the action
const r = await render(
<GridTable {...{ columns: [{ ...columns[0], wrapAction: false }, columns[1]], rows, rowStyles }} />,
withRouter(),
Expand All @@ -1019,6 +1018,30 @@ describe("GridTable", () => {
expect(cell(r, 1, 1).tagName).toBe("A");
});

it("rowLink is not triggered when IconButtons are clicked", async () => {
// Given rowStyles that specify an action for each row
const rowStyles: RowStyles<Row> = {
header: {},
data: { rowLink: () => "/bar" },
};
const columnWithIcon: GridColumn<Row> = {
id: "value",
header: () => "Value",
data: () => (
<div>
some other text <IconButton icon="add" onClick="/foo" />
</div>
),
};
// And a table where one column omits wrapping the action
const router = withRouter();
router.history.push = jest.fn();
const r = await render(<GridTable columns={[columnWithIcon]} rows={rows} rowStyles={rowStyles} />, router);
click(r.add_0);
expect(router.history.push).toHaveBeenCalledTimes(1);
// expect(router.history.location.pathname).toBe("/foo");
});

it("can handle onClick for GridCellContent", async () => {
const onClick = jest.fn();
// Given a table with an onClick specified GridCellContent
Expand Down
31 changes: 22 additions & 9 deletions src/components/Table/components/cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,21 @@ export type RenderCellFn<R extends Kinded> = (
row: R,
rowStyle: RowStyle<R> | undefined,
classNames: string | undefined,
onClick: VoidFunction | undefined,
cellOnClick: VoidFunction | undefined,
tooltip: ReactNode | undefined,
) => ReactNode;

/** Renders our default cell element, i.e. if no row links and no custom renderCell are used. */
export const defaultRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn<any> =
(as: RenderAs, colSpan) => (key, css, content, row, rowStyle, classNames: string | undefined, onClick, tooltip) => {
(as: RenderAs, colSpan) =>
(key, css, content, row, rowStyle, classNames: string | undefined, cellOnClick, tooltip) => {
const Cell = as === "table" ? "td" : "div";
return (
<Cell
key={key}
css={{ ...css, ...Css.cursor("default").$ }}
className={classNames}
onClick={onClick}
onClick={cellOnClick}
{...(as === "table" && { colSpan })}
>
{content}
Expand All @@ -70,7 +71,7 @@ export const defaultRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn<an
* Used for the Header, Totals, and Expanded Header row's cells.
* */
export const headerRenderFn: (column: GridColumnWithId<any>, as: RenderAs, colSpan: number) => RenderCellFn<any> =
(column, as, colSpan) => (key, css, content, row, rowStyle, classNames: string | undefined, onClick, tooltip) => {
(column, as, colSpan) => (key, css, content, row, rowStyle, classNames: string | undefined, cellOnClick, tooltip) => {
const Cell = as === "table" ? "th" : "div";
return (
<Cell key={key} css={{ ...css }} className={classNames} {...(as === "table" && { colSpan })}>
Expand All @@ -81,7 +82,8 @@ export const headerRenderFn: (column: GridColumnWithId<any>, as: RenderAs, colSp

/** Renders a cell element when a row link is in play. */
export const rowLinkRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn<any> =
(as: RenderAs, colSpan) => (key, css, content, row, rowStyle, classNames: string | undefined, onClick, tooltip) => {
(as: RenderAs, colSpan) =>
(key, css, content, row, rowStyle, classNames: string | undefined, cellOnClick, tooltip) => {
const to = rowStyle!.rowLink!(row);
if (as === "table") {
return (
Expand All @@ -93,7 +95,18 @@ export const rowLinkRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn<an
);
}
return (
<Link key={key} to={to} css={{ ...Css.tdn.color("unset").$, ...css }} className={`${navLink} ${classNames}`}>
<Link
key={key}
to={to}
css={{ ...Css.tdn.color("unset").$, ...css }}
onClick={(e) => {
if (e.target !== e.currentTarget) {
debugger;
}
// outer link logic
}}
className={`${navLink} ${classNames}`}
>
{content}
</Link>
);
Expand All @@ -102,16 +115,16 @@ export const rowLinkRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn<an
/** Renders a cell that will fire the RowStyle.onClick. */
export const rowClickRenderFn: (as: RenderAs, api: GridTableApi<any>, colSpan: number) => RenderCellFn<any> =
(as: RenderAs, api: GridTableApi<any>, colSpan) =>
(key, css, content, row, rowStyle, classNames: string | undefined, onClick, tooltip) => {
(key, css, content, row, rowStyle, classNames: string | undefined, cellOnClick, tooltip) => {
const Cell = as === "table" ? "td" : "div";
return (
<Cell
{...{ key }}
css={{ ...css }}
className={classNames}
onClick={(e) => {
onClick={() => {
rowStyle!.onClick!(row, api);
onClick && onClick();
cellOnClick && cellOnClick();
}}
{...(as === "table" && { colSpan })}
>
Expand Down
Loading