From 3f532ad20c6b872990c5122d93774ab6c87ee667 Mon Sep 17 00:00:00 2001 From: Masoud Amjadi Date: Wed, 16 Oct 2024 12:08:10 -0400 Subject: [PATCH] feat(ButtonToggle): implement new icon variant of toggle (#860) * feat(buttontoggle): first implementation of the ButtonToggle component * test(buttontoggle): added tests * fix(buttontoggle): fix A11y errors * refactor(buttontoggle): fix styling bugs in CellBasic --- packages/components/src/common/warnings.ts | 7 +- .../__snapshots__/index.test.tsx.snap | 8 +- .../__snapshots__/banner.test.tsx.snap | 2 +- .../Button/__storybook__/index.stories.tsx | 2 +- .../__storybook__/index.stories.tsx | 2 +- .../__snapshots__/index.test.tsx.snap | 2 +- .../components/src/core/ButtonIcon/style.ts | 12 +++ .../ButtonToggle/__storybook__/constants.tsx | 29 ++++++ .../__storybook__/index.stories.tsx | 76 ++++++++++++++++ .../__storybook__/stories/default.tsx | 8 ++ .../__storybook__/stories/test.tsx | 15 ++++ .../__tests__/ButtonToggle.namespace-test.tsx | 13 +++ .../__snapshots__/index.test.tsx.snap | 30 +++++++ .../ButtonToggle/__tests__/index.test.tsx | 90 +++++++++++++++++++ .../src/core/ButtonToggle/index.tsx | 50 +++++++++++ .../components/src/core/ButtonToggle/style.ts | 30 +++++++ .../src/core/CellBasic/__storybook__/style.ts | 14 +-- .../__snapshots__/index.test.tsx.snap | 8 +- .../__snapshots__/index.test.tsx.snap | 2 +- .../src/core/Panel/__storybook__/constants.ts | 3 + .../Panel/__storybook__/stories/default.tsx | 15 ++-- .../__snapshots__/index.test.tsx.snap | 1 - packages/components/src/core/Panel/style.ts | 2 +- .../__snapshots__/index.test.tsx.snap | 2 +- .../__snapshots__/index.test.tsx.snap | 2 +- packages/components/src/index.ts | 2 + 26 files changed, 390 insertions(+), 37 deletions(-) create mode 100644 packages/components/src/core/ButtonToggle/__storybook__/constants.tsx create mode 100644 packages/components/src/core/ButtonToggle/__storybook__/index.stories.tsx create mode 100644 packages/components/src/core/ButtonToggle/__storybook__/stories/default.tsx create mode 100644 packages/components/src/core/ButtonToggle/__storybook__/stories/test.tsx create mode 100644 packages/components/src/core/ButtonToggle/__tests__/ButtonToggle.namespace-test.tsx create mode 100644 packages/components/src/core/ButtonToggle/__tests__/__snapshots__/index.test.tsx.snap create mode 100644 packages/components/src/core/ButtonToggle/__tests__/index.test.tsx create mode 100644 packages/components/src/core/ButtonToggle/index.tsx create mode 100644 packages/components/src/core/ButtonToggle/style.ts diff --git a/packages/components/src/common/warnings.ts b/packages/components/src/common/warnings.ts index 9d18c003f..26b9957f6 100644 --- a/packages/components/src/common/warnings.ts +++ b/packages/components/src/common/warnings.ts @@ -2,6 +2,7 @@ export enum SDSWarningTypes { ButtonMinimalIsAllCaps = "buttonMinimalIsAllCaps", ButtonMissingSDSProps = "buttonMissingProps", ButtonIconMissingIconProp = "buttonIconMissingIconProp", + ButtonToggleMissingIconProp = "buttonToggleMissingIconProp", ChipDeprecated = "chipDeprecated", MenuSelectDeprecated = "menuSelectDeprecated", TooltipSubtitle = "tooltipSubtitle", @@ -9,7 +10,7 @@ export enum SDSWarningTypes { TooltipInverted = "tooltipInverted", } -const SDS_WARNINGS = { +export const SDS_WARNINGS = { [SDSWarningTypes.ButtonMinimalIsAllCaps]: { hasWarned: false, message: @@ -25,6 +26,10 @@ const SDS_WARNINGS = { message: "Warning: Buttons with an SDS type of icon require an icon prop to be provided.", }, + [SDSWarningTypes.ButtonToggleMissingIconProp]: { + hasWarned: false, + message: "Warning: Button Toggles require an icon prop to be provided.", + }, [SDSWarningTypes.ChipDeprecated]: { hasWarned: false, message: "Warning: will be deprecated and replaced with ", diff --git a/packages/components/src/core/Autocomplete/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/Autocomplete/__tests__/__snapshots__/index.test.tsx.snap index f00aeed5f..f4133fb52 100644 --- a/packages/components/src/core/Autocomplete/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/Autocomplete/__tests__/__snapshots__/index.test.tsx.snap @@ -28,7 +28,7 @@ exports[` ControlledOpen story renders snapshot 1`] = ` +`; diff --git a/packages/components/src/core/ButtonToggle/__tests__/index.test.tsx b/packages/components/src/core/ButtonToggle/__tests__/index.test.tsx new file mode 100644 index 000000000..26799bf00 --- /dev/null +++ b/packages/components/src/core/ButtonToggle/__tests__/index.test.tsx @@ -0,0 +1,90 @@ +import { generateSnapshots } from "@chanzuckerberg/story-utils"; +import { composeStories } from "@storybook/react"; +import { cleanup, render, screen } from "@testing-library/react"; +import * as stories from "../__storybook__/index.stories"; +import { SDS_WARNINGS, SDSWarningTypes } from "src/common/warnings"; + +const { Test } = composeStories(stories); + +const BUTTON_TOGGLE_TEST_ID = "button-toggle"; + +describe("", () => { + generateSnapshots(stories); + + it("renders ButtonToggle component", () => { + render(); + const panelElement = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(panelElement).not.toBeNull(); + }); + + it("renders with different sdsSize values", () => { + render(); + const smallButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(smallButton).toBeInTheDocument(); + + // (masoudmanson): cleanup is necessary to avoid having + // multiple elements with the same test id in the DOM + cleanup(); + + render(); + const largeButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(largeButton).toBeInTheDocument(); + }); + + it("renders with different sdsStage values", () => { + render(); + const onStageButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(onStageButton).toBeInTheDocument(); + + cleanup(); + + render(); + const offStageButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(offStageButton).toBeInTheDocument(); + }); + + it("renders with different sdsType values", () => { + render(); + const primaryButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(primaryButton).toBeInTheDocument(); + + cleanup(); + + render(); + const secondaryButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(secondaryButton).toBeInTheDocument(); + }); + + it("displays warning when icon is missing", () => { + const warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + render(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining( + SDS_WARNINGS[SDSWarningTypes.ButtonToggleMissingIconProp].message + ) + ); + warnSpy.mockRestore(); + }); + + it("displays an error when an icon doesn't support the ButtonToggle size", () => { + const errorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + // (masoudmanson): SlidersHorizontal icon doesn't support the small size + // make sure to change this to another icon if the SlidersHorizontal icon is updated + const SdsIconWithoutSmallSize = "SlidersHorizontal"; + render( + + ); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining( + `Error: Icon ${SdsIconWithoutSmallSize} not found for size s. This is a @czi-sds/components problem.` + ) + ); + errorSpy.mockRestore(); + }); + + it("renders with disabled state", () => { + render(); + const disabledButton = screen.getByTestId(BUTTON_TOGGLE_TEST_ID); + expect(disabledButton).toBeDisabled(); + }); +}); diff --git a/packages/components/src/core/ButtonToggle/index.tsx b/packages/components/src/core/ButtonToggle/index.tsx new file mode 100644 index 000000000..80e06641f --- /dev/null +++ b/packages/components/src/core/ButtonToggle/index.tsx @@ -0,0 +1,50 @@ +import React from "react"; +import { ButtonProps } from "@mui/material"; +import { IconNameToSizes } from "../Icon"; +import { + SDSWarningTypes, + showWarningIfFirstOccurence, +} from "src/common/warnings"; +import { StyledButtonToggle } from "./style"; + +export interface ButtonToggleProps extends ButtonProps { + disabled?: boolean; + icon: keyof IconNameToSizes | React.ReactElement; + sdsSize?: "small" | "medium" | "large"; + sdsStage?: "on" | "off"; + sdsType?: "primary" | "secondary"; +} + +/** + * @see https://mui.com/material-ui/react-button/ + */ + +const ButtonToggle = React.forwardRef( + (props, ref) => { + const { + icon, + sdsSize = "medium", + sdsStage = "off", + sdsType = "primary", + ...rest + } = props; + + if (icon !== undefined) { + return ( + + ); + } else { + showWarningIfFirstOccurence(SDSWarningTypes.ButtonToggleMissingIconProp); + return null; + } + } +); + +export default ButtonToggle; diff --git a/packages/components/src/core/ButtonToggle/style.ts b/packages/components/src/core/ButtonToggle/style.ts new file mode 100644 index 000000000..1effb6933 --- /dev/null +++ b/packages/components/src/core/ButtonToggle/style.ts @@ -0,0 +1,30 @@ +import styled from "@emotion/styled"; +import ButtonIcon from "../ButtonIcon"; +import { CommonThemeProps, getSemanticColors } from "../styles"; +import { ButtonToggleProps } from "."; + +/** + * (masoudmanson): Since StyledButtonToggle is built on top of ButtonIcon, + * we only need to exclude the `sdsStage` prop from being forwarded, + * as it is not a valid prop for ButtonIcon. + * All other props should be passed down to ButtonIcon; otherwise, + * the component won’t function as expected. + */ +const doNotForwardProps = ["sdsStage"]; + +interface ButtonToggleExtraProps extends CommonThemeProps { + sdsStage?: ButtonToggleProps["sdsStage"]; +} + +export const StyledButtonToggle = styled(ButtonIcon, { + shouldForwardProp: (prop: string) => !doNotForwardProps.includes(prop), +})` + ${(props: ButtonToggleExtraProps) => { + const { sdsStage } = props; + const semanticColors = getSemanticColors(props); + + return ` + background-color: ${sdsStage === "on" ? semanticColors?.base?.fillHover : "transparent"}; + `; + }} +`; diff --git a/packages/components/src/core/CellBasic/__storybook__/style.ts b/packages/components/src/core/CellBasic/__storybook__/style.ts index ad7abc929..546666d1f 100644 --- a/packages/components/src/core/CellBasic/__storybook__/style.ts +++ b/packages/components/src/core/CellBasic/__storybook__/style.ts @@ -16,7 +16,6 @@ export const ButtonIconsGroupRight = styled("div")` return ` align-items: center; display: inline-flex; - gap: ${spaces?.xxxs}px; height: 100%; border-left: solid 1px ${semanticColors?.base?.divider}; padding-left: ${spaces?.xs}px; @@ -25,14 +24,7 @@ export const ButtonIconsGroupRight = styled("div")` `; export const ButtonIconsGroupBottom = styled("div")` - ${(props: CommonThemeProps) => { - const spaces = getSpaces(props); - - return ` - display: inline-flex; - gap: ${spaces?.xs}px; - `; - }} + display: inline-flex; `; export const StyledButton = styled(Button)` @@ -95,8 +87,8 @@ export const StyledCellBasic = styled(CellBasic)` return ` border: dashed 1px ${semanticColors?.base?.divider}; height: 70px; - maxWidth: 250px; - width: 250px; + max-width: 300px; + width: 300px; `; }} `; diff --git a/packages/components/src/core/Dialog/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/Dialog/__tests__/__snapshots__/index.test.tsx.snap index a97786750..8339b6ff5 100644 --- a/packages/components/src/core/Dialog/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/Dialog/__tests__/__snapshots__/index.test.tsx.snap @@ -22,7 +22,7 @@ exports[` Dialog all sizes match the snapshots 1`] = ` > + sdsStage={open ? "on" : "off"} + />

{LONG_LOREM_IPSUM}

{LONG_LOREM_IPSUM}

diff --git a/packages/components/src/core/Panel/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/Panel/__tests__/__snapshots__/index.test.tsx.snap index 07ef7858d..2e49b56ce 100644 --- a/packages/components/src/core/Panel/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/Panel/__tests__/__snapshots__/index.test.tsx.snap @@ -3,7 +3,6 @@ exports[` Default story renders snapshot 1`] = `
Default story renders snapshot 1`] = `