Skip to content

Commit

Permalink
fix(flat-table): ensure that pressing arrow and space keys scroll tab…
Browse files Browse the repository at this point in the history
…le when it is scrollable

Ensures that arrow and space keys scroll `FlatTable` that has overflow set. The tab stop has been
moved to container further in the HTML tree to ensure that the scroll behaviour works as expected.

fix #6425
  • Loading branch information
edleeks87 committed Jan 11, 2024
1 parent 291e175 commit b7614e3
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 42 deletions.
46 changes: 46 additions & 0 deletions cypress/components/flat-table/flat-table.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3124,4 +3124,50 @@ context("Tests for Flat Table component", () => {
flatTableCell(4).should("have.css", "border-radius", "0px");
});
});

it("it should scroll the scrollable table when down arrow key pressed and wrapper is focused", () => {
CypressMountWithProviders(
<stories.FlatTableSpanComponent hasStickyHead height="200px" />
);

cy.get("body").tab();
flatTableBodyRowByPosition(5).should("not.be.visible");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowDown");
flatTableBodyRowByPosition(5).should("be.visible");
});

it("it should scroll the scrollable table when space key pressed and wrapper is focused", () => {
CypressMountWithProviders(
<stories.FlatTableSpanComponent hasStickyHead height="200px" />
);

cy.get("body").tab();
flatTableBodyRowByPosition(5).should("not.be.visible");
cy.get("body").realPress("Space");
cy.get("body").realPress("Space");
cy.get("body").realPress("Space");
cy.get("body").realPress("Space");
flatTableBodyRowByPosition(5).should("be.visible");
});

it("it should scroll the scrollable table when down arrow key pressed and wrapper is focused", () => {
CypressMountWithProviders(
<stories.FlatTableSpanComponent hasStickyHead height="200px" />
);

cy.get("body").tab();
flatTableBodyRowByPosition(5).should("not.be.visible");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowDown");
cy.get("body").realPress("ArrowUp");
cy.get("body").realPress("ArrowUp");
cy.get("body").realPress("ArrowUp");
cy.get("body").realPress("ArrowUp");
flatTableBodyRowByPosition(5).should("not.be.visible");
});
});
4 changes: 2 additions & 2 deletions src/components/flat-table/flat-table-expandable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ export const RowHeadersWithCustomPaddings = () => {
</FlatTableRow>,
];
return (
<Box width="380px" overflowX="auto">
<FlatTable>
<Box>
<FlatTable width="380px" overflowX="auto">
<FlatTableHead>
<FlatTableRow>
<FlatTableRowHeader px={8}>Name</FlatTableRowHeader>
Expand Down
25 changes: 20 additions & 5 deletions src/components/flat-table/flat-table.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,12 @@ export const FlatTable = ({
}: FlatTableProps) => {
const wrapperRef = useRef<HTMLDivElement>(null);
const tableRef = useRef<HTMLTableElement>(null);
const container = useRef<HTMLDivElement | null>(null);
const [hasVerticalScrollbar, setHasVerticalScrollbar] = useState(false);
const [hasHorizontalScrollbar, setHasHorizontalScrollbar] = useState(false);
const [firstColRowSpanIndex, setFirstColRowSpanIndex] = useState(-1);
const [lastColRowSpanIndex, setLastColRowSpanIndex] = useState(-1);
const [focused, setFocused] = useState(false);
const addDefaultHeight = !height && (hasStickyHead || hasStickyFooter);
const tableStylingProps = {
caption,
Expand Down Expand Up @@ -158,13 +160,15 @@ export const FlatTable = ({
FOCUSABLE_ROW_AND_CELL_QUERY
);

const focusableElementsArray = Array.from(
focusableElements || /* istanbul ignore next */ []
);

/* istanbul ignore if */
if (!focusableElements) {
if (!focusableElementsArray.length) {
return;
}

const focusableElementsArray = Array.from(focusableElements);

const currentFocusIndex = focusableElementsArray.findIndex(
(el) => el === document.activeElement
);
Expand Down Expand Up @@ -233,7 +237,6 @@ export const FlatTable = ({
display="flex"
flexDirection="column"
justifyContent={hasStickyFooter || height ? "space-between" : undefined}
tabIndex={0}
role="region"
overflowX={width ? "hidden" : undefined}
width={width}
Expand All @@ -244,9 +247,21 @@ export const FlatTable = ({
firstColRowSpanIndex={firstColRowSpanIndex}
lastColRowSpanIndex={lastColRowSpanIndex}
onKeyDown={handleKeyDown}
isFocused={focused}
{...rest}
>
<StyledTableContainer overflowX={overflowX} width={width}>
<StyledTableContainer
ref={container}
onFocus={() => {
if (container.current === document.activeElement) {
setFocused(true);
}
}}
onBlur={() => setFocused(false)}
tabIndex={0}
overflowX={overflowX}
width={width}
>
<StyledFlatTable
ref={tableRef}
data-component="flat-table"
Expand Down
48 changes: 43 additions & 5 deletions src/components/flat-table/flat-table.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import cellSizes from "./cell-sizes.style";
import { FLAT_TABLE_SIZES } from "./flat-table.config";
import Pager from "../pager/pager.component";
import Logger from "../../__internal__/utils/logger";
import CarbonProvider from "../carbon-provider";

// mock Logger.deprecate so that no console warnings occur while running the tests
const loggerSpy = jest.spyOn(Logger, "deprecate");
Expand Down Expand Up @@ -334,7 +335,6 @@ describe("FlatTable", () => {
wrapper = renderFlatTableWithDiv({ footer: <Footer /> });
assertStyleMatch(
{
boxShadow: "inset 0px 0px 0px 1px var(--colorsUtilityMajor100)",
boxSizing: "border-box",
},
wrapper.find(StyledFlatTableWrapper)
Expand Down Expand Up @@ -589,7 +589,9 @@ describe("FlatTable", () => {
footer={<Pager data-testid="pager" onPagination={() => {}} />}
>
<FlatTableHead>
<FlatTableRow>heading one</FlatTableRow>
<FlatTableRow>
<td>heading one</td>
</FlatTableRow>
</FlatTableHead>
<FlatTableBody>
<FlatTableRow>
Expand Down Expand Up @@ -709,8 +711,8 @@ describe("FlatTable", () => {
const arrowLeft = { key: "ArrowLeft" };

describe("when rows are clickable", () => {
it("should not move focus to first row when down arrow pressed and table wrapper focused", async () => {
rtlRender(
it("should not move focus to first row when down arrow pressed and table wrapper focused when focusRedesignOptOut is not set", async () => {
const { container } = rtlRender(
<FlatTable>
<FlatTableBody>
<FlatTableRow onClick={() => {}}>
Expand All @@ -724,9 +726,45 @@ describe("FlatTable", () => {
</FlatTableBody>
</FlatTable>
);
const tableWrapper = await screen.findByRole("region");
const tableWrapper = container.querySelectorAll(
"div"
)[1] as HTMLElement;
tableWrapper?.focus();
expect(tableWrapper).toHaveFocus();
expect(await screen.findByRole("region")).toHaveStyle({
outline: "transparent 3px solid",
"box-shadow":
"0px 0px 0px var(--borderWidth300) var(--colorsSemanticFocus500),0px 0px 0px var(--borderWidth600) var(--colorsUtilityYin090)",
});
fireEvent.keyDown(tableWrapper, arrowDown);
expect(tableWrapper).toHaveFocus();
});

it("should not move focus to first row when down arrow pressed and table wrapper focused when focusRedesignOptOut is set", async () => {
const { container } = rtlRender(
<CarbonProvider focusRedesignOptOut>
<FlatTable>
<FlatTableBody>
<FlatTableRow onClick={() => {}}>
<FlatTableCell>one</FlatTableCell>
<FlatTableCell>two</FlatTableCell>
</FlatTableRow>
<FlatTableRow onClick={() => {}}>
<FlatTableCell>three</FlatTableCell>
<FlatTableCell>four</FlatTableCell>
</FlatTableRow>
</FlatTableBody>
</FlatTable>
</CarbonProvider>
);
const tableWrapper = container.querySelectorAll(
"div"
)[2] as HTMLElement;
tableWrapper?.focus();
expect(tableWrapper).toHaveFocus();
expect(await screen.findByRole("region")).toHaveStyle({
outline: "2px solid var(--colorsSemanticFocus500)",
});
fireEvent.keyDown(tableWrapper, arrowDown);
expect(tableWrapper).toHaveFocus();
});
Expand Down
51 changes: 21 additions & 30 deletions src/components/flat-table/flat-table.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ const StyledTableContainer = styled.div<
${overflowX && `overflow-x: ${overflowX};`}
`}
:focus {
outline: none;
}
`;

const StyledFlatTable = styled.table<
Expand Down Expand Up @@ -110,6 +114,7 @@ interface StyledFlatTableWrapperProps
hasVerticalScrollbar: boolean;
lastColRowSpanIndex: number;
firstColRowSpanIndex: number;
isFocused: boolean;
}

const StyledFlatTableWrapper = styled(StyledBox)<StyledFlatTableWrapperProps>`
Expand All @@ -123,39 +128,25 @@ const StyledFlatTableWrapper = styled(StyledBox)<StyledFlatTableWrapperProps>`
border-bottom-right-radius: var(--borderRadius100);
`}
${({ isInSidebar, theme }) =>
${({ isInSidebar, theme, isFocused }) =>
css`
box-sizing: border-box;
:focus {
/* istanbul ignore next */
${theme.focusRedesignOptOut &&
/* istanbul ignore next */
css`
${oldFocusStyling}
:not(:focus-visible) {
outline: none;
}
:focus-visible {
${oldFocusStyling}
}
`}
${!theme.focusRedesignOptOut &&
css`
${addFocusStyling()}
:not(:focus-visible) {
outline: none;
box-shadow: none;
}
`}
}
${isInSidebar
? "min-width: fit-content"
: `box-shadow: inset 0px 0px 0px 1px var(--colorsUtilityMajor100)`};
/* istanbul ignore next */
${theme.focusRedesignOptOut &&
isFocused &&
/* istanbul ignore next */
css`
${oldFocusStyling}
`}
${!theme.focusRedesignOptOut &&
isFocused &&
css`
${addFocusStyling()}
`}
${isInSidebar ? "min-width: fit-content;" : ""}
`}
${({ colorTheme }) => {
Expand Down

0 comments on commit b7614e3

Please sign in to comment.