Merge pull request #892 from openSUSE/section-improvements-20231127

[web] Agama/Section fixes and improvements

web/package/cockpit-agama.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Wed Nov 29 13:01:04 UTC 2023 - David Diaz <[email protected]>
+
+- UI: improve the look and feel by fine tunning the sections spacing,
+  alignment, and icon sizes (gh#openSUSE/agama#892).
+
 -------------------------------------------------------------------
 Tue Nov 21 15:21:06 UTC 2023 - David Diaz <[email protected]>
 

web/src/assets/styles/blocks.scss

@@ -3,11 +3,15 @@
 // section layouts.
 section:not([class^="pf-c"]) {
   display: grid;
-  grid-template-columns: min-content 1fr;
+  grid-template-columns: var(--section-icon-size) 1fr;
   grid-template-areas:
     "icon title"
     ".... content";
   gap: var(--spacer-small);
+  padding-inline-start: calc(
+    var(--header-icon-size) - var(--section-icon-size)
+  );
+  padding-inline-end: var(--section-icon-size);
 }
 
 section:not(:last-child, [class^="pf-c"]) {
@@ -16,6 +20,8 @@ section:not(:last-child, [class^="pf-c"]) {
 
 section > svg {
   grid-area: icon;
+  block-size: var(--section-icon-size);
+  inline-size: var(--section-icon-size);
 }
 
 section > h2 {
@@ -84,6 +90,7 @@ section > .content {
   right: 0;
   z-index: 1000;
   inline-size: 70%;
+  min-inline-size: min-content;
   box-shadow: -10px 10px 20px 0 var(--color-primary);
 }
 
@@ -140,6 +147,7 @@ section > .content {
 .sidebar[data-state="hidden"] {
   transition: all 0.04s ease-in-out;
   inline-size: 0;
+  min-inline-size: 0;
   box-shadow: none;
 }
 

web/src/assets/styles/layout.scss

@@ -33,14 +33,16 @@
 
   svg {
     color: white;
+    block-size: var(--header-icon-size);
+    inline-size: var(--header-icon-size);
   }
 }
 
 .wrapper > main {
   grid-area: content;
   overflow-y: auto; // Sadly, only Firefox supports overflow-block at this moment (Jan 2023)
   overflow-block: auto;
-  padding: var(--spacer-medium);
+  padding: var(--spacer-medium) var(--spacer-small);
 }
 
 .wrapper > footer {

web/src/assets/styles/utilities.scss

@@ -59,6 +59,11 @@
   height: 24px;
 }
 
+.icon-size-28 {
+  width: 28px;
+  height: 28px;
+}
+
 .icon-size-32 {
   width: 32px;
   height: 32px;

web/src/assets/styles/variables.scss

@@ -26,7 +26,7 @@
   --stack-gutter: var(--spacer-normal);
   --split-gutter: var(--spacer-small);
 
-  --wrapper-padding: var(--spacer-normal);
+  --wrapper-padding: var(--spacer-small);
   --wrapper-background: white;
 
   --color-primary: #0c322c;
@@ -60,7 +60,8 @@
   --gradient-border-start-color: var(--color-gray);
   --gradient-border-end-color: transparent;
 
-  --icon-size-m: 32px;
+  --header-icon-size: 32px;
+  --section-icon-size: 28px;
 
   --header-block-size: auto;
   --footer-block-size: auto;

web/src/components/core/Page.jsx

@@ -87,7 +87,7 @@ export default function Page({
   return (
     <>
       { title && <Title>{title}</Title> }
-      { icon && <PageIcon><Icon name={icon} /></PageIcon> }
+      { icon && <PageIcon><Icon name={icon} size="32" /></PageIcon> }
       <MainActions>
         { action ||
           <Button size="lg" variant={actionVariant} onClick={pageAction}>

web/src/components/core/Section.jsx

@@ -77,12 +77,14 @@ export default function Section({
   const Header = () => {
     if (!title?.trim()) return;
 
-    const header = !path?.trim() ? <>{title}</> : <Link to={path}>{title}</Link>;
+    const iconName = loading ? "loading" : icon;
+    const headerIcon = iconName ? <Icon name={iconName} /> : null;
+    const headerText = !path?.trim() ? title : <Link to={path}>{title}</Link>;
 
     return (
       <>
-        <Icon name={loading ? "loading" : icon} />
-        <h2 id={headerId}>{header}</h2>
+        {headerIcon}
+        <h2 id={headerId}>{headerText}</h2>
       </>
     );
   };

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

@@ -49,6 +49,8 @@ describe("Section", () => {
       const { container } = plainRender(<Section title="Settings" />);
       const icon = container.querySelector("svg");
       expect(icon).toBeNull();
+      // Check that <Icon /> component was not mounted with 'undefined'
+      expect(console.error).not.toHaveBeenCalled();
     });
 
     it("does not render an icon if not valid icon name is given", () => {
@@ -70,12 +72,6 @@ describe("Section", () => {
       const icon = container.querySelector("svg");
       expect(icon).toBeNull();
     });
-
-    it("does not render the loading icon", () => {
-      const { container } = plainRender(<Section loading />);
-      const icon = container.querySelector("svg");
-      expect(icon).toBeNull();
-    });
   });
 
   describe("when aria-label is given", () => {
@@ -166,6 +162,7 @@ describe("Section", () => {
       expect(icon).toBeNull();
     });
   });
+
   describe("when path is given", () => {
     it("renders a link for navigating to it", async () => {
       installerRender(<Section title="Settings" path="/settings" />);

web/src/components/layout/Icon.jsx

@@ -150,7 +150,7 @@ const icons = {
  * @param {object} [props.otherProps] Other props sent to SVG icon.
  *
  */
-export default function Icon({ name, className = "", size = 32, ...otherProps }) {
+export default function Icon({ name, className = "", size = 28, ...otherProps }) {
   if (!name) {
     console.error(`Icon called without name. '${name}' given instead. Rendering nothing.`);
     return null;