diff --git a/packages/core/src/components/Button/Button.tsx b/packages/core/src/components/Button/Button.tsx index c911f73b96..7752d4e848 100644 --- a/packages/core/src/components/Button/Button.tsx +++ b/packages/core/src/components/Button/Button.tsx @@ -71,6 +71,10 @@ export interface ButtonProps extends VibeComponentProps { /** aria controls - receives id for the controlled region */ ariaControls?: string; "aria-describedby"?: AriaAttributes["aria-describedby"]; + /** + * aria to be used for screen reader to know if the button is hidden + */ + "aria-hidden"?: AriaAttributes["aria-hidden"]; /** On Button Focus callback */ onFocus?: (event: React.FocusEvent) => void; /** On Button Blur callback */ @@ -136,6 +140,7 @@ const Button: VibeComponent & { ariaExpanded, ariaControls, "aria-describedby": ariaDescribedBy, + "aria-hidden": ariaHidden, blurOnMouseUp, dataTestId: backwardCompatabilityDataTestId, "data-testid": dataTestId, @@ -250,7 +255,7 @@ const Button: VibeComponent & { id, onFocus, onBlur, - tabIndex: disabled ? -1 : tabIndex, + tabIndex: disabled || ariaHidden ? -1 : tabIndex, "data-testid": overrideDataTestId || getTestId(ComponentDefaultTestId.BUTTON, id), onMouseDown: onMouseDownClicked, "aria-disabled": disabled, @@ -260,7 +265,8 @@ const Button: VibeComponent & { "aria-haspopup": ariaHasPopup, "aria-expanded": ariaExpanded, "aria-controls": ariaControls, - "aria-describedby": ariaDescribedBy + "aria-describedby": ariaDescribedBy, + "aria-hidden": ariaHidden }; return props; }, [ @@ -284,7 +290,8 @@ const Button: VibeComponent & { ariaHasPopup, ariaExpanded, ariaControls, - ariaDescribedBy + ariaDescribedBy, + ariaHidden ]); const leftIconSize = useMemo(() => { diff --git a/packages/core/src/components/Button/__tests__/button.jest.js b/packages/core/src/components/Button/__tests__/button.jest.js index b855657734..b6a70bbf03 100644 --- a/packages/core/src/components/Button/__tests__/button.jest.js +++ b/packages/core/src/components/Button/__tests__/button.jest.js @@ -1,33 +1,30 @@ import React from "react"; import { render, fireEvent, cleanup, waitFor } from "@testing-library/react"; +import "@testing-library/jest-dom"; import Button from "../Button"; const text = "Click Me!"; -const className = "test-class"; - -function renderComponent(props) { - return render(); -} - -describe("", () => { - let clickActionStub; - let onMouseDownStub; - let buttonComponent; - - beforeEach(() => { - clickActionStub = jest.fn(); - onMouseDownStub = jest.fn(); - buttonComponent = render( - - ); - }); +describe(" + ); + }); + it("should call the click callback when clicked", () => { const { getByText } = buttonComponent; fireEvent.click(getByText(text)); @@ -99,8 +96,7 @@ describe("", () => { it("should call on blur", () => { const onBlur = jest.fn(); - cleanup(); - const { getByText } = renderComponent({ onBlur }); + const { getByText } = render(); const button = getByText(text); fireEvent.blur(button); expect(onBlur.mock.calls.length).toEqual(1); @@ -108,8 +104,11 @@ describe("", () => { it("should call do blur on mouseup", () => { const onBlur = jest.fn(); - cleanup(); - const { getByText } = renderComponent({ onBlur, blurOnMouseUp: false }); + const { getByText } = render( + + ); const button = getByText(text); fireEvent.focus(button); fireEvent.mouseUp(button); @@ -118,25 +117,24 @@ describe("", () => { it("should call on focus", () => { const onFocus = jest.fn(); - cleanup(); - const { getByText } = renderComponent({ onFocus }); + const { getByText } = render(); const button = getByText(text); fireEvent.focus(button); expect(onFocus.mock.calls.length).toEqual(1); }); describe("mouse down", () => { + const onClick = jest.fn(); it("should call the click callback when clicked", () => { - const { getByText } = buttonComponent; + const { getByText } = render(); fireEvent.mouseDown(getByText(text)); - expect(clickActionStub.mock.calls.length).toEqual(0); + expect(onClick).not.toBeCalled(); }); it("should call mouse down callback", () => { - const { getByText, rerender } = buttonComponent; const onMouseDown = jest.fn(); - rerender( - ); @@ -144,17 +142,31 @@ describe("", () => { expect(onMouseDown.mock.calls.length).toEqual(1); }); }); + describe("a11y", () => { it("Aria label should be connected to the button", () => { const ariaLabel = "Icon Name"; + const onClick = jest.fn(); const { getByLabelText } = render( - ); const buttonElement = getByLabelText(ariaLabel); fireEvent.click(buttonElement); - expect(clickActionStub.mock.calls.length).toEqual(1); + expect(onClick.mock.calls.length).toEqual(1); + }); + + it("should apply aria-hidden attribute to button", () => { + const { getByRole } = render(); + const buttonElement = getByRole("button", { hidden: true }); + expect(buttonElement).toHaveAttribute("aria-hidden", "true"); + }); + + it("should not apply aria-hidden attribute to button", () => { + const { getByRole } = render(); + const buttonElement = getByRole("button"); + expect(buttonElement).not.toHaveAttribute("aria-hidden"); }); }); }); diff --git a/packages/core/src/components/IconButton/IconButton.tsx b/packages/core/src/components/IconButton/IconButton.tsx index 0a6746909c..4d3c23f490 100644 --- a/packages/core/src/components/IconButton/IconButton.tsx +++ b/packages/core/src/components/IconButton/IconButton.tsx @@ -1,4 +1,4 @@ -import React, { forwardRef, Fragment, useMemo, useRef } from "react"; +import React, { AriaAttributes, forwardRef, Fragment, useMemo, useRef } from "react"; import cx from "classnames"; import { noop as NOOP } from "lodash-es"; import useMergeRef from "../../hooks/useMergeRef"; @@ -43,6 +43,10 @@ export interface IconButtonProps extends VibeComponentProps { * a11y property to be added, used for screen reader to know if the button is expanded */ ariaExpanded?: boolean; + /** + * a11y property to be added, used for screen reader to know if the button is hidden + */ + "aria-hidden"?: AriaAttributes["aria-hidden"]; /** * Size of the icon */ @@ -101,6 +105,7 @@ const IconButton: VibeComponent & { tooltipContent, ariaLabel, ariaExpanded, + "aria-hidden": ariaHidden, hideTooltip = false, kind = IconButton.kinds.TERTIARY, active, @@ -183,6 +188,7 @@ const IconButton: VibeComponent & { kind={kind} ariaLabel={buttonAriaLabel} ariaExpanded={ariaExpanded} + aria-hidden={ariaHidden} ref={mergedRef} id={id} data-testid={overrideDataTestId || getTestId(ComponentDefaultTestId.ICON_BUTTON, id)} diff --git a/packages/core/src/components/IconButton/__tests__/iconButton-tests.jest.tsx b/packages/core/src/components/IconButton/__tests__/iconButton-tests.jest.tsx index 04a6d88ea6..1ecbea9dc0 100644 --- a/packages/core/src/components/IconButton/__tests__/iconButton-tests.jest.tsx +++ b/packages/core/src/components/IconButton/__tests__/iconButton-tests.jest.tsx @@ -1,12 +1,15 @@ import React from "react"; import { fireEvent, render, cleanup, screen } from "@testing-library/react"; +import "@testing-library/jest-dom"; import { act } from "@testing-library/react-hooks"; import IconButton, { IconButtonProps } from "../IconButton"; jest.useFakeTimers(); -const renderComponent = (props: IconButtonProps) => { - return render(); +const ariaLabel = "Button Icon"; + +const renderComponent = (props: IconButtonProps = {}) => { + return render(); }; describe("IconButton tests", () => { @@ -17,8 +20,7 @@ describe("IconButton tests", () => { describe("click", () => { it("should call the callback function when clicked ", () => { const onClick = jest.fn(); - const ariaLabel = "Button Icon"; - renderComponent({ onClick, ariaLabel }); + renderComponent({ onClick }); const component = screen.getByLabelText(ariaLabel); fireEvent.click(component); expect(onClick.mock.calls.length).toBe(1); @@ -26,8 +28,7 @@ describe("IconButton tests", () => { it("should not call the callback if disabled when clicked ", () => { const onClick = jest.fn(); - const ariaLabel = "Button Icon"; - renderComponent({ onClick, ariaLabel, disabled: true }); + renderComponent({ onClick, disabled: true }); const component = screen.getByLabelText(ariaLabel); fireEvent.click(component); expect(onClick.mock.calls.length).toBe(0); @@ -35,11 +36,10 @@ describe("IconButton tests", () => { }); describe("Tooltips", () => { - it("should display the tooltip content", () => { + it("should display the tooltip content", async () => { const tooltipContent = "My Text"; - const ariaLabel = "Button Icon"; - renderComponent({ tooltipContent, ariaLabel }); + renderComponent({ tooltipContent }); const component = screen.getByLabelText(ariaLabel); act(() => { fireEvent.mouseEnter(component); @@ -54,9 +54,7 @@ describe("IconButton tests", () => { }); it("should display the tooltip with aria label", () => { - const ariaLabel = "Button Icon"; - - renderComponent({ ariaLabel }); + renderComponent(); const component = screen.getByLabelText(ariaLabel); act(() => { fireEvent.mouseEnter(component); @@ -71,10 +69,9 @@ describe("IconButton tests", () => { }); it("should display not disabledReason if disabled is false", () => { - const ariaLabel = "Button Icon"; const disabledReason = "I'm a disabled button"; - renderComponent({ ariaLabel, disabledReason }); + renderComponent({ disabledReason }); const component = screen.getByLabelText(ariaLabel); act(() => { fireEvent.mouseEnter(component); @@ -89,10 +86,9 @@ describe("IconButton tests", () => { }); it("should display disabledReason if disabled is true", () => { - const ariaLabel = "Button Icon"; const disabledReason = "I'm a disabled button"; - renderComponent({ ariaLabel, disabledReason, disabled: true }); + renderComponent({ disabledReason, disabled: true }); const component = screen.getByLabelText(ariaLabel); act(() => { fireEvent.mouseEnter(component); @@ -106,4 +102,20 @@ describe("IconButton tests", () => { jest.advanceTimersByTime(1000); }); }); + + describe("a11y", () => { + describe("aria-hidden", () => { + it("should pass aria-hidden attribute to Button component", () => { + const { getByRole } = renderComponent({ "aria-hidden": true }); + const buttonComponent = getByRole("button", { hidden: true }); + expect(buttonComponent).toHaveAttribute("aria-hidden", "true"); + }); + + it("should not have aria-hidden attribute on Button component if not specified", () => { + const { getByRole } = renderComponent(); + const buttonComponent = getByRole("button"); + expect(buttonComponent).not.toHaveAttribute("aria-hidden"); + }); + }); + }); }); diff --git a/packages/core/src/components/Table/Table/__tests__/table.jest.tsx b/packages/core/src/components/Table/Table/__tests__/table.jest.tsx index e763990f93..f8ed809f2b 100644 --- a/packages/core/src/components/Table/Table/__tests__/table.jest.tsx +++ b/packages/core/src/components/Table/Table/__tests__/table.jest.tsx @@ -201,12 +201,20 @@ describe("Table", () => { describe.each([ ["asc", "ascending"], ["desc", "descending"], - ["none", null] + ["none", "none"] ])("Sort", (sortState: ITableHeaderCellProps["sortState"], ariaSort) => { - it(`Should apply aria-sort to header element (${sortState}, ${ariaSort})`, () => { - const { getByRole } = render(); + it(`Should apply aria-sort to header element (${sortState}, ${ariaSort}) when onSortClicked is defined`, () => { + const onSortClicked = jest.fn(); + const { getByRole } = render( + + ); expect(getByRole("columnheader").getAttribute("aria-sort")).toBe(ariaSort); }); + + it(`Should not apply aria-sort to header element (${sortState}, ${ariaSort}) when onSortClicked isn't defined`, () => { + const { getByRole } = render(); + expect(getByRole("columnheader")).not.toHaveAttribute("aria-sort"); + }); }); }); }); diff --git a/packages/core/src/components/Table/Table/tableHelpers.ts b/packages/core/src/components/Table/Table/tableHelpers.ts index 5c5cd91338..28be34722d 100644 --- a/packages/core/src/components/Table/Table/tableHelpers.ts +++ b/packages/core/src/components/Table/Table/tableHelpers.ts @@ -48,9 +48,11 @@ export function getNextSortState(sortState: ITableHeaderCellProps["sortState"]): export function getAriaSort(sortState: ITableHeaderCellProps["sortState"]): AriaAttributes["aria-sort"] { if (sortState === "asc") { return "ascending"; - } else if (sortState === "desc") { + } + if (sortState === "desc") { return "descending"; } + return "none"; } export function getSkeletonType(loadingStateType: TableLoadingStateType): SkeletonType { diff --git a/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.module.scss b/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.module.scss index e0654b564c..96c2cb7ec5 100644 --- a/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.module.scss +++ b/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.module.scss @@ -1,3 +1,5 @@ +@import "~monday-ui-style/dist/mixins"; + .tableHeaderCell { height: var(--table-row-size); padding: var(--spacing-small); @@ -9,8 +11,10 @@ display: flex; justify-content: space-between; background-color: inherit; + @include focus-style(); - &:hover { + &:hover, + &.sortActive { background-color: var(--primary-background-hover-color); } @@ -31,11 +35,17 @@ .sort { color: var(--icon-color); + transition: opacity 0.1s; &.asc, &.desc { color: var(--primary-text-color); } + + &:not(.show) { + opacity: 0; + pointer-events: none; + } } } } diff --git a/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.tsx b/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.tsx index 17d1af925a..1a5030f218 100644 --- a/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.tsx +++ b/packages/core/src/components/Table/TableHeaderCell/TableHeaderCell.tsx @@ -1,4 +1,4 @@ -import React, { forwardRef } from "react"; +import React, { forwardRef, useState } from "react"; import cx from "classnames"; import { SubIcon, VibeComponent, VibeComponentProps } from "../../../types"; import styles from "./TableHeaderCell.module.scss"; @@ -12,6 +12,7 @@ import { getAriaSort, getNextSortState, getSortIcon } from "../Table/tableHelper import Tooltip from "../../Tooltip/Tooltip"; import { getTestId } from "../../../tests/test-ids-utils"; import { ComponentDefaultTestId } from "../../../tests/constants"; +import { getStyle } from "../../../helpers/typesciptCssModulesHelper"; export interface ITableHeaderCellProps extends VibeComponentProps { title: string; @@ -37,14 +38,24 @@ const TableHeaderCell: VibeComponent = fo }, ref ) => { + const [isHovered, setIsHovered] = useState(false); + const ariaSort = getAriaSort(sortState); + const isSortActive = onSortClicked && ariaSort !== "none"; + const shouldShowSortIcon = ariaSort !== "none" || isHovered; + return (
setIsHovered(true)} + onMouseLeave={() => setIsHovered(false)} + onFocus={() => setIsHovered(true)} + onBlur={() => setIsHovered(false)} + aria-sort={onSortClicked ? ariaSort : undefined} + tabIndex={onSortClicked ? 0 : undefined} > = fo kind={ButtonType.TERTIARY} size={IconButton.sizes.XS} ariaLabel={sortButtonAriaLabel} - className={cx(styles.sort, { [styles.asc]: sortState === "asc", [styles.desc]: sortState === "desc" })} + aria-hidden={!shouldShowSortIcon} + className={cx(styles.sort, getStyle(styles, sortState), { + [styles.show]: shouldShowSortIcon + })} onClick={() => onSortClicked(getNextSortState(sortState))} /> diff --git a/packages/core/src/components/Table/TableRow/TableRow.module.scss b/packages/core/src/components/Table/TableRow/TableRow.module.scss index 1c6b5fed65..a82dfc5d9a 100644 --- a/packages/core/src/components/Table/TableRow/TableRow.module.scss +++ b/packages/core/src/components/Table/TableRow/TableRow.module.scss @@ -11,7 +11,7 @@ &:hover { > * { - background-color: var(--allgrey-background-color); + background-color: var(--primary-background-hover-color); } &[aria-selected="true"] > * {