Fix attribute Save button dirty state, double-submit, and loading state (#6659)

Author: mirekm

.changeset/attribute-save-dirty-state.md

@@ -0,0 +1,9 @@
+---
+"saleor-dashboard": patch
+---
+
+**Attribute detail page – Save fixes**
+
+- Saving an attribute no longer leaves the form in a "dirty" state, so the "you have unsaved changes" warning is no longer shown after a successful save.
+- The Save button now disables immediately on the first click, preventing accidental double submits.
+- The Save button keeps its primary appearance while saving and while showing the success checkmark (no more switching to the disabled/grey look on hover), and can no longer be clicked again until it returns to its idle state.

src/attributes/components/AttributePage/AttributePage.tsx

@@ -220,6 +220,7 @@ const AttributePage = ({
         set,
         data,
         isSaveDisabled,
+        isSubmitting,
         submit,
         errors,
         setError,
@@ -399,7 +400,7 @@ const AttributePage = ({
                 <Savebar.Spacer />
                 <Savebar.CancelButton onClick={() => navigate(attributePageBackLink)} />
                 <Savebar.ConfirmButton
-                  transitionState={saveButtonBarState}
+                  transitionState={isSubmitting ? "loading" : saveButtonBarState}
                   onClick={submit}
                   disabled={!!isSaveDisabled}
                 />

src/attributes/mutations.ts

@@ -24,7 +24,7 @@ export const attributeUpdateMutation = gql`
   mutation AttributeUpdate($id: ID!, $input: AttributeUpdateInput!) {
     attributeUpdate(id: $id, input: $input) {
       attribute {
-        ...Attribute
+        ...AttributeUpdateResult
       }
       errors {
         ...AttributeError

src/components/ButtonWithLoader/ButtonWithLoader.module.css

@@ -0,0 +1,6 @@
+/* While loading the button keeps its variant appearance. Disabling pointer
+ * events prevents hover/active styles from shifting the color and blocks
+ * repeated clicks without the "inactive" (disabled) look. */
+.noInteraction {
+  pointer-events: none;
+}

src/components/ButtonWithLoader/ButtonWithLoader.test.tsx

@@ -17,6 +17,16 @@ describe("ButtonWithLoader", () => {
     // Assert
     expect(screen.getByRole("button")).toBeInTheDocument();
     expect(screen.getByTestId("button-progress")).toBeInTheDocument();
+    expect(screen.getByRole("button")).toBeEnabled();
+    expect(screen.getByRole("button")).toHaveAttribute("aria-busy", "true");
+  });
+
+  it("should not call onClick while loading", () => {
+    const onClick = jest.fn();
+
+    render(<ButtonWithLoader transitionState="loading" onClick={onClick} />);
+    fireEvent.click(screen.getByRole("button"));
+    expect(onClick).not.toHaveBeenCalled();
   });
 
   it("should call onClick when clicked", () => {

src/components/ButtonWithLoader/ButtonWithLoader.tsx

@@ -1,9 +1,11 @@
 import { SaleorThrobber } from "@dashboard/components/Throbber";
 import { buttonMessages } from "@dashboard/intl";
 import { Button, type ButtonProps, sprinkles } from "@saleor/macaw-ui-next";
+import clsx from "clsx";
 import { useIntl } from "react-intl";
 
 import { type ConfirmButtonTransitionState } from "../ConfirmButton";
+import styles from "./ButtonWithLoader.module.css";
 
 interface ButtonWithLoaderProps extends ButtonProps {
   transitionState: ConfirmButtonTransitionState;
@@ -14,6 +16,7 @@ export const ButtonWithLoader = ({
   onClick,
   disabled,
   children,
+  className,
   ...props
 }: ButtonWithLoaderProps) => {
   const intl = useIntl();
@@ -42,7 +45,10 @@ export const ButtonWithLoader = ({
   return (
     <Button
       {...props}
-      disabled={isLoading || disabled}
+      className={clsx(className, isLoading && styles.noInteraction)}
+      disabled={disabled}
+      aria-busy={isLoading}
+      tabIndex={isLoading ? -1 : undefined}
       onClick={isLoading ? undefined : onClick}
       data-test-state={isLoading ? "loading" : "default"}
     >

src/components/ConfirmButton/ConfirmButton.module.css

@@ -0,0 +1,7 @@
+/* While the action is in progress or showing success feedback the button keeps
+ * its variant appearance. Disabling pointer events prevents hover/active styles
+ * from shifting the color and blocks repeated clicks without the "inactive"
+ * (disabled) look. */
+.noInteraction {
+  pointer-events: none;
+}

src/components/ConfirmButton/ConfirmButton.test.tsx

@@ -31,6 +31,37 @@ describe("ConfirmButton", () => {
     expect(screen.getByRole("button")).toBeInTheDocument();
     expect(screen.getByTestId("button-progress")).toBeInTheDocument();
   });
+  it("should keep loading button enabled for primary styling and block interaction", () => {
+    render(<ConfirmButton noTransition transitionState="loading" onClick={jest.fn()} />);
+
+    const button = screen.getByRole("button");
+
+    expect(button).toBeEnabled();
+    expect(button).toHaveAttribute("aria-busy", "true");
+    fireEvent.click(button);
+    expect(button).not.toHaveAttribute("disabled");
+  });
+  it("should not call onClick while loading", () => {
+    const onClick = jest.fn();
+
+    render(<ConfirmButton noTransition transitionState="loading" onClick={onClick} />);
+    fireEvent.click(screen.getByRole("button"));
+    expect(onClick).not.toHaveBeenCalled();
+  });
+  it("should not call onClick while success feedback is shown", () => {
+    const onClick = jest.fn();
+
+    render(<ConfirmButton noTransition transitionState="success" onClick={onClick} />);
+    fireEvent.click(screen.getByRole("button"));
+    expect(onClick).not.toHaveBeenCalled();
+  });
+  it("should call onClick in error state so user can try again", () => {
+    const onClick = jest.fn();
+
+    render(<ConfirmButton noTransition transitionState="error" onClick={onClick} />);
+    fireEvent.click(screen.getByRole("button"));
+    expect(onClick).toHaveBeenCalled();
+  });
   it("should render a button with success", () => {
     render(<ConfirmButton noTransition transitionState="success" />);
     expect(screen.getByRole("button")).toBeInTheDocument();

src/components/ConfirmButton/ConfirmButton.tsx

@@ -1,10 +1,13 @@
 import { SaleorThrobber } from "@dashboard/components/Throbber";
 import { buttonMessages } from "@dashboard/intl";
 import { Button, type ButtonProps, sprinkles } from "@saleor/macaw-ui-next";
+import clsx from "clsx";
 import { Check } from "lucide-react";
 import { useEffect, useRef, useState } from "react";
 import { defineMessages, useIntl } from "react-intl";
 
+import styles from "./ConfirmButton.module.css";
+
 const DEFAULT_NOTIFICATION_SHOW_TIME = 3000;
 
 const messages = defineMessages({
@@ -38,13 +41,18 @@ export const ConfirmButton = ({
   disabled,
   children,
   variant,
+  className,
   ...props
 }: ConfirmButtonProps) => {
   const intl = useIntl();
   const [displayCompletedActionState, setDisplayCompletedActionState] = useState(false);
   const timeout = useRef<number>();
+  const isLoading = transitionState === "loading";
   const isCompleted = noTransition ? transitionState !== "default" : displayCompletedActionState;
+  const isSaveDisabled = !isCompleted && !!disabled;
+  const isSuccess = transitionState === "success" && isCompleted;
   const isError = transitionState === "error" && isCompleted;
+  const isInteractionLocked = isLoading || isSuccess;
   const defaultLabels: ConfirmButtonLabels = {
     confirm: intl.formatMessage(buttonMessages.save),
     error: intl.formatMessage(messages.tryAgain),
@@ -122,9 +130,12 @@ export const ConfirmButton = ({
   return (
     <Button
       {...props}
+      className={clsx(className, isInteractionLocked && styles.noInteraction)}
       variant={isError ? "error" : variant}
-      disabled={!isCompleted && disabled}
-      onClick={transitionState === "loading" ? undefined : onClick}
+      disabled={isSaveDisabled}
+      aria-busy={isLoading}
+      tabIndex={isInteractionLocked ? -1 : undefined}
+      onClick={isInteractionLocked ? undefined : onClick}
       data-test-state={isCompleted ? transitionState : "default"}
     >
       {renderContent()}

src/fragments/attributes.ts

@@ -39,6 +39,25 @@ export const attributeFragment = gql`
   }
 `;
 
+export const attributeUpdateResultFragment = gql`
+  fragment AttributeUpdateResult on Attribute {
+    ...Attribute
+    availableInGrid
+    storefrontSearchPosition
+    valueRequired
+    referenceTypes {
+      ... on ProductType {
+        id
+        name
+      }
+      ... on PageType {
+        id
+        name
+      }
+    }
+  }
+`;
+
 export const attributeDetailsFragment = gql`
   fragment AttributeDetails on Attribute {
     ...Attribute