fix: header spacing and forms layout MAASENG-2892

cypress/e2e/with-users/controllers/details.spec.ts

@@ -63,7 +63,7 @@ context("Controller details", () => {
   });
 
   it("lists valid actions on the controller details page", () => {
-    cy.findByText(/Take action/).click();
+    cy.findByRole("button", { name: "Take action" }).click();
 
     [/Set zone/i, /Delete/i].forEach((name) =>
       cy

src/app/base/components/FormikFormContent/FormikFormContent.test.tsx

@@ -21,6 +21,7 @@ import {
   render,
   screen,
   renderWithBrowserRouter,
+  renderWithMockStore,
 } from "@/testing/utils";
 
 const mockStore = configureStore<RootState>();
@@ -497,4 +498,21 @@ describe("FormikFormContent", () => {
 
     expect(screen.getByTestId("footer")).toBeInTheDocument();
   });
+
+  it("renders inline form correctly when inline prop is true", () => {
+    renderWithMockStore(
+      <Formik initialValues={{}} onSubmit={vi.fn()}>
+        <FormikFormContent aria-label="inline form" inline>
+          <Field aria-label="test field" name="testField" />
+        </FormikFormContent>
+      </Formik>
+    );
+
+    expect(screen.getByRole("form", { name: "inline form" })).toHaveClass(
+      "p-form--inline"
+    );
+    expect(
+      screen.getByRole("textbox", { name: "test field" })
+    ).toBeInTheDocument();
+  });
 });

src/app/base/components/FormikFormContent/FormikFormContent.tsx

@@ -142,6 +142,7 @@ const FormikFormContent = <V extends object, E = null>({
   }, [onSuccess, hasErrors, hasSaved, values]);
 
   // Send an analytics event when form is saved.
+
   useSendAnalyticsWhen(
     saved,
     onSaveAnalytics.category,
@@ -167,44 +168,53 @@ const FormikFormContent = <V extends object, E = null>({
     }
   }, [navigate, savedRedirect, saved]);
 
+  const renderFormContent = () => (
+    <>
+      {!!nonFieldError && (
+        <Notification severity="negative" title="Error:">
+          {nonFieldError}
+        </Notification>
+      )}
+      {typeof children === "function" ? children(formikContext) : children}
+    </>
+  );
+
+  const renderFormButtons = () => (
+    <FormikFormButtons
+      {...buttonsProps}
+      buttonsBehavior={buttonsBehavior}
+      cancelDisabled={cancelDisabled === false ? false : saving}
+      inline={inline}
+      saved={saved}
+      saving={saving}
+      submitDisabled={loading || saving || formDisabled || submitDisabled}
+    />
+  );
+
   return (
     <Form
       aria-label={ariaLabel}
       className={className}
       inline={inline}
       onSubmit={handleSubmit}
     >
-      {/* collapse content section when there's no content */}
-      <ContentSection className={!children ? "u-no-padding--top" : undefined}>
-        <ContentSection.Content>
-          {!!nonFieldError && (
-            <Notification severity="negative" title="Error:">
-              {nonFieldError}
-            </Notification>
-          )}
-          {typeof children === "function"
-            ? children({ ...formikContext })
-            : children}
-        </ContentSection.Content>
-        <ContentSection.Footer
-          className={!children ? "u-no-margin--top" : undefined}
-        >
-          {editable && (
-            <FormikFormButtons
-              {...buttonsProps}
-              buttonsBehavior={buttonsBehavior}
-              cancelDisabled={cancelDisabled === false ? false : saving}
-              inline={inline}
-              saved={saved}
-              saving={saving}
-              submitDisabled={
-                loading || saving || formDisabled || submitDisabled
-              }
-            />
-          )}
+      {inline ? (
+        <>
+          {renderFormContent()}
+          {editable && renderFormButtons()}
           {footer}
-        </ContentSection.Footer>
-      </ContentSection>
+        </>
+      ) : (
+        <ContentSection className={!children ? "u-no-padding--top" : undefined}>
+          <ContentSection.Content>{renderFormContent()}</ContentSection.Content>
+          <ContentSection.Footer
+            className={!children ? "u-no-margin--top" : undefined}
+          >
+            {editable && renderFormButtons()}
+            {footer}
+          </ContentSection.Footer>
+        </ContentSection>
+      )}
     </Form>
   );
 };

src/app/base/components/NodeActionMenuGroup/index.scss

@@ -3,6 +3,7 @@
     display: inline-flex;
     flex-wrap: nowrap;
     border: 1px solid $colors--light-theme--border-high-contrast;
+    margin-bottom: $spv--medium;
 
     .p-button {
       .p-icon--delete {

src/app/base/components/SectionHeader/SectionHeader.test.tsx

@@ -19,14 +19,17 @@ describe("SectionHeader", () => {
     renderWithBrowserRouter(<SectionHeader title="Title" />);
     const title = screen.getByRole("heading", { level: 1, name: "Title" });
     expect(title).toBeInTheDocument();
-    expect(title.classList.contains("p-heading--4")).toBe(true);
+    expect(title).toHaveClass("p-heading--4");
   });
 
   it("can change the title element", () => {
     renderWithBrowserRouter(<SectionHeader title="Title" titleElement="div" />);
     const title = screen.getByTestId("section-header-title");
+    expect(
+      screen.queryByRole("heading", { name: "Title" })
+    ).not.toBeInTheDocument();
     expect(title).toBeInTheDocument();
-    expect(title.classList.contains("p-heading--4")).toBe(false);
+    expect(title).toHaveTextContent("Title");
   });
 
   it("shows a spinner instead of title if loading", () => {
@@ -56,7 +59,12 @@ describe("SectionHeader", () => {
       <button key="button-2">Button 2</button>,
     ];
     renderWithBrowserRouter(<SectionHeader buttons={buttons} title="Title" />);
-    expect(screen.getByTestId("section-header-buttons")).toBeInTheDocument();
+    expect(
+      screen.getByRole("button", { name: "Button 1" })
+    ).toBeInTheDocument();
+    expect(
+      screen.getByRole("button", { name: "Button 2" })
+    ).toBeInTheDocument();
   });
 
   it("can render tabs", () => {

src/app/base/components/SectionHeader/SectionHeader.tsx

@@ -1,5 +1,6 @@
 import type { ReactNode } from "react";
 
+import { MainToolbar } from "@canonical/maas-react-components";
 import type { ClassName } from "@canonical/react-components";
 import { List, Spinner, Tabs } from "@canonical/react-components";
 import type { TabLink } from "@canonical/react-components/dist/components/Tabs/Tabs";
@@ -71,59 +72,53 @@ const SectionHeader = <P,>({
   tabLinks,
   title,
   titleClassName,
-  titleElement: TitleElement = "h1",
+  titleElement = "h1",
   ...props
 }: Props<P>): JSX.Element | null => {
   return (
     <div className={classNames("section-header", className)} {...props}>
-      <div className="section-header__main-row">
+      <MainToolbar>
         {loading ? (
-          <div className="section-header__titles u-flex--align-center u-flex--grow u-flex--wrap">
-            <h4
-              aria-label="loading"
-              className="section-header__title"
-              data-testid="section-header-title-spinner"
-            >
-              <Spinner aria-hidden="true" text="Loading..." />
-            </h4>
-          </div>
+          <MainToolbar.Title
+            aria-label="loading"
+            as="h4"
+            className={titleClassName}
+            data-testid="section-header-title-spinner"
+          >
+            <Spinner aria-hidden="true" text="Loading..." />
+          </MainToolbar.Title>
         ) : title ? (
-          <div className="section-header__titles u-flex--align-center u-flex--grow u-flex--wrap">
-            <TitleElement
-              className={classNames(
-                "section-header__title u-flex--no-shrink",
-                titleClassName,
-                {
-                  "p-heading--4": TitleElement === "h1",
-                }
-              )}
-              data-testid="section-header-title"
-            >
-              {title}
-            </TitleElement>
-          </div>
+          <MainToolbar.Title
+            as={titleElement}
+            className={titleClassName}
+            data-testid="section-header-title"
+          >
+            {title}
+          </MainToolbar.Title>
         ) : null}
         {generateSubtitle(
           subtitle,
           subtitleClassName,
           subtitleLoading,
           loading
         )}
-        {buttons?.length ? (
-          <List
-            className="section-header__buttons u-flex--between"
-            data-testid="section-header-buttons"
-            inline
-            items={buttons.map((button, i) => ({
-              content: button,
-              key: `section-header-button-${i}`,
-            }))}
-          />
-        ) : null}
-      </div>
-      {renderButtons && typeof renderButtons === "function"
-        ? renderButtons()
-        : null}
+        <MainToolbar.Controls>
+          {buttons?.length ? (
+            <List
+              className="section-header__buttons u-flex--between"
+              data-testid="section-header-buttons"
+              inline
+              items={buttons.map((button, i) => ({
+                content: button,
+                key: `section-header-button-${i}`,
+              }))}
+            />
+          ) : null}
+          {renderButtons && typeof renderButtons === "function"
+            ? renderButtons()
+            : null}
+        </MainToolbar.Controls>
+      </MainToolbar>
       {actionMenuGroup ? <>{actionMenuGroup}</> : null}
       {tabLinks?.length ? (
         <div className="section-header__tabs" data-testid="section-header-tabs">

src/app/base/components/SectionHeader/_index.scss

@@ -1,8 +1,4 @@
 @mixin SectionHeader {
-  .section-header {
-    padding-top: $spv--medium;
-  }
-
   .section-header__title {
     margin: 0 $sph--large $spv--large 0;
     padding-top: 0.051rem;
@@ -24,4 +20,7 @@
       margin-bottom: $spv--small;
     }
   }
+  .section-header__tabs {
+    margin-bottom: $spv--medium;
+  }
 }

src/app/kvm/components/KVMDetailsHeader/_index.scss

@@ -1,4 +1,7 @@
 @mixin KVMDetailsHeader {
+  .kvm-details-header {
+      margin-bottom: $spv--medium;
+  }
   .kvm-details-header__title-block {
     border-left: $border;
     padding-left: $sph--large;

src/app/kvm/components/LXDHostToolbar/_index.scss

@@ -24,6 +24,7 @@
       border-left: $border;
       padding-left: $sph--large;
       margin-bottom: $spv--medium;
+      margin-top: $spv--medium;
 
       &:not(:last-of-type) {
         padding-right: $sph--large;

src/app/machines/views/MachineDetails/MachineHeader/MachineHeader.tsx

@@ -89,10 +89,43 @@ const MachineHeader = ({
 
   return (
     <SectionHeader
+      renderButtons={() => (
+        <div>
+          <div className="u-hide--medium u-hide--small u-nudge-right">
+            <NodeActionMenuGroup
+              alwaysShowLifecycle
+              excludeActions={[NodeActions.IMPORT_IMAGES]}
+              filterActions
+              hasSelection={true}
+              isNodeLocked={machine.locked}
+              nodeDisplay="machine"
+              nodes={[machine]}
+              onActionClick={handleActionClick}
+              singleNode
+            />
+          </div>
+          <div className="u-hide--large u-nudge-right">
+            <NodeActionMenu
+              alwaysShowLifecycle
+              className="u-hide--large"
+              excludeActions={[NodeActions.IMPORT_IMAGES]}
+              filterActions
+              hasSelection={true}
+              key="action-dropdown"
+              nodeDisplay="machine"
+              nodes={[machine]}
+              onActionClick={handleActionClick}
+              toggleAppearance=""
+              toggleClassName="p-action-menu u-no-margin--bottom"
+              toggleLabel="Menu"
+            />
+          </div>
+        </div>
+      )}
       subtitle={
         editingName ? null : (
           <div className="u-flex--wrap u-flex--align-center">
-            <div className="u-nudge-left u-nudge-right">
+            <div className="u-nudge-left">
               {machine.locked ? (
                 <TooltipButton
                   aria-label="locked"
@@ -115,42 +148,12 @@ const MachineHeader = ({
                   : `Power ${machine.power_state}`}
               </PowerIcon>
             </div>
-            <div className="u-hide--medium u-hide--small u-nudge-right">
-              <NodeActionMenuGroup
-                alwaysShowLifecycle
-                excludeActions={[NodeActions.IMPORT_IMAGES]}
-                filterActions
-                hasSelection={true}
-                isNodeLocked={machine.locked}
-                nodeDisplay="machine"
-                nodes={[machine]}
-                onActionClick={handleActionClick}
-                singleNode
-              />
-            </div>
-            <div className="u-hide--large u-nudge-right">
-              <NodeActionMenu
-                alwaysShowLifecycle
-                className="u-hide--large"
-                excludeActions={[NodeActions.IMPORT_IMAGES]}
-                filterActions
-                hasSelection={true}
-                key="action-dropdown"
-                nodeDisplay="machine"
-                nodes={[machine]}
-                onActionClick={handleActionClick}
-                toggleAppearance=""
-                toggleClassName="p-action-menu u-no-margin--bottom"
-                toggleLabel="Menu"
-              />
-            </div>
           </div>
         )
       }
       subtitleLoading={!isMachineDetails(machine)}
       tabLinks={[
         {
-          active: pathname.startsWith(`${urlBase}/summary`),
           component: Link,
           label: "Summary",
           to: `${urlBase}/summary`,