Skip to content

Commit

Permalink
fix(list): ListItem no longer focusable by default when disabled
Browse files Browse the repository at this point in the history
This better mimics the button behavior that it inherits, but can still
be set to 0 or another tabIndex if needed.

fix #997
  • Loading branch information
mlaursen committed Nov 14, 2020
1 parent 0bc495d commit 06e91ca
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
6 changes: 3 additions & 3 deletions packages/list/src/ListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,12 @@ export const ListItem = forwardRef<HTMLLIElement, ListItemProps>(
rippleClassName,
rippleContainerClassName,
role = "button",
tabIndex = 0,
disabled = false,
tabIndex = disabled ? -1 : 0,
...props
},
ref
) {
const { disabled } = props;

const { ripples, className, handlers } = useInteractionStates({
className: propClassName,
handlers: props,
Expand Down Expand Up @@ -84,6 +83,7 @@ export const ListItem = forwardRef<HTMLLIElement, ListItemProps>(
{...handlers}
ref={ref}
tabIndex={tabIndex}
disabled={disabled}
role={role}
className={className}
clickable
Expand Down
20 changes: 20 additions & 0 deletions packages/list/src/__tests__/ListItem.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable jsx-a11y/tabindex-no-positive */
import React from "react";
import { render, fireEvent } from "@testing-library/react";

Expand Down Expand Up @@ -29,4 +30,23 @@ describe("ListItem", () => {
fireEvent.click(item);
expect(onClick).not.toBeCalled();
});

it("should default the tabIndex to 0 when not disabled and to -1 when disabled", () => {
const props = { children: "Content" };
const { rerender, getByRole } = render(<ListItem {...props} />);
const item = getByRole("button");
expect(item.tabIndex).toBe(0);

rerender(<ListItem {...props} disabled />);
expect(item.tabIndex).toBe(-1);

rerender(<ListItem {...props} tabIndex={1} />);
expect(item.tabIndex).toBe(1);

rerender(<ListItem {...props} tabIndex={1} disabled />);
expect(item.tabIndex).toBe(1);

rerender(<ListItem {...props} tabIndex={0} disabled />);
expect(item.tabIndex).toBe(0);
});
});

0 comments on commit 06e91ca

Please sign in to comment.