From 2033fecd4e0630b7ca88e591416db7d661458697 Mon Sep 17 00:00:00 2001 From: Russell Anderson Date: Mon, 26 Sep 2022 18:18:40 -0500 Subject: [PATCH] fix: pagination as links bug, fullscreen props (#773) * fix: pagination as links bug, fullscreen props This began as a refactor of several components away from classes. While doing so I was able to remove the Delegate library that was throwing react errors because it was very out of date. Additonally I found a bug with the way proptypes were defined for headerComponent in two components, and also a bug with the pagination when trying to use the pager buttons as links. * refactor: allow any to preserve bwards compat --- package-lock.json | 17 -- package.json | 1 - packages/donutChart/components/DonutChart.tsx | 117 ++++++------- .../donutChart/stories/DonutChart.stories.tsx | 2 +- packages/donutChart/tests/DonutChart.test.tsx | 11 +- .../__snapshots__/DonutChart.test.tsx.snap | 2 - .../stories/helpers/DropdownStory.tsx | 12 +- .../stories/helpers/DropdownStoryFit.tsx | 109 +++++------- .../components/FullscreenView.tsx | 45 ++--- .../fullscreenView.test.tsx.snap | 154 ++++++++--------- packages/modal/components/FullscreenModal.tsx | 30 ++-- .../tests/__snapshots__/Modal.test.tsx.snap | 158 ++++++++---------- packages/pagination/Pagination.tsx | 62 +++---- .../__snapshots__/pagination.test.tsx.snap | 1 - 14 files changed, 317 insertions(+), 404 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5d078b710..a00900230 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,7 +18,6 @@ "copy-to-clipboard": "3.3.2", "downshift": "6.1.9", "react-click-outside": "3.0.1", - "react-delegate-component": "1.0.0", "react-focus-lock": "2.9.1", "react-id-generator": "3.0.2", "react-popper": "2.3.0", @@ -30856,16 +30855,6 @@ "react": "^15.3.0 || ^16.0.0 || ^17.0.0 || ^18.0.0" } }, - "node_modules/react-delegate-component": { - "version": "1.0.0", - "license": "MIT", - "dependencies": { - "prop-types": "^15.6.0" - }, - "peerDependencies": { - "react": ">=15" - } - }, "node_modules/react-docgen": { "version": "5.4.0", "dev": true, @@ -59193,12 +59182,6 @@ "@babel/runtime": "^7.12.13" } }, - "react-delegate-component": { - "version": "1.0.0", - "requires": { - "prop-types": "^15.6.0" - } - }, "react-docgen": { "version": "5.4.0", "dev": true, diff --git a/package.json b/package.json index 68604decf..13f0a31c8 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,6 @@ "copy-to-clipboard": "3.3.2", "downshift": "6.1.9", "react-click-outside": "3.0.1", - "react-delegate-component": "1.0.0", "react-focus-lock": "2.9.1", "react-id-generator": "3.0.2", "react-popper": "2.3.0", diff --git a/packages/donutChart/components/DonutChart.tsx b/packages/donutChart/components/DonutChart.tsx index 7171cb002..442b506d5 100644 --- a/packages/donutChart/components/DonutChart.tsx +++ b/packages/donutChart/components/DonutChart.tsx @@ -27,74 +27,67 @@ export interface DonutChartProps { text?: string; } -class DonutChart extends React.PureComponent { - public render() { - const { data, label, text } = this.props; - const strokeWidth = 1.5; - const radius = 100 / (Math.PI * 2); - const diameter = radius * 2 + strokeWidth; - const circleCenter = diameter / 2; +const DonutChart = React.memo(({ data, label, text }: DonutChartProps) => { + const strokeWidth = 1.5; + const radius = 100 / (Math.PI * 2); + const diameter = radius * 2 + strokeWidth; + const circleCenter = diameter / 2; - return ( - - - {data.map((datum, i) => { - const { percentage, color } = datum; - const precedingTotal = data - .slice(0, i) - .reduce((acc, curr) => acc + curr.percentage, 0); + return ( + + + {data.map((datum, i) => { + const { percentage, color } = datum; + const precedingTotal = data + .slice(0, i) + .reduce((acc, curr) => acc + curr.percentage, 0); - // total - sum of preceding segments + 1/4 total to offset the segments -90deg - const dashoffset = 100 - precedingTotal + 25; + // total - sum of preceding segments + 1/4 total to offset the segments -90deg + const dashoffset = 100 - precedingTotal + 25; - return ( - - ); - })} - + ); + })} + + + {label} + + {text && ( - {label} + {text} - {text && ( - - {text} - - )} - - - ); - } -} + )} + + + ); +}); export default DonutChart; diff --git a/packages/donutChart/stories/DonutChart.stories.tsx b/packages/donutChart/stories/DonutChart.stories.tsx index 3e8f706ab..6a0875754 100644 --- a/packages/donutChart/stories/DonutChart.stories.tsx +++ b/packages/donutChart/stories/DonutChart.stories.tsx @@ -1,6 +1,6 @@ import * as React from "react"; import { DonutChart } from "../index"; -import { Story, Meta } from "@storybook/react"; +import { Meta } from "@storybook/react"; import { purple, pink, blue } from "../../design-tokens/build/js/designTokens"; import { css } from "@emotion/css"; diff --git a/packages/donutChart/tests/DonutChart.test.tsx b/packages/donutChart/tests/DonutChart.test.tsx index 226ac3228..fc40bdc47 100644 --- a/packages/donutChart/tests/DonutChart.test.tsx +++ b/packages/donutChart/tests/DonutChart.test.tsx @@ -1,7 +1,6 @@ import React from "react"; -import { shallow } from "enzyme"; import { createSerializer } from "@emotion/jest"; -import toJson from "enzyme-to-json"; +import { create } from "react-test-renderer"; import { DonutChart } from "../"; @@ -9,7 +8,7 @@ expect.addSnapshotSerializer(createSerializer()); describe("DonutChart", () => { it("renders default", () => { - const component = shallow( + const component = create( { /> ); - expect(toJson(component)).toMatchSnapshot(); + expect(component.toJSON()).toMatchSnapshot(); }); it("renders with text and label", () => { - const component = shallow( + const component = create( { /> ); - expect(toJson(component)).toMatchSnapshot(); + expect(component.toJSON()).toMatchSnapshot(); }); }); diff --git a/packages/donutChart/tests/__snapshots__/DonutChart.test.tsx.snap b/packages/donutChart/tests/__snapshots__/DonutChart.test.tsx.snap index d8ac1d7e3..cb9bcd9b2 100644 --- a/packages/donutChart/tests/__snapshots__/DonutChart.test.tsx.snap +++ b/packages/donutChart/tests/__snapshots__/DonutChart.test.tsx.snap @@ -21,7 +21,6 @@ exports[`DonutChart renders default 1`] = ` cx={16.665494309189533} cy={16.665494309189533} fill="none" - key="0" r={15.915494309189533} strokeDasharray="10 90" strokeDashoffset={125} @@ -82,7 +81,6 @@ exports[`DonutChart renders with text and label 1`] = ` cx={16.665494309189533} cy={16.665494309189533} fill="none" - key="0" r={15.915494309189533} strokeDasharray="10 90" strokeDashoffset={125} diff --git a/packages/dropdownable/stories/helpers/DropdownStory.tsx b/packages/dropdownable/stories/helpers/DropdownStory.tsx index 8196e036d..e26476d07 100644 --- a/packages/dropdownable/stories/helpers/DropdownStory.tsx +++ b/packages/dropdownable/stories/helpers/DropdownStory.tsx @@ -1,8 +1,8 @@ -import React from "react"; +import React, { ReactNode } from "react"; import { css } from "@emotion/css"; import styled from "@emotion/styled"; -import Dropdownable from "../../components/Dropdownable"; +import Dropdownable, { Direction } from "../../components/Dropdownable"; import { PrimaryButton } from "../../../button"; import { themeBgPrimary, @@ -16,7 +16,13 @@ export const DropdownContentContainer = styled.div` padding: 5px; `; -const DropdownStory = ({ children, preferredDirections }) => { +const DropdownStory = ({ + children, + preferredDirections +}: { + children: ReactNode; + preferredDirections?: Direction | Direction[]; +}) => { const [isShowing, setIsShowing] = React.useState(false); function toggle() { diff --git a/packages/dropdownable/stories/helpers/DropdownStoryFit.tsx b/packages/dropdownable/stories/helpers/DropdownStoryFit.tsx index 7c6cee7e1..0b3520de5 100644 --- a/packages/dropdownable/stories/helpers/DropdownStoryFit.tsx +++ b/packages/dropdownable/stories/helpers/DropdownStoryFit.tsx @@ -5,76 +5,57 @@ import Dropdownable, { Direction } from "../../components/Dropdownable"; import { PrimaryButton, SecondaryButton } from "../../../button"; import { DropdownContentContainer } from "./DropdownStory"; -class DropdownStoryFit extends React.PureComponent< - {}, - { isOpen: boolean; expanded: boolean } -> { - constructor(props) { - super(props); - - this.state = { - expanded: false, - isOpen: false - }; - - this.toggleExpand = this.toggleExpand.bind(this); - this.handleClose = this.handleClose.bind(this); - this.handleOpen = this.handleOpen.bind(this); - } - - toggleExpand() { - this.setState({ expanded: !this.state.expanded }); +const DropdownStoryFit = ({ children }) => { + const [isOpen, setIsOpen] = React.useState(false); + const [expanded, setExpanded] = React.useState(false); + + const containerStyle = css({ + display: "flex", + ["align-items"]: "center", + ["justify-content"]: "flex-end", + ["min-height"]: expanded ? "400px" : "40px" + }); + + function toggleExpand() { + setExpanded(!expanded); } - handleClose() { - this.setState({ - isOpen: false - }); + function handleClose() { + setIsOpen(false); } - handleOpen() { - this.setState({ isOpen: true }); + function handleOpen() { + setIsOpen(true); } - render() { - const { children } = this.props; - - const containerStyle = css({ - display: "flex", - ["align-items"]: "center", - ["justify-content"]: "flex-end", - ["min-height"]: this.state.expanded ? "400px" : "40px" - }); - - return ( -
-
- -

I prefer to be positioned on the top

-

Click outside to dismiss

-

Also try resizing

-

Click the other button to make more vertical space

-

Click the other button to make more vertical space

-

Click the other button to make more vertical space

-

Click the other button to make more vertical space

-

Click the other button to make more vertical space

- - } - > - {children} -
-
- - {this.state.expanded ? "Collapse" : "Expand"} - + return ( +
+
+ +

I prefer to be positioned on the top

+

Click outside to dismiss

+

Also try resizing

+

Click the other button to make more vertical space

+

Click the other button to make more vertical space

+

Click the other button to make more vertical space

+

Click the other button to make more vertical space

+

Click the other button to make more vertical space

+ + } + > + {children} +
- ); - } -} + + {expanded ? "Collapse" : "Expand"} + +
+ ); +}; export default DropdownStoryFit; diff --git a/packages/fullscreenView/components/FullscreenView.tsx b/packages/fullscreenView/components/FullscreenView.tsx index 8ee5fdfdd..3039cb1c7 100644 --- a/packages/fullscreenView/components/FullscreenView.tsx +++ b/packages/fullscreenView/components/FullscreenView.tsx @@ -1,7 +1,5 @@ import * as React from "react"; import { cx } from "@emotion/css"; -// TODO: This component contains `willReceiveProps` -import Delegate from "react-delegate-component"; import { ButtonProps } from "../../button/components/ButtonBase"; import { flex, padding, flexItem } from "../../shared/styles/styleUtils"; import { modalContent, fullscreenModalHeader } from "../style"; @@ -19,29 +17,30 @@ export interface FullscreenViewProps { /** Whether we automatically add padding to the body of the modal. */ isContentFlush?: boolean; /** Custom header content component. ⚠️Use rarely and with caution⚠️ */ - headerComponent?: React.ReactNode; + headerComponent?: React.JSXElementConstructor; /** Custom banner component that will appear above the header. */ bannerComponent?: React.ReactNode; /** Function that gets called when the modal is closed */ onClose: (event?: React.SyntheticEvent) => void; /** The base of the `data-cy` value. This is used for writing selectors in Cypress. */ cypressSelectorBase?: string; + children?: React.ReactNode; } -class FullscreenView extends React.PureComponent { - public render() { - const { - children, - ctaButton, - closeText, - isContentFlush, - onClose, - title, - subtitle, - bannerComponent, - headerComponent, - cypressSelectorBase = "fullscreenView" - } = this.props; +const FullscreenView = React.memo( + ({ + children, + ctaButton, + closeText, + isContentFlush, + onClose, + title, + subtitle, + bannerComponent, + headerComponent, + cypressSelectorBase = "fullscreenView" + }: FullscreenViewProps) => { + const HeaderComponent = headerComponent ?? FullscreenViewHeader; return (
@@ -55,10 +54,12 @@ class FullscreenView extends React.PureComponent {
)}
-
@@ -73,6 +74,6 @@ class FullscreenView extends React.PureComponent { ); } -} +); export default FullscreenView; diff --git a/packages/fullscreenView/tests/__snapshots__/fullscreenView.test.tsx.snap b/packages/fullscreenView/tests/__snapshots__/fullscreenView.test.tsx.snap index eeda56bcf..1b8527f36 100644 --- a/packages/fullscreenView/tests/__snapshots__/fullscreenView.test.tsx.snap +++ b/packages/fullscreenView/tests/__snapshots__/fullscreenView.test.tsx.snap @@ -216,7 +216,7 @@ exports[`FullscreenView renders FullscreenView 1`] = ` padding: 32px; } - @@ -236,104 +236,88 @@ exports[`FullscreenView renders FullscreenView 1`] = `
- - CTA - , - "onClose": [MockFunction], - "subtitle": undefined, - "title": "Title", - } + + CTA + } + onClose={[MockFunction]} + title="Title" > - - CTA - - } - onClose={[MockFunction]} - title="Title" +
-
- - - - - - - -
-
+ Close + + + + + +
+
+

-

+

+

+
+ + - Title - -

-

-
- - - - - - - -
+ + CTA + + + + +
- - +
+
-
+ `; diff --git a/packages/modal/components/FullscreenModal.tsx b/packages/modal/components/FullscreenModal.tsx index 193231d4e..417b90fa1 100644 --- a/packages/modal/components/FullscreenModal.tsx +++ b/packages/modal/components/FullscreenModal.tsx @@ -16,23 +16,21 @@ interface FullscreenModalProps extends ModalBaseProps { /** Whether we automatically add padding to the body of the modal. */ isContentFlush?: boolean; /** Custom header content component. ⚠️Use rarely and with caution⚠️ */ - headerComponent?: React.ReactNode; + headerComponent?: React.JSXElementConstructor; } -class FullscreenModal extends React.PureComponent { - public render() { - const { - children, - ctaButton, - closeText, - isContentFlush, - onClose, - title, - subtitle, - headerComponent, - ...other - } = this.props; - +const FullscreenModal = React.memo( + ({ + children, + ctaButton, + closeText, + isContentFlush, + onClose, + title, + subtitle, + headerComponent, + ...other + }: FullscreenModalProps) => { return ( { ); } -} +); export default FullscreenModal; diff --git a/packages/modal/tests/__snapshots__/Modal.test.tsx.snap b/packages/modal/tests/__snapshots__/Modal.test.tsx.snap index 413b99734..7aeb72c54 100644 --- a/packages/modal/tests/__snapshots__/Modal.test.tsx.snap +++ b/packages/modal/tests/__snapshots__/Modal.test.tsx.snap @@ -3106,7 +3106,7 @@ exports[`Modal FullscreenModal renders FullscreenModal 1`] = ` padding: 32px; } - @@ -5589,7 +5589,7 @@ exports[`Modal FullscreenModal renders FullscreenModal 1`] = ` className="emotion-3" data-cy="fullscreenModal" > - @@ -5610,104 +5610,88 @@ exports[`Modal FullscreenModal renders FullscreenModal 1`] = `
- - CTA - , - "onClose": [MockFunction], - "subtitle": undefined, - "title": "Title", - } + + CTA + } + onClose={[MockFunction]} + title="Title" > - - CTA - - } - onClose={[MockFunction]} - title="Title" +
-
- - - - - - - -
-
+ Close + + + + + +
+
+

-

+

+

+
+ + - Title - -

-

-
- - - - - - - -
+ + CTA + + + + +
- - +
+
-
+ @@ -5750,7 +5734,7 @@ exports[`Modal FullscreenModal renders FullscreenModal 1`] = ` -
+ `; exports[`Modal ModalBase renders ModalBase 1`] = ` diff --git a/packages/pagination/Pagination.tsx b/packages/pagination/Pagination.tsx index fa19d7c80..a22907128 100644 --- a/packages/pagination/Pagination.tsx +++ b/packages/pagination/Pagination.tsx @@ -1,5 +1,4 @@ import * as React from "react"; -import Delegate from "react-delegate-component"; import { useId } from "react-id-generator"; import { cx } from "@emotion/css"; import { Flex, FlexItem } from "../styleUtils/layout"; @@ -73,41 +72,36 @@ const getItemCountClassName = (showPageLengthMenu?: boolean) => { return cx(border("right"), padding("right"), margin("right")); }; -const NavButton: React.FC< - Partial> & - Partial & { direction: "prev" | "next" } -> = ({ direction, disabled, url, ...other }) => { - delete other.children; - +const NavButton = ({ + direction, + disabled, + url, + ...other +}: Partial> & + Partial & { direction: "prev" | "next" }) => { const ariaLabel = direction === "prev" ? "previous page" : "next page"; - const children = ( - - ); + + const Component = url ? ResetLink : ResetButton; return ( - + + + ); }; -const Pagination: React.FC = ({ +const Pagination = ({ activePage: activePageProp, initialActivePage = 1, itemsLabel = DEFAULT_ITEMS_LABEL, @@ -119,7 +113,7 @@ const Pagination: React.FC = ({ showPageLengthMenu, totalItems, totalPages: totalPagesProp -}) => { +}: PaginationProps) => { const [pageLengthMenuId] = useId(1, "pageLengthMenu"); const [activePageState, setActivePageState] = React.useState(initialActivePage); @@ -259,10 +253,4 @@ const Pagination: React.FC = ({ ); }; -Pagination.defaultProps = { - initialActivePage: 1, - initialPageLength: INITIAL_PAGE_LENGTH, - itemsLabel: DEFAULT_ITEMS_LABEL -}; - export default Pagination; diff --git a/packages/pagination/__snapshots__/pagination.test.tsx.snap b/packages/pagination/__snapshots__/pagination.test.tsx.snap index 71721cb7b..8f2f1978a 100644 --- a/packages/pagination/__snapshots__/pagination.test.tsx.snap +++ b/packages/pagination/__snapshots__/pagination.test.tsx.snap @@ -462,7 +462,6 @@ exports[`Pagination rendering renders wrapped in a PaginationContainer 1`] = `