From d61ab65d825d36fd768f731925e225b94614f3b5 Mon Sep 17 00:00:00 2001
From: Nathan Vasse <nathan.vasse@gmail.com>
Date: Wed, 31 Jan 2024 17:55:41 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(react)=20fix=20multi=20select=20me?=
 =?UTF-8?q?nu=20opening?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

At first we had a simple bug were it was not possible to open the
menu by clicking on the label. This fix is a rework to consider
the select wrapper as the toggle button itself, which is what we
do on the mono version. This change caused various tests to fail.
---
 .changeset/eighty-teachers-collect.md                    | 5 +++++
 .../react/src/components/Forms/Select/mono-common.tsx    | 1 -
 .../react/src/components/Forms/Select/multi-common.tsx   | 6 +++++-
 .../src/components/Forms/Select/multi-searchable.tsx     | 1 +
 .../react/src/components/Forms/Select/multi-simple.tsx   | 7 -------
 .../react/src/components/Forms/Select/multi.spec.tsx     | 9 +++++----
 6 files changed, 16 insertions(+), 13 deletions(-)
 create mode 100644 .changeset/eighty-teachers-collect.md

diff --git a/.changeset/eighty-teachers-collect.md b/.changeset/eighty-teachers-collect.md
new file mode 100644
index 0000000..cfaf7b0
--- /dev/null
+++ b/.changeset/eighty-teachers-collect.md
@@ -0,0 +1,5 @@
+---
+"@openfun/cunningham-react": patch
+---
+
+fix multi select menu opening
diff --git a/packages/react/src/components/Forms/Select/mono-common.tsx b/packages/react/src/components/Forms/Select/mono-common.tsx
index 8f32447..5ce5d31 100644
--- a/packages/react/src/components/Forms/Select/mono-common.tsx
+++ b/packages/react/src/components/Forms/Select/mono-common.tsx
@@ -87,7 +87,6 @@ export const SelectMonoAux = ({
 }: SelectAuxProps) => {
   const { t } = useCunningham();
   const labelProps = downshiftReturn.getLabelProps();
-
   return (
     <Field state={state} {...props}>
       <div
diff --git a/packages/react/src/components/Forms/Select/multi-common.tsx b/packages/react/src/components/Forms/Select/multi-common.tsx
index bc5d997..a8aca78 100644
--- a/packages/react/src/components/Forms/Select/multi-common.tsx
+++ b/packages/react/src/components/Forms/Select/multi-common.tsx
@@ -63,6 +63,10 @@ export interface SelectMultiAuxProps extends SubProps {
 export const SelectMultiAux = ({ children, ...props }: SelectMultiAuxProps) => {
   const { t } = useCunningham();
   const labelProps = props.downshiftReturn.getLabelProps();
+
+  // We need to remove onBlur from toggleButtonProps because it triggers a menu closing each time
+  // we tick a checkbox using the monoline style.
+  const { onBlur, ...toggleProps } = props.downshiftReturn.toggleButtonProps;
   return (
     <Field {...props}>
       <div
@@ -85,6 +89,7 @@ export const SelectMultiAux = ({ children, ...props }: SelectMultiAuxProps) => {
               props.downshiftReturn.isOpen && !props.disabled,
           })}
           {...props.downshiftReturn.wrapperProps}
+          {...toggleProps}
         >
           {props.selectedItems.map((selectedItem, index) => (
             <input
@@ -138,7 +143,6 @@ export const SelectMultiAux = ({ children, ...props }: SelectMultiAuxProps) => {
                     </span>
                   }
                   disabled={props.disabled}
-                  {...props.downshiftReturn.toggleButtonProps}
                 />
               </div>
               <div className="c__select__inner__value">
diff --git a/packages/react/src/components/Forms/Select/multi-searchable.tsx b/packages/react/src/components/Forms/Select/multi-searchable.tsx
index 98b3af0..16587da 100644
--- a/packages/react/src/components/Forms/Select/multi-searchable.tsx
+++ b/packages/react/src/components/Forms/Select/multi-searchable.tsx
@@ -155,6 +155,7 @@ export const SelectMultiSearchable = forwardRef<SelectHandle, SubProps>(
             {...inputProps}
             onFocus={() => {
               setHasInputFocused(true);
+              downshiftReturn.openMenu();
             }}
             onBlur={() => {
               setHasInputFocused(false);
diff --git a/packages/react/src/components/Forms/Select/multi-simple.tsx b/packages/react/src/components/Forms/Select/multi-simple.tsx
index b57d37c..ff901c4 100644
--- a/packages/react/src/components/Forms/Select/multi-simple.tsx
+++ b/packages/react/src/components/Forms/Select/multi-simple.tsx
@@ -115,13 +115,6 @@ export const SelectMultiSimple = forwardRef<SelectHandle, SubProps>(
         menuOptionsStyle={props.monoline ? "checkbox" : "plain"}
         downshiftReturn={{
           ...downshiftReturn,
-          wrapperProps: {
-            onClick: () => {
-              if (!props.disabled) {
-                downshiftReturn.toggleMenu();
-              }
-            },
-          },
           toggleButtonProps: downshiftReturn.getToggleButtonProps({
             ...useMultipleSelectionReturn.getDropdownProps({
               preventKeyAction: downshiftReturn.isOpen,
diff --git a/packages/react/src/components/Forms/Select/multi.spec.tsx b/packages/react/src/components/Forms/Select/multi.spec.tsx
index 7345cce..8ee97b5 100644
--- a/packages/react/src/components/Forms/Select/multi.spec.tsx
+++ b/packages/react/src/components/Forms/Select/multi.spec.tsx
@@ -781,8 +781,11 @@ describe("<Select multi={true} />", () => {
       await user.click(clearButton);
       expectSelectedOptions([]);
 
-      // Select again London.
+      // Close and re-open the menu. ( This is needed in this tests but in real life it works, I don't know exactly why .. )
       await user.click(input);
+      await user.click(input);
+
+      // Select again Paris.
       await user.click(
         screen.getByRole("option", {
           name: "Paris",
@@ -830,7 +833,7 @@ describe("<Select multi={true} />", () => {
       // Focus the select.
       await user.click(input);
       expectMenuToBeOpen(menu);
-      expect(document.activeElement?.className).toContain("c__button");
+      expect(document.activeElement?.className).toContain("c__select__wrapper");
 
       // Blur the select.
       ref.current?.blur();
@@ -1041,8 +1044,6 @@ describe("<Select multi={true} />", () => {
       // Make sure the option is selected.
       expectSelectedOptionsText(["London", "Paris"]);
 
-      screen.logTestingPlaygroundURL();
-
       // Make sure the menu stays open.
       expectMenuToBeOpen(menu);