Merge pull request #462 from yast/sidebar-improvements

[web] Sidebar improvements
agama-project · Mar 14, 2023 · aaa58ee · aaa58ee
2 parents fc1a850 + 205d649
commit aaa58ee
Show file tree

Hide file tree

Showing 41 changed files with 636 additions and 488 deletions.
diff --git a/web/package/cockpit-d-installer.changes b/web/package/cockpit-d-installer.changes
@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Mon Mar 13 15:51:08 UTC 2023 - David Diaz <[email protected]>
+
+- Sidebar improvements (gh#yast/d-installer#462)
+  * Allow adding actions from a page.
+  * Remove network information.
+  * Use underlined links and darker green color for improving contrast.
+  * Start using a disclosure widget for grouping related actions.
+
 -------------------------------------------------------------------
 Fri Mar  3 15:00:05 UTC 2023 - David Diaz <[email protected]>
 

diff --git a/web/src/App.test.jsx b/web/src/App.test.jsx
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2022] SUSE LLC
+ * Copyright (c) [2022-2023] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -28,11 +28,6 @@ import { STARTUP, CONFIG, INSTALL } from "~/client/phase";
 import { IDLE, BUSY } from "~/client/status";
 
 jest.mock("~/client");
-
-jest.mock('react-router-dom', () => ({
-  Outlet: mockComponent("Content"),
-}));
-
 jest.mock("~/components/layout/Layout", () => mockLayout());
 
 // Mock some components,
@@ -147,7 +142,7 @@ describe("App", () => {
 
     it("renders the application content", async () => {
       installerRender(<App />);
-      await screen.findByText("Content");
+      await screen.findByText(/Outlet Content/);
     });
   });
 
@@ -169,7 +164,7 @@ describe("App", () => {
 
     it("renders the Installation component on the INSTALL phase", async () => {
       installerRender(<App />);
-      await screen.findByText("Content");
+      await screen.findByText(/Outlet Content/);
       changePhaseTo(INSTALL);
       await screen.findByText("Installation Mock");
     });
@@ -183,7 +178,7 @@ describe("App", () => {
 
     it("renders the application's content", async () => {
       installerRender(<App />);
-      await screen.findByText("Content");
+      await screen.findByText(/Outlet Content/);
     });
   });
 });
diff --git a/web/src/Main.test.jsx b/web/src/Main.test.jsx
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2022] SUSE LLC
+ * Copyright (c) [2022-2023] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -21,18 +21,16 @@
 
 import React from "react";
 import { screen } from "@testing-library/react";
-import { installerRender, mockComponent } from "~/test-utils";
+import { plainRender, mockComponent } from "~/test-utils";
 
 import Main from "~/Main";
 
 jest.mock("~/components/questions/Questions", () => mockComponent("Questions Mock"));
-jest.mock('react-router-dom', () => ({
-  Outlet: mockComponent("Content"),
-}));
 
 it("renders the Questions component and the content", async () => {
-  installerRender(<Main />);
+  plainRender(<Main />);
 
   await screen.findByText("Questions Mock");
-  await screen.findByText("Content");
+  // react-router-dom Outlet is mocked. See test-utils for more details
+  await screen.findByText("Outlet Content");
 });
diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss
@@ -32,7 +32,7 @@ section > h2 {
     transition: all 0.15s ease-in-out;
 
     &:hover {
-      color: var(--color-primary);
+      color: var(--color-link-hover);
     }
   }
 }
@@ -72,15 +72,15 @@ section > .content {
 }
 
 .sidebar {
-  --color-background-primary: var(--color-primary);
+  --color-background-primary: var(--color-primary-lighter);
   --wrapper-background: var(--color-gray-light);
 
   position: absolute;
   padding: 0;
   right: 0;
   z-index: 1;
   inline-size: 70%;
-  box-shadow: 0 0 20px 10px var(--color-primary-darkest);
+  box-shadow: 0 0 20px 10px var(--color-primary);
 }
 
 .sidebar header {
@@ -91,6 +91,43 @@ section > .content {
   border-top: 1px solid var(--color-gray);
 }
 
+.sidebar > div {
+  margin-inline-start: var(--pf-global--spacer--md);
+}
+
+.sidebar a, .sidebar button {
+  font-size: 16px;
+  font-weight: var(--fw-bold);
+  text-decoration: underline;
+  text-underline-offset: 2px;
+  padding-block: 0;
+
+  &:hover {
+    color: var(--color-link-hover);
+    text-decoration: underline;
+
+    svg {
+      color: var(--color-link);
+    }
+  }
+
+  svg {
+    color: var(--color-link);
+    vertical-align: text-bottom;
+    margin-block-end: -2px;
+  }
+}
+
+.sidebar a {
+  margin-inline-start: var(--pf-global--spacer--md);
+
+  // Keep links and buttons labels aligned by adding the same margin than
+  // .pf-c-button__icon.pf-m-start
+  svg {
+    margin-inline-end: var(--pf-global--spacer--xs);
+  }
+}
+
 // Remove not wanted PatternFly padding left on a loading link
 .sidebar button.pf-m-progress {
   --pf-c-button--m-progress--PaddingLeft: var(--pf-global--spacer--md);
@@ -110,6 +147,37 @@ section > .content {
   transition: all 0.2s ease-in-out;
 }
 
+.disclosure > button {
+  margin-inline-start: var(--pf-global--spacer--md);
+  display: inline-flex;
+  align-items: center;
+  // Keep links and buttons labels aligned by adding the same margin than
+  // .pf-c-button__icon.pf-m-start
+  svg {
+    margin-inline-end: var(--pf-global--spacer--xs);
+    transition: transform 0.2s ease-in-out;
+  }
+
+  &[aria-expanded="true"] {
+    svg {
+      transform: rotate(90deg);
+    }
+  }
+
+  &[aria-expanded="false"] + div {
+    display: none;
+    visibility: hidden;
+  }
+}
+
+.disclosure > div {
+  margin-inline-start: calc(
+    var(--pf-global--spacer--md) + 12px // half of the icon size;
+  );
+  border-inline-start: 1px solid var(--color-primary-lighter);
+  padding-block: var(--spacer-small);
+}
+
 // raw file content with formatting similar to <pre>
 .filecontent {
   font-family: var(--ff-code);

diff --git a/web/src/assets/styles/global.scss b/web/src/assets/styles/global.scss
@@ -21,6 +21,21 @@ a {
   color: currentcolor;
 }
 
+a,
+// TODO: make it better, using PatternFly custom properties for overriding it
+button.pf-m-plain,
+button.pf-m-link {
+  text-decoration: underline;
+  text-decoration-thickness: 0.1em;
+  text-underline-offset: 0.2em;
+  transition: all 0.15s ease-in-out;
+
+  &:hover {
+    color: var(--color-link-hover);
+    text-decoration: underline;
+  }
+}
+
 fieldset {
   padding: var(--fs-base);
   border: 0;

diff --git a/web/src/assets/styles/patternfly-overrides.scss b/web/src/assets/styles/patternfly-overrides.scss
@@ -56,11 +56,9 @@
   --pf-c-button--m-plain--hover--Color: var(--color-button-plain-link-hover);
 }
 
-// Adds a tiny "padding" when focusing a primary or secondary action to avoid
-// https://github.com/yast/d-installer/issues/115#issuecomment-1087375598
-.pf-c-button.pf-m-primary:focus-visible,
-.pf-c-button.pf-m-secondary:focus-visible {
-  box-shadow: 0 0 0 1px white, 0 0 0 2px var(--focus-color);
+.pf-c-button.pf-m-secondary {
+  --pf-c-button--m-secondary--hover--after--BorderColor: var(--color-link-hover);
+  --pf-c-button--m-secondary--hover--Color: var(--color-link-hover);
 }
 
 // SVG icons does not obey font-size

diff --git a/web/src/assets/styles/utilities.scss b/web/src/assets/styles/utilities.scss
@@ -108,6 +108,14 @@
   border: none;
 }
 
+.plain-button {
+  border: none;
+  background: none;
+  color: inherit;
+  font: inherit;
+  padding: 0;
+}
+
 .tallest {
   /** block-size fallbacks **/
   height: 95dvh;

diff --git a/web/src/assets/styles/variables.scss b/web/src/assets/styles/variables.scss
@@ -27,26 +27,26 @@
   --wrapper-padding: var(--spacer-normal);
   --wrapper-background: white;
 
-  --color-primary: #30ba78;
-  --color-primary-darkest: #0c322c;
+  --color-primary: #0c322c;
+  --color-primary-lighter: #30ba78;
   --color-gray-light: #fcfcfc;
   --color-gray: #f2f2f2;
   --color-gray-dark: #efefef; // Fog
   --color-gray-darker: #999;
 
-  --color-link: #30ba78;
-  --color-link-hover: #0c322c;
+  --color-link: #0c322c;
+  --color-link-hover: #30ba78;
 
-  --color-button-primary: #30ba78;
-  --color-button-primary-hover: #0c322c;
+  --color-button-primary: var(--color-link);
+  --color-button-primary-hover: var(--color-link-hover);
 
-  --color-button-plain-link: #30ba78;
-  --color-button-plain-link-hover: #0c322c;
+  --color-button-plain-link: var(--color-link);
+  --color-button-plain-link-hover: var(--color-link-hover);
 
-  --color-background-primary: var(--color-primary-darkest);
+  --color-background-primary: var(--color-primary);
   --color-background-secondary: var(--color-gray-dark);
 
-  --color-text-primary: var(--color-primary-darkest);
+  --color-text-primary: var(--color-primary);
   --color-text-secondary: var(--color-gray-dark);
 
   --color-success: #30ba78;

diff --git a/web/src/components/core/About.jsx b/web/src/components/core/About.jsx
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2022] SUSE LLC
+ * Copyright (c) [2022-2023] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -20,19 +20,14 @@
  */
 
 import React, { useState } from "react";
-import { noop } from "~/utils";
 import { Button, Text } from "@patternfly/react-core";
 import { Icon } from "~/components/layout";
 import { Popup } from "~/components/core";
 
-export default function About({ onClickCallback = noop }) {
+export default function About() {
   const [isOpen, setIsOpen] = useState(false);
 
-  const open = () => {
-    setIsOpen(true);
-    onClickCallback();
-  };
-
+  const open = () => setIsOpen(true);
   const close = () => setIsOpen(false);
 
   return (
@@ -42,7 +37,7 @@ export default function About({ onClickCallback = noop }) {
         icon={<Icon name="help" size="24" />}
         onClick={open}
       >
-        About
+        About D-Installer
       </Button>
 
       <Popup

diff --git a/web/src/components/core/About.test.jsx b/web/src/components/core/About.test.jsx
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2022] SUSE LLC
+ * Copyright (c) [2022-2023] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -44,14 +44,4 @@ describe("About", () => {
       expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
     });
   });
-
-  it("triggers given onClickCallback function when opening the dialog", async () => {
-    const onClickCallback = jest.fn();
-
-    const { user } = plainRender(<About onClickCallback={onClickCallback} />);
-    const button = screen.getByRole("button", { name: /About/i });
-
-    await user.click(button);
-    expect(onClickCallback).toHaveBeenCalled();
-  });
 });