Merge pull request #545 from openSUSE/contextual-actions-2nd-approach

[web] Change the location of page options

web/package/cockpit-agama.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Mon Apr 24 15:53:35 UTC 2023 - David Diaz <[email protected]>
+
+- Extract page options from the Sidebar to make them
+  more discoverable (gh#openSUSE/agama#545)
+
 -------------------------------------------------------------------
 Fri Apr 14 13:08:05 UTC 2023 - José Iván López González <[email protected]>
 

web/src/assets/styles/blocks.scss

@@ -1,7 +1,7 @@
 // Standard section
 // In the future we might need to use an specific CSS class for it if we start having different
 // section layouts.
-section {
+section:not([class^="pf-c"]) {
   display: grid;
   grid-template-columns: var(--icon-size-m) 1fr;
   grid-template-areas:
@@ -10,7 +10,7 @@ section {
   gap: var(--spacer-small);
 }
 
-section:not(:last-child) {
+section:not(:last-child, [class^="pf-c"]) {
   margin-block-end: var(--spacer-large);
 }
 
@@ -196,3 +196,25 @@ section > .content {
     --pf-c-progress--GridGap: var(--spacer-small);
   }
 }
+
+.page-options > button {
+  --pf-c-button--PaddingRight: 0
+}
+
+.page-options a {
+  font-size: 16px;
+  font-weight: var(--fw-bold);
+  text-decoration: none;
+
+  svg {
+    color: inherit;
+  }
+
+  &:hover {
+    color: var(--color-link-hover);
+
+    svg {
+      color: var(--color-link);
+    }
+  }
+}

web/src/components/core/PageOptions.jsx

@@ -19,64 +19,120 @@
  * find current contact information at www.suse.com.
  */
 
-// @ts-check
+import React, { useState } from 'react';
+import { Button, Dropdown, DropdownItem, DropdownGroup } from '@patternfly/react-core';
+import { Icon, PageOptions as PageOptionsSlot } from "~/components/layout";
 
-import React from "react";
-import { PageOptionsContent } from '~/components/layout';
+/**
+ * Internal component to build the {PageOptions} toggler
+ * @component
+ *
+ * @param {object} props
+ * @param {function} props.onClick
+ */
+const Toggler = ({ onClick }) => {
+  return (
+    <Button onClick={onClick} variant="plain">
+      <Icon name="expand_more" />
+    </Button>
+  );
+};
+
+/**
+ * A group of actions belonging to a {PageOptions} component
+ * @component
+ *
+ * Built on top of {@link https://www.patternfly.org/v4/components/dropdown/#dropdowngroup PF DropdownGroup}
+ *
+ * @see {PageOptions } examples.
+ *
+ * @param {object} props - PF DropdownItem props, See {@link https://www.patternfly.org/v4/components/dropdowngroup}
+ */
+const Group = ({ children, ...props }) => {
+  return (
+    <DropdownGroup {...props}>
+      {children}
+    </DropdownGroup>
+  );
+};
 
 /**
- * Wrapper for teleported page options that bubbles onClick
- * event to the slot element.
- *
- * Needed to "dispatch" the onClick events bind to any
- * parent on the DOM tree for "teleported nodes", bypassing the
- * default React Portal behavior of bubbling events up through the
- * React tree only.
- *
- * @example <caption>Simple usage</caption>
- *   <PageOptions title="Storage options">
- *     <Link to="/storage/iscsi">Configure iSCSI devices</Link>
- *     <Button
- *       onClick={showStorageHwInfo}
- *       data-keep-sidebar-open
+ * An action belonging to a {PageOptions} component
+ * @component
+ *
+ * Built on top of {@link https://www.patternfly.org/v4/components/dropdown/#dropdownitem PF DropdownItem}
+ *
+ * @see {PageOptions } examples.
+ *
+ * @param {object} props - PF DropdownItem props, See {@link https://www.patternfly.org/v4/components/dropdownitem}
+ */
+const Item = ({ children, ...props }) => {
+  return (
+    <DropdownItem {...props}>
+      {children}
+    </DropdownItem>
+  );
+};
+
+/**
+ * Component for rendering actions related to the current page
+ * @component
+ *
+ * It consist in a {@link https://www.patternfly.org/v4/components/dropdown
+ * PatternFly Dropdown} "teleported" to the header, close to the
+ * action for opening the Sidebar
+ *
+ * @example <caption>Usage example</caption>
+ *   <PageOptions>
+ *     <PageOptions.Item
+ *       key="reprobe-link"
+ *       description="Run a storage device detection"
  *     >
- *       Show Storage Hardware info
- *     </Button>
+ *
+ *       Reprobe
+ *     </PageOptions.Item>
+ *     <PageOptions.Group key="configuration-links" label="Configure">
+ *       <PageOptions.Item
+ *         key="dasd-link"
+ *         href={href}
+ *         description="Manage and format"
+ *       >
+ *         DASD
+ *       </PageOptions.Item>
+ *       <PageOptions.Item
+ *         key="iscsi-link"
+ *         href={href}
+ *         description="Connect to iSCSI targets"
+ *        >
+ *         iSCSI
+ *       </PageOptions.Item>
+ *     </PageOptions.Group>
  *   </PageOptions>
  *
  * @param {object} props
- * @param {string} [props.title="Page options"] - a title for the group
- * @param {string} [props.className="flex-stack"] - CSS class for the wrapper div
- * @param {React.ReactElement} props.children - the teleported content
+ * @param {Group|Item|Array<Group|Item>} props.children
  */
-export default function PageOptions({
-  title = "Page options",
-  className = "flex-stack",
-  children
-}) {
-  const forwardEvents = (target) => {
-    return (
-      <div
-        className={className}
-        onClick={ e => {
-          // Using a CustomEvent because the originalTarget is needed to check the dataset.
-          // See Sidebar.jsx for better understanding
-          const customEvent = new CustomEvent(
-            e.type,
-            { ...e, detail: { originalTarget: e.target } }
-          );
-          target.dispatchEvent(customEvent);
-        }}
-      >
-        <h3>{title}</h3>
-        {children}
-      </div>
-    );
-  };
+const PageOptions = ({ children }) => {
+  const [isOpen, setIsOpen] = useState(false);
+  const onToggle = () => setIsOpen(!isOpen);
+  const onSelect = () => setIsOpen(false);
 
   return (
-    <PageOptionsContent>
-      { (target) => forwardEvents(target) }
-    </PageOptionsContent>
+    <PageOptionsSlot>
+      <Dropdown
+        isOpen={isOpen}
+        toggle={<Toggler onClick={onToggle} />}
+        onSelect={onSelect}
+        dropdownItems={Array(children)}
+        position="right"
+        className="page-options"
+        isGrouped
+      />
+    </PageOptionsSlot>
   );
-}
+};
+
+PageOptions.Group = Group;
+PageOptions.Item = Item;
+
+export default PageOptions;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2023] SUSE LLC
+ * Copyright (c) [2022-2023] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -21,51 +21,56 @@
 
 import React from "react";
 import { screen } from "@testing-library/react";
-import { plainRender } from "~/test-utils";
-import { PageOptionsSlot } from "~/components/layout";
+import { plainRender, mockLayout } from "~/test-utils";
 import { PageOptions } from "~/components/core";
 
-describe("PageOptions", () => {
-  it("renders given title", () => {
-    plainRender(
-      <>
-        <PageOptionsSlot />
-        <PageOptions title="Awesome options">
-          The page options content
-        </PageOptions>
-      </>
-    );
+jest.mock("~/components/layout/Layout", () => mockLayout());
 
-    screen.getByText("Awesome options");
-  });
+it("renders the component initially closed", async () => {
+  plainRender(
+    <PageOptions>
+      <PageOptions.Item>A dummy action</PageOptions.Item>
+    </PageOptions>
+  );
 
-  it("renders given children", () => {
-    plainRender(
-      <>
-        <PageOptionsSlot />
-        <PageOptions title="Awesome options">
-          The page options content
-        </PageOptions>
-      </>
-    );
+  expect(screen.queryByRole("menuitem", { name: "A dummy action" })).toBeNull();
+});
+
+it("show and hide the component content on user request", async () => {
+  const { user } = plainRender(
+    <PageOptions>
+      <PageOptions.Item><>A dummy action</></PageOptions.Item>
+    </PageOptions>
+  );
+
+  const toggler = screen.getByRole("button");
+
+  expect(screen.queryByRole("menuitem", { name: "A dummy action" })).toBeNull();
 
-    screen.getByText("The page options content");
-  });
+  await user.click(toggler);
+
+  screen.getByRole("menuitem", { name: "A dummy action" });
+
+  await user.click(toggler);
+
+  expect(screen.queryByRole("menuitem", { name: "A dummy action" })).toBeNull();
+});
 
-  it("dispatches onClick events to the target", async () => {
-    const onClickHandler = jest.fn();
-    const { user } = plainRender(
-      <>
-        <PageOptionsSlot onClick={onClickHandler} />
-        <PageOptions title="Awesome options">
-          <button>Click tester</button>
-        </PageOptions>
-      </>
-    );
+it("hide the component content when the user clicks on one of its actions", async () => {
+  const { user } = plainRender(
+    <PageOptions>
+      <PageOptions.Group label="Refresh">
+        <PageOptions.Item><>Section</></PageOptions.Item>
+        <PageOptions.Item><>Page</></PageOptions.Item>
+      </PageOptions.Group>
+      <PageOptions.Item><>Exit</></PageOptions.Item>
+    </PageOptions>
+  );
 
-    const button = screen.getByRole("button", { name: "Click tester" });
-    await user.click(button);
+  const toggler = screen.getByRole("button");
+  await user.click(toggler);
+  const action = screen.getByRole("menuitem", { name: "Section" });
+  await user.click(action);
 
-    expect(onClickHandler).toHaveBeenCalled();
-  });
+  expect(screen.queryByRole("menuitem", { name: "A dummy action" })).toBeNull();
 });

web/src/components/core/Sidebar.jsx

@@ -21,10 +21,7 @@
 
 import React, { useEffect, useRef, useState } from "react";
 import { Button, Text } from "@patternfly/react-core";
-import { Icon, PageActions } from "~/components/layout";
-
-// FIXME: look for a better way to allow opening the Sidebar from outside
-let openButtonRef = {};
+import { Icon, AppActions } from "~/components/layout";
 
 /**
  * Agama sidebar navigation
@@ -33,9 +30,8 @@ let openButtonRef = {};
  * @param {object} props
  * @param {React.ReactElement} props.children
  */
-const Sidebar = ({ children }) => {
+export default function Sidebar ({ children }) {
   const [isOpen, setIsOpen] = useState(false);
-  openButtonRef = useRef(null);
   const closeButtonRef = useRef(null);
 
   const open = () => setIsOpen(true);
@@ -89,9 +85,8 @@ const Sidebar = ({ children }) => {
 
   return (
     <>
-      <PageActions>
+      <AppActions>
         <button
-          ref={openButtonRef}
           onClick={open}
           className="plain-control"
           aria-label="Show navigation and other options"
@@ -100,7 +95,7 @@ const Sidebar = ({ children }) => {
         >
           <Icon name="menu" />
         </button>
-      </PageActions>
+      </AppActions>
 
       <nav
         id="navigation-and-options"
@@ -134,23 +129,4 @@ const Sidebar = ({ children }) => {
       </nav>
     </>
   );
-};
-
-/**
- * Button for opening the sidebar
- * @component
- *
- * @param {object} props
- * @param {onClickFn} [props.onClick] - On click callback
- * @param {React.ReactElement} props.children
- */
-Sidebar.OpenButton = ({ onClick: onClickProp, children }) => {
-  const onClick = () => {
-    if (onClickProp !== undefined) onClickProp();
-    openButtonRef.current.click();
-  };
-
-  return <Button variant="link" isInline onClick={onClick}>{children}</Button>;
-};
-
-export default Sidebar;
+}