Repository: Sage/carbon (ts)

Issue:
[AdvancedColorPicker] - onBlur not being triggered the first time a new color is clicked
### Description

When changing the input value in AdvancedColorPicker by clicking a non-selected color, the onBlur callback should be triggered. But this does not happen the first time a change happens (it does on subsequent ones)

### Reproduction

https://stackblitz.com/edit/parsium-carbon-starter-bigb4p?file=src%2FApp.tsx

### Steps to reproduce

Open the dialog with the picker in, click a non-red color, and note that there is no output in the console.
Subsequently clicking a new color fires the console message as expected.

### JIRA ticket numbers (Sage only)

_No response_

### Suggested solution

_No response_

### Carbon version

141.0.6

### Design tokens version

4.29.0

### Relevant browsers

Chrome

### Relevant OSs

MacOS

### Additional context

Initial investigation suggests the problem is to do with the setting of `isBlurBlocked` in the `handleMouseDown` function (https://stackblitz.com/edit/parsium-carbon-starter-bigb4p?file=src%2FApp.tsx), with the value incorrectly being `true` when the call fails. I do not know why this is happening specifically the first time, or why we are even blocking blur at all.

### Confidentiality

- [X] I confirm there is no confidential or commercially sensitive information included.

Pull request:
fix(simple-color-picker, advanced-color-picker): ensure blur blocking works when switching colors - FE-6707
Blur should be blocked when switching between colors and fired when the component itself is blurred.

fixes #6850

### Proposed behaviour

Blur event should not be fired when switching between colours.
Add deprecation warning to `isBlurBlocked` prop.

### Current behaviour

Blur event is triggered every time a colour is changed.

### Checklist

<!-- Each PR should include the following -->

- [x] Commits follow our style guide
- [x] Related issues linked in commit messages if required
- [ ] Screenshots are included in the PR if useful
- [ ] All themes are supported if required
- [x] Unit tests added or updated if required
- [x] Playwright automation tests added or updated if required
- [x] Storybook added or updated if required
- [ ] Translations added or updated (including creating or amending translation keys table in storybook) if required
- [ ] Typescript `d.ts` file added or updated if required
- [ ] Related docs have been updated if required

#### QA

- [ ] Tested in provided StackBlitz sandbox/Storybook
- [ ] Add new Playwright test coverage if required
- [ ] Carbon implementation matches Design System/designs
- [ ] UI Tests GitHub check reviewed if required

### Additional context

N/A

### Testing instructions

New test story called `OnBlurExample` has been added to `AdvancedColorPickers` test stories. This example allows you to test when the `onBlur` event is fired as it will be logged in the console. 

Blur should only be fired if the component itself is blurred or the component is closed using the `X` button.


Patch:
diff --git a/src/components/advanced-color-picker/advanced-color-picker.pw.tsx b/src/components/advanced-color-picker/advanced-color-picker.pw.tsx
index a3f49bb3d..d6da3c6ea 100644
--- a/src/components/advanced-color-picker/advanced-color-picker.pw.tsx
+++ b/src/components/advanced-color-picker/advanced-color-picker.pw.tsx
@@ -377,7 +377,7 @@ test.describe(
       expect(callbackCount).toBe(1);
     });
 
-    test("should call onBlur callback when a blur event is triggered", async ({
+    test("should not call onBlur callback when a blur event is triggered on another color", async ({
       mount,
       page,
     }) => {
@@ -394,6 +394,25 @@ test.describe(
       await elementToFocus.focus();
       const elementToBlur = simpleColorPickerInput(page, 7);
       await elementToBlur.blur();
+      expect(callbackCount).toBe(0);
+    });
+
+    test("should call onBlur callback when a blur event is triggered", async ({
+      mount,
+      page,
+    }) => {
+      let callbackCount = 0;
+      await mount(
+        <AdvancedColorPickerCustom
+          onBlur={() => {
+            callbackCount += 1;
+          }}
+        />
+      );
+
+      const elementToFocus = simpleColorPickerInput(page, 7);
+      await elementToFocus.focus();
+      await page.locator("body").click({ position: { x: 0, y: 0 } });
       expect(callbackCount).toBe(1);
     });
   }
diff --git a/src/components/simple-color-picker/simple-color-picker.component.tsx b/src/components/simple-color-picker/simple-color-picker.component.tsx
index 3ee66239b..9db85b77a 100644
--- a/src/components/simple-color-picker/simple-color-picker.component.tsx
+++ b/src/components/simple-color-picker/simple-color-picker.component.tsx
@@ -23,6 +23,7 @@ import { ValidationProps } from "../../__internal__/validations";
 import Logger from "../../__internal__/utils/logger";
 
 let deprecateUncontrolledWarnTriggered = false;
+const isBlurBlockedDeprecateWarnTriggered = false;
 
 export interface SimpleColorPickerProps extends ValidationProps, MarginProps {
   /** The SimpleColor components to be rendered in the group */
@@ -260,9 +261,15 @@ export const SimpleColorPicker = React.forwardRef<
   const handleOnBlur = (ev: React.FocusEvent<HTMLInputElement>) => {
     ev.preventDefault();
 
-    if (!blurBlocked && onBlur) {
-      onBlur(ev);
-    }
+    setTimeout(() => {
+      const hasBlurred = !gridItemRefs?.current?.find(
+        (colorRef) => colorRef === document.activeElement
+      );
+      /* istanbul ignore else */
+      if (onBlur && hasBlurred && !blurBlocked) {
+        onBlur(ev);
+      }
+    }, 5);
   };
 
   const handleOnMouseDown = (ev: React.MouseEvent<HTMLElement>) => {
@@ -298,6 +305,13 @@ export const SimpleColorPicker = React.forwardRef<
     );
   }
 
+  if (!isBlurBlockedDeprecateWarnTriggered && isBlurBlocked) {
+    deprecateUncontrolledWarnTriggered = true;
+    Logger.deprecate(
+      `The 'isBlurBlocked' prop in ${SimpleColorPicker.displayName} is deprecated and support will soon be removed.`
+    );
+  }
+
   return (
     <Fieldset
       role="radiogroup"
diff --git a/src/components/simple-color-picker/simple-color-picker.pw.tsx b/src/components/simple-color-picker/simple-color-picker.pw.tsx
index c16d02c3c..2f3d3e054 100644
--- a/src/components/simple-color-picker/simple-color-picker.pw.tsx
+++ b/src/components/simple-color-picker/simple-color-picker.pw.tsx
@@ -512,7 +512,7 @@ test.describe("Check events for SimpleColorPicker component", () => {
     });
   });
 
-  test("should call onBlur callback when a blur event is triggered", async ({
+  test("should call onBlur callback when a blur event is triggered on the component", async ({
     mount,
     page,
   }) => {
@@ -524,10 +524,26 @@ test.describe("Check events for SimpleColorPicker component", () => {
 
     const colorInput = simpleColorPickerInput(page, 5);
     await colorInput.focus();
-    await colorInput.blur();
+    await page.locator("body").click({ position: { x: 0, y: 0 } });
     await expect(callbackCount).toBe(1);
   });
 
+  test("should not call onBlur callback when a blur event is triggered on a color", async ({
+    mount,
+    page,
+  }) => {
+    let callbackCount = 0;
+    const callback: SimpleColorPickerProps["onBlur"] = () => {
+      callbackCount += 1;
+    };
+    await mount(<SimpleColorPickerCustom onBlur={callback} />);
+
+    const colorInput = simpleColorPickerInput(page, 5);
+    await colorInput.focus();
+    await colorInput.blur();
+    await expect(callbackCount).toBe(0);
+  });
+
   test("should not call onBlur callback when a blur event is triggered and isBlurBlocked prop is true", async ({
     mount,
     page,
diff --git a/src/components/simple-color-picker/simple-color-picker.spec.tsx b/src/components/simple-color-picker/simple-color-picker.spec.tsx
index 4df4030d2..f6b032838 100644
--- a/src/components/simple-color-picker/simple-color-picker.spec.tsx
+++ b/src/components/simple-color-picker/simple-color-picker.spec.tsx
@@ -542,7 +542,7 @@ describe("SimpleColorPicker", () => {
 
     describe("handleOnMouseDown", () => {
       describe('if a SimpleColor receives "mousedown, blur"', () => {
-        it("SimpleColorPicker calls onBlur", () => {
+        it("should not call onBlur", () => {
           const firstSCinput = wrapper
             .find(SimpleColor)
             .first()
@@ -551,12 +551,12 @@ describe("SimpleColorPicker", () => {
           firstSCinput.simulate("mousedown");
           fireDocumentMousedown();
           firstSCinput.simulate("blur");
-          expect(onBlur).toHaveBeenCalledTimes(1);
+          expect(onBlur).not.toHaveBeenCalled();
         });
       });
 
       describe('if a SimpleColor receives "mousedown, mousedown, blur"', () => {
-        it("SimpleColorPicker calls onBlur", () => {
+        it("should not call onBlur", () => {
           const firstSCinput = wrapper
             .find(SimpleColor)
             .first()
@@ -566,12 +566,12 @@ describe("SimpleColorPicker", () => {
           firstSCinput.simulate("mousedown");
           fireDocumentMousedown();
           firstSCinput.simulate("blur");
-          expect(onBlur).toHaveBeenCalledTimes(1);
+          expect(onBlur).not.toHaveBeenCalled();
         });
       });
 
       describe("if a mousedown is received by first SimpleColor and then second SimpleColor", () => {
-        it("SimpleColorPicker calls onBlur", () => {
+        it("should not call onBlur", () => {
           const firstSCinput = wrapper
             .find(SimpleColor)
             .first()
@@ -586,7 +586,7 @@ describe("SimpleColorPicker", () => {
           lastSCinput.simulate("mousedown");
           fireDocumentMousedown();
           firstSCinput.simulate("blur");
-          expect(onBlur).toHaveBeenCalledTimes(1);
+          expect(onBlur).not.toHaveBeenCalled();
         });
       });
     });
@@ -625,12 +625,12 @@ describe("SimpleColorPicker", () => {
               .find("input")
               .first()
               .simulate("blur");
-            expect(onBlur).toHaveBeenCalledTimes(1);
+            expect(onBlur).not.toHaveBeenCalled();
           });
         });
 
         describe("when blur is blocked", () => {
-          it("calls onBlur on blur event", () => {
+          it("does not call onBlur on blur event", () => {
             wrapper = render({ isBlurBlocked: true, onBlur });
             wrapper
               .find(SimpleColor)
@@ -778,3 +778,35 @@ describe("SimpleColorPicker", () => {
     });
   });
 });
+
+describe("isBlurBlocked deprecation warning", () => {
+  let loggerSpy: jest.SpyInstance<void, [message: string]> | jest.Mock;
+
+  beforeEach(() => {
+    loggerSpy = jest.spyOn(Logger, "deprecate");
+  });
+
+  afterEach(() => {
+    loggerSpy.mockRestore();
+  });
+
+  afterAll(() => {
+    loggerSpy.mockClear();
+  });
+
+  it("should display the expected deprecation warning once", () => {
+    mount(
+      <SimpleColorPicker
+        legend="SimpleColorPicker Legend"
+        name="test"
+        isBlurBlocked
+      />
+    );
+
+    expect(loggerSpy).toHaveBeenCalledWith(
+      "The 'isBlurBlocked' prop in SimpleColorPicker is deprecated and support will soon be removed."
+    );
+
+    expect(loggerSpy).toHaveBeenCalledTimes(1);
+  });
+});


Test patch:
diff --git a/src/components/advanced-color-picker/advanced-color-picker-test.stories.tsx b/src/components/advanced-color-picker/advanced-color-picker-test.stories.tsx
index cb9220b06..07752d22c 100644
--- a/src/components/advanced-color-picker/advanced-color-picker-test.stories.tsx
+++ b/src/components/advanced-color-picker/advanced-color-picker-test.stories.tsx
@@ -3,7 +3,7 @@ import AdvancedColorPicker, { AdvancedColorPickerProps } from ".";
 
 export default {
   title: "Advanced Color Picker/Test",
-  includeStories: ["Default"],
+  includeStories: ["Default", "OnBlurExample"],
   parameters: {
     info: { disable: true },
     chromatic: {
@@ -95,3 +95,31 @@ export const AdvancedColorPickerCustom = ({
     />
   );
 };
+
+const colors = [
+  { label: "red", value: "red" },
+  { label: "yellow", value: "yellow" },
+  { label: "green", value: "green" },
+  { label: "blue", value: "blue" },
+  { label: "hotpink", value: "hotpink" },
+];
+
+export const OnBlurExample = () => {
+  const [color, setColor] = useState("red");
+  const onChange = (e: { target: { value: React.SetStateAction<string> } }) => {
+    setColor(e.target.value);
+  };
+  const onBlur = () => {
+    console.log("onBlur called");
+  };
+  return (
+    <AdvancedColorPicker
+      availableColors={colors}
+      selectedColor={color}
+      onChange={onChange}
+      defaultColor=""
+      name="choose a colour"
+      onBlur={onBlur}
+    />
+  );
+};
diff --git a/src/components/advanced-color-picker/advanced-color-picker.test.tsx b/src/components/advanced-color-picker/advanced-color-picker.test.tsx
index 60a24e04e..162c406df 100644
--- a/src/components/advanced-color-picker/advanced-color-picker.test.tsx
+++ b/src/components/advanced-color-picker/advanced-color-picker.test.tsx
@@ -328,7 +328,7 @@ test("when a color input is clicked, it triggers the onChange callback", async (
   expect(onChange.mock.calls[0][0].target).toHaveAccessibleName("pink");
 });
 
-test("when a color input is clicked, it triggers the onBlur callback", async () => {
+test("when the user closes the component, it does trigger the onBlur callback", async () => {
   const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
 
   const onBlur = jest.fn();
@@ -337,17 +337,36 @@ test("when a color input is clicked, it triggers the onBlur callback", async ()
   await user.click(screen.getByRole("button", { name: "Change colour" }));
 
   expect(onBlur).not.toHaveBeenCalled();
-  // due to a bug with the component, the first click does not trigger the onBlur callback (https://github.com/Sage/carbon/issues/6850)
-  // To work around this, we fire 2 clicks - the first of these can be removed, and the assertions changed to assert that "orchid" was
-  // focused/blurred, once the bug is fixed
   await user.click(screen.getByRole("radio", { name: "white" }));
+
   expect(await screen.findByRole("radio", { name: "white" })).toBeFocused();
-  await user.click(screen.getByRole("radio", { name: "pink" }));
+
+  await user.click(screen.getByRole("button", { name: "Close" }));
+  jest.runAllTimers();
 
   expect(onBlur).toHaveBeenCalledTimes(1);
   expect(onBlur.mock.calls[0][0].target).toHaveAccessibleName("white");
 });
 
+test("when another color input is clicked, it does not trigger the onBlur callback", async () => {
+  const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
+
+  const onBlur = jest.fn();
+
+  render(<ControlledColorPicker selectedColor="#EBAEDE" onBlur={onBlur} />);
+  await user.click(screen.getByRole("button", { name: "Change colour" }));
+
+  expect(onBlur).not.toHaveBeenCalled();
+  await user.click(screen.getByRole("radio", { name: "white" }));
+
+  expect(await screen.findByRole("radio", { name: "white" })).toBeFocused();
+
+  await user.click(screen.getByRole("radio", { name: "pink" }));
+  jest.runAllTimers();
+
+  expect(onBlur).toHaveBeenCalledTimes(0);
+});
+
 test("when the 'open' prop is true, the dialog is open on mount", async () => {
   render(<ControlledColorPicker open />);