From 1215f779590faeb790bfee2262abbab6f1e6a842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 31 Oct 2023 19:37:33 +0000 Subject: [PATCH 1/4] [web] Drop not needed props from internal Section component Section#SectionIcon#size was no really in use, just falling back to its default value. Section#SectionIcon#aria-hidden attribute is not needed since the core/Icon component already sets it. --- web/src/components/core/Section.jsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx index 1e16d6b486..9b165b8643 100644 --- a/web/src/components/core/Section.jsx +++ b/web/src/components/core/Section.jsx @@ -31,14 +31,13 @@ import { ValidationErrors } from "~/components/core"; * * @param {object} props * @param {string} [props.name] - the name of the icon - * @param {number} [props.size=32] - the icon size * * @return {React.ReactElement} */ -const SectionIcon = ({ name, size = 32 }) => { +const SectionIcon = ({ name }) => { if (!name) return null; - return ; + return ; }; /** From 0eb901845b921e84dcd100a900549411feb50c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 Nov 2023 08:14:07 +0000 Subject: [PATCH 2/4] [web] Allow Icon renders nothing if name not given --- web/src/components/layout/Icon.jsx | 18 +++++-- web/src/components/layout/Icon.test.jsx | 71 +++++++++++++++++-------- 2 files changed, 61 insertions(+), 28 deletions(-) diff --git a/web/src/components/layout/Icon.jsx b/web/src/components/layout/Icon.jsx index a516f65361..ab3d6fb4da 100644 --- a/web/src/components/layout/Icon.jsx +++ b/web/src/components/layout/Icon.jsx @@ -132,6 +132,9 @@ const icons = { * * If exists, it renders requested icon with given size. * + * @note: if either, name prop has a falsy value or requested icon is not found, + * it will outputs a message to the console.error and renders nothing. + * * @todo: import icons dynamically if the list grows too much. See * - https://stackoverflow.com/a/61472427 * - https://ryanhutzley.medium.com/dynamic-svg-imports-in-create-react-app-d6d411f6d6c6 @@ -139,14 +142,19 @@ const icons = { * @example * * - * @param {object} props - component props - * @param {string} props.name - desired icon - * @param {string} [props.className=""] - CSS classes - * @param {string|number} [props.size=32] - the icon width and height - * @param {object} [props.otherProps] other props sent to SVG icon + * @param {object} props - Component props + * @param {string} props.name - Name of the desired icon. + * @param {string} [props.className=""] - CSS classes. + * @param {string|number} [props.size=32] - Size used for both, width and height. + * @param {object} [props.otherProps] Other props sent to SVG icon. * */ export default function Icon({ name, className = "", size = 32, ...otherProps }) { + if (!name) { + console.error(sprintf(_("Icon called without name. `%s` given instead. Rendering nothing."), name)); + return null; + } + if (!icons[name]) { console.error(sprintf(_("Icon %s not found!"), name)); return null; diff --git a/web/src/components/layout/Icon.test.jsx b/web/src/components/layout/Icon.test.jsx index a2a154d22f..8f8339214f 100644 --- a/web/src/components/layout/Icon.test.jsx +++ b/web/src/components/layout/Icon.test.jsx @@ -23,21 +23,7 @@ import React from "react"; import { plainRender } from "~/test-utils"; import { Icon } from "~/components/layout"; -describe("when given a known name", () => { - it("renders an aria-hidden SVG element", async () => { - const { container } = plainRender(); - const svgElement = container.querySelector('svg'); - expect(svgElement).toHaveAttribute("aria-hidden", "true"); - }); - - it("includes the icon name as a data attribute of the SVG", async () => { - const { container } = plainRender(); - const svgElement = container.querySelector('svg'); - expect(svgElement).toHaveAttribute("data-icon-name", "wifi"); - }); -}); - -describe("when given an unknown name", () => { +describe("Icon", () => { beforeAll(() => { jest.spyOn(console, "error").mockImplementation(); }); @@ -46,15 +32,54 @@ describe("when given an unknown name", () => { console.error.mockRestore(); }); - it("outputs to console.error", () => { - plainRender(); - expect(console.error).toHaveBeenCalledWith( - expect.stringContaining("apsens not found") - ); + describe("mounted with a falsy value as name", () => { + it("outputs to console.error", () => { + plainRender(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("Rendering nothing") + ); + }); + + it("renders nothing", () => { + const { container: contentWhenNotDefined } = plainRender(); + expect(contentWhenNotDefined).toBeEmptyDOMElement(); + + const { container: contentWhenEmpty } = plainRender(); + expect(contentWhenEmpty).toBeEmptyDOMElement(); + + const { container: contentWhenFalse } = plainRender(); + expect(contentWhenFalse).toBeEmptyDOMElement(); + + const { container: contentWhenNull } = plainRender(); + expect(contentWhenNull).toBeEmptyDOMElement(); + }); + }); + + describe("mounted with a known name", () => { + it("renders an aria-hidden SVG element", async () => { + const { container } = plainRender(); + const svgElement = container.querySelector('svg'); + expect(svgElement).toHaveAttribute("aria-hidden", "true"); + }); + + it("includes the icon name as a data attribute of the SVG", async () => { + const { container } = plainRender(); + const svgElement = container.querySelector('svg'); + expect(svgElement).toHaveAttribute("data-icon-name", "wifi"); + }); }); - it("renders nothing", async () => { - const { container } = plainRender(); - expect(container).toBeEmptyDOMElement(); + describe("mounted with unknown name", () => { + it("outputs to console.error", () => { + plainRender(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("apsens not found") + ); + }); + + it("renders nothing", async () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); + }); }); }); From 116c8b476d306ca83331805db2dfa2d25e9c30ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 Nov 2023 08:23:31 +0000 Subject: [PATCH 3/4] [web] Simplify Section component Using less internal components. --- web/src/components/core/Section.jsx | 63 +++++------------------------ 1 file changed, 9 insertions(+), 54 deletions(-) diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx index 9b165b8643..99a219eb6c 100644 --- a/web/src/components/core/Section.jsx +++ b/web/src/components/core/Section.jsx @@ -26,53 +26,6 @@ import { Link } from "react-router-dom"; import { Icon } from '~/components/layout'; import { ValidationErrors } from "~/components/core"; -/** - * Internal component for rendering the section icon - * - * @param {object} props - * @param {string} [props.name] - the name of the icon - * - * @return {React.ReactElement} - */ -const SectionIcon = ({ name }) => { - if (!name) return null; - - return ; -}; - -/** - * Internal component for rendering the section title - * - * @param {object} props - * @param {string} props.id - the id for the header. - * @param {string} props.text - the title for the section. - * @param {string} props.path - the path where the section links to. - * - * @return {JSX.Element} - */ -const SectionTitle = ({ id, text, path }) => { - if (!text?.trim()) return null; - - const title = !path?.trim() ? <>{text} : {text}; - - return

{title}

; -}; - -/** - * Internal component for wrapping and rendering the section content - * - * @param {object} props - * @param {React.ReactElement|React.ReactElement[]} props.children - the content to be wrapped - * @return {JSX.Element} - */ -const SectionContent = ({ children }) => { - return ( -
- {children} -
- ); -}; - /** * Renders children into an HTML section * @component @@ -121,13 +74,15 @@ export default function Section({ console.error("The Section component must have either, a 'title' or an 'aria-label'"); } - const SectionHeader = () => { - if (!title) return; + const Header = () => { + if (!title?.trim()) return; + + const header = !path?.trim() ? <>{title} : {title}; return ( <> - - + +

{header}

); }; @@ -139,12 +94,12 @@ export default function Section({ aria-label={ariaLabel || undefined} aria-labelledby={ title && !ariaLabel ? headerId : undefined} > - - +
+
{errors?.length > 0 && } {children} - +
); } From d687cfcbb71ed247e865e15cdb3a5bad219c7293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 Nov 2023 09:57:46 +0000 Subject: [PATCH 4/4] [web] Do not translate console.error --- web/src/components/layout/Icon.jsx | 7 ++----- web/src/components/layout/Icon.test.jsx | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/web/src/components/layout/Icon.jsx b/web/src/components/layout/Icon.jsx index ab3d6fb4da..f3866cccf3 100644 --- a/web/src/components/layout/Icon.jsx +++ b/web/src/components/layout/Icon.jsx @@ -20,9 +20,6 @@ */ import React from 'react'; -import { sprintf } from "sprintf-js"; - -import { _ } from "~/i18n"; // NOTE: "@icons" is an alias to use a shorter path to real @material-symbols // icons location. Check the tsconfig.json file to see its value. @@ -151,12 +148,12 @@ const icons = { */ export default function Icon({ name, className = "", size = 32, ...otherProps }) { if (!name) { - console.error(sprintf(_("Icon called without name. `%s` given instead. Rendering nothing."), name)); + console.error(`Icon called without name. '${name}' given instead. Rendering nothing.`); return null; } if (!icons[name]) { - console.error(sprintf(_("Icon %s not found!"), name)); + console.error(`Icon '${name}' not found!`); return null; } diff --git a/web/src/components/layout/Icon.test.jsx b/web/src/components/layout/Icon.test.jsx index 8f8339214f..490598a13d 100644 --- a/web/src/components/layout/Icon.test.jsx +++ b/web/src/components/layout/Icon.test.jsx @@ -73,7 +73,7 @@ describe("Icon", () => { it("outputs to console.error", () => { plainRender(); expect(console.error).toHaveBeenCalledWith( - expect.stringContaining("apsens not found") + expect.stringContaining("'apsens' not found") ); });