[web] Allow defining Sidebar contents via children prop

That way, the use of React.useMemo to avoid re-rendering them in each
visibility change is not longer needed. It simplifies things a lot.

web/src/components/core/Sidebar.jsx

@@ -20,51 +20,12 @@
  */
 
 import React, { useEffect, useRef, useState } from "react";
-import { Icon, PageActions, PageOptionsSlot } from "~/components/layout";
-import { About, Disclosure, LogsButton, ShowLogButton, ShowTerminalButton } from "~/components/core";
-
-/**
- * Internal and memoized component for rendering links only once
- *
- * The reason of this is to avoid re-rendering those links each time
- * the sidebar visibility is changed. It does not make sense to re-trigger
- * their logic (which might be complicated or not) just because the
- * "expanded"/"visible" state of the Sidebar has changed.
- *
- * The issue can be fixed in other ways, but there is a reason for discarded
- * them at this moment. Namely,
- *
- *   - Use a stateless Sidebar, handling the visibility with pure CSS
- *
- *     That would be great, but it's (almost?) not possible to make it
- *     completely accessible using just CSS.
- *
- *   - Move these children outside, placing them in the same location the
- *     <Sidebar /> is used.
- *
- *     It will not work when using the function as child component pattern,
- *     which is the case of the PageOptions content, which we will use for
- *     teleporting a Page options to the Sidebar by now.
- *
- * To know more about how children and parent re-renders behavior, have a look to
- * https://www.developerway.com/posts/react-elements-children-parents
- */
-const FixedLinks = React.memo(() => (
-  <div className="flex-stack">
-    <h3>Other options</h3>
-    <Disclosure label="Diagnostic tools" data-keep-sidebar-open>
-      <ShowLogButton />
-      <LogsButton data-keep-sidebar-open="true" />
-      <ShowTerminalButton />
-    </Disclosure>
-    <About />
-  </div>
-), () => true);
+import { Icon, PageActions } from "~/components/layout";
 
 /**
  * D-Installer sidebar navigation
  */
-export default function Sidebar() {
+export default function Sidebar({ children }) {
   const [isOpen, setIsOpen] = useState(false);
   const closeButtonRef = useRef(null);
 
@@ -125,8 +86,7 @@ export default function Sidebar() {
         </header>
 
         <div className="flex-stack" onClick={onClick}>
-          <PageOptionsSlot />
-          <FixedLinks />
+          { children }
         </div>
 
         <footer className="split" data-state="reversed">

web/src/components/core/Sidebar.test.jsx

@@ -22,12 +22,17 @@
 import React from "react";
 import { screen, within } from "@testing-library/react";
 import { plainRender, mockComponent, mockLayout } from "~/test-utils";
-import { Sidebar } from "~/components/core";
+import { PageOptions, Sidebar } from "~/components/core";
 
 jest.mock("~/components/layout/Layout", () => mockLayout());
-jest.mock("~/components/core/About", () => mockComponent(<a href="#">About link mock</a>));
-jest.mock("~/components/core/LogsButton", () => mockComponent(<button data-keep-sidebar-open="true">Download logs mock</button>));
-jest.mock("~/components/core/ShowLogButton", () => mockComponent(<button href="#">Show logs mock</button>));
+jest.mock("~/components/core/PageOptions", () => mockComponent(
+  <>
+    <a href="#">Goes somewhere</a>
+    <a href="#" data-keep-sidebar-open="true">Keep it open!</a>
+    <button>Do something</button>
+    <button data-keep-sidebar-open="true">Keep it open!</button>
+  </>
+));
 
 it("renders the sidebar initially hidden", async () => {
   plainRender(<Sidebar />);
@@ -36,7 +41,7 @@ it("renders the sidebar initially hidden", async () => {
 });
 
 it("renders a link for displaying the sidebar", async () => {
-  const { user } = plainRender(<Sidebar />);
+  const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
 
   const link = await screen.findByLabelText(/Show/i);
   const nav = await screen.findByRole("navigation", { name: /options/i });
@@ -47,7 +52,7 @@ it("renders a link for displaying the sidebar", async () => {
 });
 
 it("renders a link for hiding the sidebar", async () => {
-  const { user } = plainRender(<Sidebar />);
+  const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
 
   const openLink = await screen.findByLabelText(/Show/i);
   const closeLink = await screen.findByLabelText(/Hide/i);
@@ -61,7 +66,7 @@ it("renders a link for hiding the sidebar", async () => {
 });
 
 it("moves the focus to the close action after opening it", async () => {
-  const { user } = plainRender(<Sidebar />);
+  const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
 
   const openLink = await screen.findByLabelText(/Show/i);
   const closeLink = await screen.findByLabelText(/Hide/i);
@@ -73,7 +78,7 @@ it("moves the focus to the close action after opening it", async () => {
 
 describe("onClick bubbling", () => {
   it("hides the sidebar only if the user clicked on a link or button w/o keepSidebarOpen attribute", async () => {
-    const { user } = plainRender(<Sidebar />);
+    const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
     const openLink = screen.getByLabelText(/Show/i);
     await user.click(openLink);
     const nav = screen.getByRole("navigation", { name: /options/i });
@@ -84,22 +89,27 @@ describe("onClick bubbling", () => {
     expect(nav).toHaveAttribute("data-state", "visible");
 
     // user clicks on a button set for keeping the sidebar open
-    const downloadButton = within(nav).getByRole("button", { name: "Download logs mock" });
-    await user.click(downloadButton);
+    const keepOpenButton = within(nav).getByRole("button", { name: "Keep it open!" });
+    await user.click(keepOpenButton);
     expect(nav).toHaveAttribute("data-state", "visible");
 
     // user clicks a button NOT set for keeping the sidebar open
-    const showLogsButton = within(nav).getByRole("button", { name: "Show logs mock" });
-    await user.click(showLogsButton);
+    const button = within(nav).getByRole("button", { name: "Do something" });
+    await user.click(button);
     expect(nav).toHaveAttribute("data-state", "hidden");
 
     // open it again
     await user.click(openLink);
     expect(nav).toHaveAttribute("data-state", "visible");
 
-    // user clicks on a button
-    const aboutLink = within(nav).getByRole("link", { name: "About link mock" });
-    await user.click(aboutLink);
+    // user clicks on link set for keeping the sidebar open
+    const keepOpenLink = within(nav).getByRole("link", { name: "Keep it open!" });
+    await user.click(keepOpenLink);
+    expect(nav).toHaveAttribute("data-state", "visible");
+
+    // user clicks on link NOT set for keeping the sidebar open
+    const link = within(nav).getByRole("link", { name: "Goes somewhere" });
+    await user.click(link);
     expect(nav).toHaveAttribute("data-state", "hidden");
   });
 });

web/src/components/layout/Layout.jsx

@@ -23,7 +23,7 @@ import React from "react";
 
 import logoUrl from "~/assets/suse-horizontal-logo.svg";
 import { createTeleporter } from "react-teleporter";
-import { Sidebar } from "~/components/core";
+import { About, Disclosure, LogsButton, Sidebar, ShowLogButton, ShowTerminalButton } from "~/components/core";
 
 const PageTitle = createTeleporter();
 const PageOptions = createTeleporter();
@@ -79,7 +79,19 @@ function Layout({ children }) {
         <HeaderActions.Target as="span" />
       </header>
 
-      <Sidebar />
+      <Sidebar>
+        <>
+          <PageOptions.Target />
+
+          <h3>Other options</h3>
+          <Disclosure label="Diagnostic tools" data-keep-sidebar-open>
+            <ShowLogButton />
+            <LogsButton data-keep-sidebar-open="true" />
+            <ShowTerminalButton />
+          </Disclosure>
+          <About />
+        </>
+      </Sidebar>
 
       <main className="stack">
         {children}

web/src/test-utils.js

@@ -147,7 +147,6 @@ const mockLayout = () => ({
   PageActions: ({ children }) => children,
   MainActions: ({ children }) => children,
   AdditionalInfo: ({ children }) => children,
-  PageOptionsSlot: ({ children }) => children,
   PageOptionsContent: ({ children }) => children,
 });