Merge pull request #839 from openSUSE/simplify-section-component

[web] Simplify code of Section component
agama-project · Nov 2, 2023 · 72cbbbd · 72cbbbd
2 parents 335066c + d687cfc
commit 72cbbbd
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 87 deletions.
diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx
@@ -26,54 +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
- * @param {number} [props.size=32] - the icon size
- *
- * @return {React.ReactElement}
- */
-const SectionIcon = ({ name, size = 32 }) => {
-  if (!name) return null;
-
-  return <Icon name={name} size={size} aria-hidden />;
-};
-
-/**
- * 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}</> : <Link to={path}>{text}</Link>;
-
-  return <h2 id={id}>{title}</h2>;
-};
-
-/**
- * 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 (
-    <div className="stack content">
-      {children}
-    </div>
-  );
-};
-
 /**
  * Renders children into an HTML section
  * @component
@@ -122,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}</> : <Link to={path}>{title}</Link>;
 
     return (
       <>
-        <SectionIcon name={loading ? "loading" : icon} />
-        <SectionTitle id={headerId} text={title} path={path} />
+        <Icon name={loading ? "loading" : icon} />
+        <h2 id={headerId}>{header}</h2>
       </>
     );
   };
@@ -140,12 +94,12 @@ export default function Section({
       aria-label={ariaLabel || undefined}
       aria-labelledby={ title && !ariaLabel ? headerId : undefined}
     >
-      <SectionHeader />
-      <SectionContent>
+      <Header />
+      <div className="stack content">
         {errors?.length > 0 &&
           <ValidationErrors errors={errors} title={`${title} errors`} />}
         {children}
-      </SectionContent>
+      </div>
     </section>
   );
 }
diff --git 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.
@@ -132,23 +129,31 @@ 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
  *
  * @example
  *   <Icon name="warning" size="16" />
  *
- * @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(`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
@@ -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(<Icon name="wifi" />);
-    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(<Icon name="wifi" />);
-    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(<Icon name="apsens" />);
-    expect(console.error).toHaveBeenCalledWith(
-      expect.stringContaining("apsens not found")
-    );
+  describe("mounted with a falsy value as name", () => {
+    it("outputs to console.error", () => {
+      plainRender(<Icon name="" />);
+      expect(console.error).toHaveBeenCalledWith(
+        expect.stringContaining("Rendering nothing")
+      );
+    });
+
+    it("renders nothing", () => {
+      const { container: contentWhenNotDefined } = plainRender(<Icon />);
+      expect(contentWhenNotDefined).toBeEmptyDOMElement();
+
+      const { container: contentWhenEmpty } = plainRender(<Icon name="" />);
+      expect(contentWhenEmpty).toBeEmptyDOMElement();
+
+      const { container: contentWhenFalse } = plainRender(<Icon name={false} />);
+      expect(contentWhenFalse).toBeEmptyDOMElement();
+
+      const { container: contentWhenNull } = plainRender(<Icon name={null} />);
+      expect(contentWhenNull).toBeEmptyDOMElement();
+    });
+  });
+
+  describe("mounted with a known name", () => {
+    it("renders an aria-hidden SVG element", async () => {
+      const { container } = plainRender(<Icon name="wifi" />);
+      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(<Icon name="wifi" />);
+      const svgElement = container.querySelector('svg');
+      expect(svgElement).toHaveAttribute("data-icon-name", "wifi");
+    });
   });
 
-  it("renders nothing", async () => {
-    const { container } = plainRender(<Icon name="apsens" />);
-    expect(container).toBeEmptyDOMElement();
+  describe("mounted with unknown name", () => {
+    it("outputs to console.error", () => {
+      plainRender(<Icon name="apsens" />);
+      expect(console.error).toHaveBeenCalledWith(
+        expect.stringContaining("'apsens' not found")
+      );
+    });
+
+    it("renders nothing", async () => {
+      const { container } = plainRender(<Icon name="apsens" />);
+      expect(container).toBeEmptyDOMElement();
+    });
   });
 });