From 99b0c8639f3a03d5aea57b24a1aaae5fe2d11abe Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Wed, 12 Apr 2023 10:02:16 +0300 Subject: [PATCH] Introduce Element pattern to reduce duplication in UI code Co-authored-by: Mikko Aspiala Signed-off-by: Janne Savolainen --- .../technical-features/ui-components/index.ts | 2 + .../ui-components/package.json | 6 +- .../src/element/element-props.ts | 9 +++ .../ui-components/src/element/element.tsx | 18 ++++++ .../ui-components/src/element/elements.ts | 5 ++ .../class-names/class-names.test.tsx | 56 +++++++++++++++++++ .../class-names/class-names.ts | 18 ++++++ .../vanilla-class-name-adapter.test.tsx | 42 ++++++++++++++ .../class-names/vanilla-class-name-adapter.ts | 10 ++++ .../flex/flex-parent.test.tsx | 50 +++++++++++++++++ .../prop-modifications/flex/flex-parent.ts | 35 ++++++++++++ 11 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 packages/technical-features/ui-components/src/element/element-props.ts create mode 100644 packages/technical-features/ui-components/src/element/element.tsx create mode 100644 packages/technical-features/ui-components/src/element/elements.ts create mode 100644 packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.test.tsx create mode 100644 packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.ts create mode 100644 packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.test.tsx create mode 100644 packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.ts create mode 100644 packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.test.tsx create mode 100644 packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.ts diff --git a/packages/technical-features/ui-components/index.ts b/packages/technical-features/ui-components/index.ts index ad8efe538a..b60fb0485a 100644 --- a/packages/technical-features/ui-components/index.ts +++ b/packages/technical-features/ui-components/index.ts @@ -1 +1,3 @@ export { uiComponentsFeature } from "./src/feature"; + +export * from "./src/element/elements"; diff --git a/packages/technical-features/ui-components/package.json b/packages/technical-features/ui-components/package.json index 65b089a0f6..079163c475 100644 --- a/packages/technical-features/ui-components/package.json +++ b/packages/technical-features/ui-components/package.json @@ -30,12 +30,16 @@ "lint": "lens-lint", "lint:fix": "lens-lint --fix" }, + "dependencies": { + "classnames": "^2.3.2" + }, "peerDependencies": { "@k8slens/feature-core": "^6.5.0-alpha.0", "@ogre-tools/injectable": "^15.1.2", "@ogre-tools/injectable-extension-for-auto-registration": "^15.1.2", "@ogre-tools/fp": "^15.1.2", - "lodash": "^4.17.21" + "lodash": "^4.17.21", + "react": "^17" }, "devDependencies": { "@async-fn/jest": "^1.6.4", diff --git a/packages/technical-features/ui-components/src/element/element-props.ts b/packages/technical-features/ui-components/src/element/element-props.ts new file mode 100644 index 0000000000..8ede16933b --- /dev/null +++ b/packages/technical-features/ui-components/src/element/element-props.ts @@ -0,0 +1,9 @@ +import type React from "react"; + +export {}; + +declare global { + interface ElementProps { + children?: React.ReactNode; + } +} diff --git a/packages/technical-features/ui-components/src/element/element.tsx b/packages/technical-features/ui-components/src/element/element.tsx new file mode 100644 index 0000000000..152ea0eb92 --- /dev/null +++ b/packages/technical-features/ui-components/src/element/element.tsx @@ -0,0 +1,18 @@ +import React, { HTMLAttributes } from "react"; +import { pipeline } from "@ogre-tools/fp"; +import { flexParentModification } from "./prop-modifications/flex/flex-parent"; +import { classNameModification } from "./prop-modifications/class-names/class-names"; +import { vanillaClassNameAdapterModification } from "./prop-modifications/class-names/vanilla-class-name-adapter"; + +export const ElementFor = + >(TagName: React.ElementType) => + (props: React.DetailedHTMLProps & ElementProps) => { + const modifiedProps = pipeline( + props, + vanillaClassNameAdapterModification, + flexParentModification, + classNameModification, + ); + + return ; + }; diff --git a/packages/technical-features/ui-components/src/element/elements.ts b/packages/technical-features/ui-components/src/element/elements.ts new file mode 100644 index 0000000000..61f60a69f6 --- /dev/null +++ b/packages/technical-features/ui-components/src/element/elements.ts @@ -0,0 +1,5 @@ +import { ElementFor } from "./element"; +import type React from "react"; + +export const Div = ElementFor>("div"); +export const Span = ElementFor>("span"); diff --git a/packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.test.tsx b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.test.tsx new file mode 100644 index 0000000000..f37a69cdcd --- /dev/null +++ b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.test.tsx @@ -0,0 +1,56 @@ +import { Div } from "../../elements"; +import { render } from "@testing-library/react"; +import React from "react"; +import { discoverFor } from "@k8slens/react-testing-library-discovery"; + +describe("class-names", () => { + it("given complex class names, renders with class name", () => { + const rendered = render( +
, + ); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe( + "some-class-name some-present-class-name first-class-name-in-array second-class-name-in-array", + ); + }); + + it("given minimal class name, renders with class name", () => { + const rendered = render(
); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("some-class-name"); + }); + + it("given complex class names leading to class name not being present, renders without class name", () => { + const rendered = render( +
, + ); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered).not.toHaveAttribute('class'); + }); +}); diff --git a/packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.ts b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.ts new file mode 100644 index 0000000000..9fb6b131e6 --- /dev/null +++ b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/class-names.ts @@ -0,0 +1,18 @@ +import classnames from "classnames"; + +export type ClassName = classnames.Argument; + +declare global { + interface ElementProps { + _className?: ClassName; + } +} + +export const classNameModification = ({ _className, ...props }: T) => { + const classNameString = classnames(_className); + + return { + ...props, + ...(classNameString ? { className: classNameString } : {}), + }; +}; diff --git a/packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.test.tsx b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.test.tsx new file mode 100644 index 0000000000..fe98dcdb1d --- /dev/null +++ b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.test.tsx @@ -0,0 +1,42 @@ +import { Div } from "../../elements"; +import { render } from "@testing-library/react"; +import React from "react"; +import { discoverFor } from "@k8slens/react-testing-library-discovery"; + +describe("vanilla-class-name-adapter", () => { + it("given vanilla class name, has class name", () => { + const rendered = render(
); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("some-class-name"); + }); + + it("given custom class name, has class name", () => { + const rendered = render(
); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("some-class-name"); + }); + + it("given both vanilla and custom class names, has class names", () => { + const rendered = render( +
, + ); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("some-vanilla-class-name some-class-name"); + }); +}); diff --git a/packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.ts b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.ts new file mode 100644 index 0000000000..12cab83c2b --- /dev/null +++ b/packages/technical-features/ui-components/src/element/prop-modifications/class-names/vanilla-class-name-adapter.ts @@ -0,0 +1,10 @@ +import type { ClassName } from "./class-names"; + +export const vanillaClassNameAdapterModification = ({ + className, + _className, + ...props +}: T & { className?: string; _className?: ClassName }) => ({ + ...props, + _className: [className, _className], +}); diff --git a/packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.test.tsx b/packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.test.tsx new file mode 100644 index 0000000000..3117ecb571 --- /dev/null +++ b/packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.test.tsx @@ -0,0 +1,50 @@ +import { Div } from "../../elements"; +import { render } from "@testing-library/react"; +import React from "react"; +import { discoverFor } from "@k8slens/react-testing-library-discovery"; + +describe("flexParent", () => { + it("given flex parent without specific configuration, renders", () => { + const rendered = render(
); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("flex"); + }); + + it("given flex parent prop with false, renders", () => { + const rendered = render(
); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe(""); + }); + + it("given flex parent with aligning child vertically center, renders", () => { + const rendered = render( +
, + ); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("flex align-center"); + }); + + it("given flex parent with explicitly not aligning child vertically center, renders", () => { + const rendered = render( +
, + ); + + const discover = discoverFor(() => rendered); + + const { discovered } = discover.getSingleElement("some-element"); + + expect(discovered.className).toBe("flex"); + }); +}); diff --git a/packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.ts b/packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.ts new file mode 100644 index 0000000000..71ce53f7dc --- /dev/null +++ b/packages/technical-features/ui-components/src/element/prop-modifications/flex/flex-parent.ts @@ -0,0 +1,35 @@ +import type { ClassName } from "../class-names/class-names"; + +declare global { + interface ElementProps { + _className?: ClassName; + _flexParent?: { centeredVertically: boolean } | boolean; + } +} + +export const flexParentModification = ({ + _flexParent, + _className, + ...props +}: T) => { + if (!_flexParent) { + return { _className, ...props }; + } + + const centeredVertically = + typeof _flexParent === "boolean" ? false : _flexParent.centeredVertically; + + return { + ...props, + + _className: [ + "flex", + + { + "align-center": centeredVertically, + }, + + _className, + ], + }; +};