From 2efaf34953a8b6a095d7c2b19460db4742086ebe Mon Sep 17 00:00:00 2001 From: Masoud Amjadi Date: Mon, 27 Jan 2025 21:09:47 -0500 Subject: [PATCH 1/3] fix(headercell): added new hover prop to control component hover state --- .../__storybook__/index.stories.tsx | 4 ++++ .../__tests__/CellHeader.namespace-test.tsx | 1 + .../components/src/core/CellHeader/index.tsx | 19 ++++++++++++------ .../components/src/core/CellHeader/style.ts | 20 ++++++++++++++----- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx b/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx index 4e435cd4b..e25d77c56 100644 --- a/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx +++ b/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx @@ -20,6 +20,9 @@ export default { control: { type: "select" }, options: ["left", "center", "right"], }, + hover: { + control: { type: "boolean" }, + }, shouldShowTooltipOnHover: { control: { type: "boolean" }, }, @@ -44,6 +47,7 @@ export const Default = { active: false, direction: "desc", hideSortIcon: false, + hover: true, shouldShowTooltipOnHover: true, tooltipProps: { sdsStyle: "dark" }, tooltipText: "This is a header cell", diff --git a/packages/components/src/core/CellHeader/__tests__/CellHeader.namespace-test.tsx b/packages/components/src/core/CellHeader/__tests__/CellHeader.namespace-test.tsx index 542afda00..a482b6b51 100644 --- a/packages/components/src/core/CellHeader/__tests__/CellHeader.namespace-test.tsx +++ b/packages/components/src/core/CellHeader/__tests__/CellHeader.namespace-test.tsx @@ -12,6 +12,7 @@ const CellBasicNameSpaceTest = (props: CellHeaderProps) => { direction="asc" active hideSortIcon + hover horizontalAlign="center" shouldShowTooltipOnHover tooltipProps={{ sdsStyle: "light" }} diff --git a/packages/components/src/core/CellHeader/index.tsx b/packages/components/src/core/CellHeader/index.tsx index cdcdb42f4..241a3d0ed 100644 --- a/packages/components/src/core/CellHeader/index.tsx +++ b/packages/components/src/core/CellHeader/index.tsx @@ -16,6 +16,7 @@ interface CellHeaderContentProps { hideSortIcon?: boolean; horizontalAlign?: "left" | "center" | "right"; children: React.ReactNode; + hover?: boolean; } interface CellHeaderRawProps @@ -39,6 +40,7 @@ const CellHeaderContent = ( direction = "desc", hideSortIcon = false, horizontalAlign, + hover, } = props; const sdsIconName: keyof IconNameToSizes = @@ -61,7 +63,7 @@ const CellHeaderContent = ( return ( {children} - {(!hideSortIcon || active) && sortIcon} + {(!hideSortIcon || active) && hover && sortIcon} ); }; @@ -74,10 +76,11 @@ const CellHeader = forwardRef( tooltipProps, tooltipText = "", tooltipSubtitle, + hover = true, ...rest } = props; - if (shouldShowTooltipOnHover) { + if (shouldShowTooltipOnHover && hover) { return ( ( title={tooltipText} {...tooltipProps} > - - {children} + + + {children} + ); } return ( - - {children} + + + {children} + ); } diff --git a/packages/components/src/core/CellHeader/style.ts b/packages/components/src/core/CellHeader/style.ts index 3a2f83535..df6118280 100644 --- a/packages/components/src/core/CellHeader/style.ts +++ b/packages/components/src/core/CellHeader/style.ts @@ -12,6 +12,7 @@ export interface CellHeaderExtraProps extends CommonThemeProps { active?: boolean; hideSortIcon?: boolean; horizontalAlign?: "left" | "center" | "right"; + hover?: boolean; } const contentPositionMapping = { @@ -27,6 +28,7 @@ const doNotForwardProps = [ "tooltipProps", "tooltipText", "hideSortIcon", + "hover", ]; export const StyledSortingIcon = styled(Icon, { @@ -55,17 +57,25 @@ export const StyledTableHeader = styled("th", { ${focusVisibleA11yStyle} ${(props: CellHeaderExtraProps) => { - const { active = false, horizontalAlign = "left" } = props; + const { active = false, horizontalAlign = "left", hover = true } = props; const spaces = getSpaces(props); const semanticColors = getSemanticColors(props); + const defaultColor = active + ? semanticColors?.accent?.textAction + : semanticColors?.base?.textSecondary; + + const hoverColor = active + ? semanticColors?.accent?.textActionHover + : semanticColors?.base?.textPrimary; + return ` - color: ${active ? semanticColors?.accent?.textAction : semanticColors?.base?.textSecondary}; + color: ${defaultColor}; padding: ${spaces?.l}px ${spaces?.m}px; text-align: ${horizontalAlign}; min-width: 96px; - cursor: pointer; + cursor: ${hover ? "pointer" : "default"}; vertical-align: bottom; & .MuiButtonBase-root { @@ -73,10 +83,10 @@ export const StyledTableHeader = styled("th", { } &:hover { - color: ${active ? semanticColors?.accent?.textActionHover : semanticColors?.base?.textPrimary}; + color: ${hover ? hoverColor : defaultColor}; & .MuiButtonBase-root { - color: ${active ? semanticColors?.accent?.textActionHover : semanticColors?.base?.textPrimary}; + color: ${hoverColor}; opacity: 1; } From 6938adfd1e5762cec05f26aa1434579f417367d6 Mon Sep 17 00:00:00 2001 From: Masoud Amjadi Date: Thu, 30 Jan 2025 13:32:47 -0500 Subject: [PATCH 2/3] refactor(cellheader): changes default value of hover prop to false --- .../__storybook__/index.stories.tsx | 2 +- .../CellHeader/__storybook__/stories/test.tsx | 4 +- .../__snapshots__/index.test.tsx.snap | 19 +------ .../core/CellHeader/__tests__/index.test.tsx | 6 +-- .../components/src/core/CellHeader/index.tsx | 2 +- .../__snapshots__/index.test.tsx.snap | 40 ++------------- .../__snapshots__/index.test.tsx.snap | 51 ++----------------- 7 files changed, 17 insertions(+), 107 deletions(-) diff --git a/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx b/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx index e25d77c56..262132d4f 100644 --- a/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx +++ b/packages/components/src/core/CellHeader/__storybook__/index.stories.tsx @@ -47,7 +47,7 @@ export const Default = { active: false, direction: "desc", hideSortIcon: false, - hover: true, + hover: false, shouldShowTooltipOnHover: true, tooltipProps: { sdsStyle: "dark" }, tooltipText: "This is a header cell", diff --git a/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx b/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx index 6e85f594b..44b5aa4d0 100644 --- a/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx +++ b/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx @@ -1,6 +1,7 @@ +import { Args } from "@storybook/types"; import RawCellHeader from "src/core/CellHeader"; -export const TestDemo = (): JSX.Element => ( +export const TestDemo = (props: Args): JSX.Element => ( @@ -10,6 +11,7 @@ export const TestDemo = (): JSX.Element => ( shouldShowTooltipOnHover active tooltipText="testTooltipTitle" + {...props} > Header diff --git a/packages/components/src/core/CellHeader/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/CellHeader/__tests__/__snapshots__/index.test.tsx.snap index 563cf01ff..736ce6faa 100644 --- a/packages/components/src/core/CellHeader/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/CellHeader/__tests__/__snapshots__/index.test.tsx.snap @@ -5,10 +5,8 @@ exports[` Default story renders snapshot 1`] = ` diff --git a/packages/components/src/core/CellHeader/__tests__/index.test.tsx b/packages/components/src/core/CellHeader/__tests__/index.test.tsx index 0459cb089..5113fb701 100644 --- a/packages/components/src/core/CellHeader/__tests__/index.test.tsx +++ b/packages/components/src/core/CellHeader/__tests__/index.test.tsx @@ -16,7 +16,7 @@ describe("", () => { }); it("renders tooltip on hover", async () => { - render(); + render(); const headerCellElement = screen.getByTestId("CellHeader"); fireEvent.mouseOver(headerCellElement); await screen.findByText("testTooltipTitle"); @@ -29,8 +29,8 @@ describe("", () => { expect(style.textAlign).toBe("right"); }); - it("renders a sort icon when header is active", async () => { - render(); + it("renders a sort icon when header is active and hover is true", async () => { + render(); const headerCellElement = screen.getByTestId("CellHeader"); const sortIcon = headerCellElement.getElementsByClassName("MuiSvgIcon-root")[0]; diff --git a/packages/components/src/core/CellHeader/index.tsx b/packages/components/src/core/CellHeader/index.tsx index 241a3d0ed..4f2fccfa8 100644 --- a/packages/components/src/core/CellHeader/index.tsx +++ b/packages/components/src/core/CellHeader/index.tsx @@ -76,7 +76,7 @@ const CellHeader = forwardRef( tooltipProps, tooltipText = "", tooltipSubtitle, - hover = true, + hover = false, ...rest } = props; diff --git a/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap index 49a89cb5c..f53968f0a 100644 --- a/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap @@ -11,7 +11,7 @@ exports[`
Default story renders snapshot 1`] = ` Header -
- -
Default story renders snapshot 1`] = ` class="css-fitsu2" > From b2ee0df3f49ccd07713d0275620e710d62898f17 Mon Sep 17 00:00:00 2001 From: Masoud Amjadi Date: Thu, 30 Jan 2025 13:48:21 -0500 Subject: [PATCH 3/3] fix(cellheader): fixed test stories --- .../CellHeader/__storybook__/stories/test.tsx | 1 + .../Table/__storybook__/stories/default.tsx | 6 ++- .../__snapshots__/index.test.tsx.snap | 34 ++++++++++++- .../__storybook__/stories/default.tsx | 8 +-- .../__storybook__/stories/test.tsx | 8 +-- .../__snapshots__/index.test.tsx.snap | 51 +++++++++++++++++-- 6 files changed, 95 insertions(+), 13 deletions(-) diff --git a/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx b/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx index 44b5aa4d0..5f2383b6b 100644 --- a/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx +++ b/packages/components/src/core/CellHeader/__storybook__/stories/test.tsx @@ -11,6 +11,7 @@ export const TestDemo = (props: Args): JSX.Element => ( shouldShowTooltipOnHover active tooltipText="testTooltipTitle" + hover {...props} > Header diff --git a/packages/components/src/core/Table/__storybook__/stories/default.tsx b/packages/components/src/core/Table/__storybook__/stories/default.tsx index 93f996fc3..a3dae5aa1 100644 --- a/packages/components/src/core/Table/__storybook__/stories/default.tsx +++ b/packages/components/src/core/Table/__storybook__/stories/default.tsx @@ -19,8 +19,10 @@ export const Table = (props: Args): JSX.Element => { Category - Active Header - + + Active Header + + A very long table header title to test sort icon positioning Component diff --git a/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap index f53968f0a..0d9e23f07 100644 --- a/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/Table/__tests__/__snapshots__/index.test.tsx.snap @@ -22,7 +22,7 @@ exports[`
Default story renders snapshot 1`] = `
Default story renders snapshot 1`] = ` Active Header -
- -
Default story renders snapshot 1`] = ` A very long table header title to test sort icon positioning -
- -
Default story renders snapshot 1`] = `
Default story renders snapshot 1`] = ` class="css-fitsu2" >
Default story renders snapshot 1`] = ` Column 1 -
- -
Default story renders snapshot 1`] = ` Column 2 -
- -
Default story renders snapshot 1`] = ` Column 3 -
- -
Default story renders snapshot 1`] = `
Default story renders snapshot 1`] = ` Active Header +
+ +
Default story renders snapshot 1`] = ` A very long table header title to test sort icon positioning +
+ +
{ return ( - Column 1 - Column 2 - Column 3 + + Column 1 + + Column 2 + Column 3
); diff --git a/packages/components/src/core/TableHeader/__storybook__/stories/test.tsx b/packages/components/src/core/TableHeader/__storybook__/stories/test.tsx index fbf69a6df..c77ddb134 100644 --- a/packages/components/src/core/TableHeader/__storybook__/stories/test.tsx +++ b/packages/components/src/core/TableHeader/__storybook__/stories/test.tsx @@ -4,9 +4,11 @@ import RawTableHeader from "src/core/TableHeader"; export const TestDemo = (): JSX.Element => ( - Column 1 - Column 2 - Column 3 + + Column 1 + + Column 2 + Column 3
); diff --git a/packages/components/src/core/TableHeader/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/TableHeader/__tests__/__snapshots__/index.test.tsx.snap index 327a55352..586c8390d 100644 --- a/packages/components/src/core/TableHeader/__tests__/__snapshots__/index.test.tsx.snap +++ b/packages/components/src/core/TableHeader/__tests__/__snapshots__/index.test.tsx.snap @@ -9,7 +9,7 @@ exports[` Default story renders snapshot 1`] = ` class="css-fitsu2" >
Default story renders snapshot 1`] = ` Column 1 +
+ +
Default story renders snapshot 1`] = ` Column 2 +
+ +
Default story renders snapshot 1`] = ` Column 3 +
+ +