From afdcac2465d147abc58166d88f039e1640eb28cb Mon Sep 17 00:00:00 2001
From: Dhruwang Jariwala <67850763+Dhruwang@users.noreply.github.com>
Date: Fri, 17 May 2024 14:48:32 +0530
Subject: [PATCH] fix: Shuffled choices issue (#2639)

---
 .../edit/components/SelectQuestionChoice.tsx  |   3 +-
 .../questions/MultipleChoiceMultiQuestion.tsx | 124 ++++++++++--------
 .../MultipleChoiceSingleQuestion.tsx          | 106 ++++++++-------
 packages/surveys/src/lib/utils.ts             |  32 +++--
 4 files changed, 149 insertions(+), 116 deletions(-)

diff --git a/apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/SelectQuestionChoice.tsx b/apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/SelectQuestionChoice.tsx
index e4242afb71..030cac66b4 100644
--- a/apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/SelectQuestionChoice.tsx
+++ b/apps/web/app/(app)/(survey-editor)/environments/[environmentId]/surveys/[surveyId]/edit/components/SelectQuestionChoice.tsx
@@ -55,9 +55,10 @@ export const SelectQuestionChoice = ({
   surveyLanguageCodes,
   updateQuestion,
 }: ChoiceProps) => {
+  const isDragDisabled = choice.id === "other";
   const { attributes, listeners, setNodeRef, transform, transition } = useSortable({
     id: choice.id,
-    disabled: choice.id === "other",
+    disabled: isDragDisabled,
   });
 
   const style = {
diff --git a/packages/surveys/src/components/questions/MultipleChoiceMultiQuestion.tsx b/packages/surveys/src/components/questions/MultipleChoiceMultiQuestion.tsx
index 7cefe21e01..011fc936c4 100644
--- a/packages/surveys/src/components/questions/MultipleChoiceMultiQuestion.tsx
+++ b/packages/surveys/src/components/questions/MultipleChoiceMultiQuestion.tsx
@@ -5,7 +5,7 @@ import { QuestionMedia } from "@/components/general/QuestionMedia";
 import { Subheader } from "@/components/general/Subheader";
 import { ScrollableContainer } from "@/components/wrappers/ScrollableContainer";
 import { getUpdatedTtc, useTtc } from "@/lib/ttc";
-import { cn, shuffleQuestions } from "@/lib/utils";
+import { cn, getShuffledChoicesIds } from "@/lib/utils";
 import { useCallback, useEffect, useMemo, useRef, useState } from "preact/hooks";
 
 import { getLocalizedValue } from "@formbricks/lib/i18n/utils";
@@ -45,6 +45,13 @@ export const MultipleChoiceMultiQuestion = ({
   const isMediaAvailable = question.imageUrl || question.videoUrl;
   useTtc(question.id, ttc, setTtc, startTime, setStartTime, question.id === currentQuestionId);
 
+  const shuffledChoicesIds = useMemo(() => {
+    if (question.shuffleOption) {
+      return getShuffledChoicesIds(question.choices, question.shuffleOption);
+    } else return question.choices.map((choice) => choice.id);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [question.shuffleOption, question.choices.length, question.choices[question.choices.length - 1].id]);
+
   const getChoicesWithoutOtherLabels = useCallback(
     () =>
       question.choices
@@ -73,15 +80,17 @@ export const MultipleChoiceMultiQuestion = ({
     if (!question.choices) {
       return [];
     }
-    const choicesWithoutOther = question.choices.filter((choice) => choice.id !== "other");
-    if (question.shuffleOption) {
-      return shuffleQuestions(choicesWithoutOther, question.shuffleOption);
-    }
-    return choicesWithoutOther;
-  }, [question.choices, question.shuffleOption]);
+    if (question.shuffleOption === "none" || question.shuffleOption === undefined) return question.choices;
+    return shuffledChoicesIds.map((choiceId) => {
+      const choice = question.choices.find((choice) => {
+        return choice.id === choiceId;
+      });
+      return choice;
+    });
+  }, [question.choices, question.shuffleOption, shuffledChoicesIds]);
 
   const questionChoiceLabels = questionChoices.map((questionChoice) => {
-    return questionChoice.label[languageCode];
+    return questionChoice?.label[languageCode];
   });
 
   const otherOption = useMemo(
@@ -152,55 +161,60 @@ export const MultipleChoiceMultiQuestion = ({
             <fieldset>
               <legend className="sr-only">Options</legend>
               <div className="bg-survey-bg relative space-y-2" ref={choicesContainerRef}>
-                {questionChoices.map((choice, idx) => (
-                  <label
-                    key={choice.id}
-                    tabIndex={idx + 1}
-                    className={cn(
-                      value === choice.label ? "border-border bg-input-selected-bg z-10" : "border-border",
-                      "text-heading bg-input-bg focus-within:border-brand hover:bg-input-bg-selected focus:bg-input-bg-selected rounded-custom relative flex cursor-pointer flex-col border p-4 focus:outline-none"
-                    )}
-                    onKeyDown={(e) => {
-                      // Accessibility: if spacebar was pressed pass this down to the input
-                      if (e.key === " ") {
-                        e.preventDefault();
-                        document.getElementById(choice.id)?.click();
-                        document.getElementById(choice.id)?.focus();
-                      }
-                    }}
-                    autoFocus={idx === 0 && !isInIframe}>
-                    <span className="flex items-center text-sm">
-                      <input
-                        type="checkbox"
-                        id={choice.id}
-                        name={question.id}
-                        tabIndex={-1}
-                        value={choice.label}
-                        className="border-brand text-brand h-4 w-4 border focus:ring-0 focus:ring-offset-0"
-                        aria-labelledby={`${choice.id}-label`}
-                        onChange={(e) => {
-                          if ((e.target as HTMLInputElement)?.checked) {
-                            addItem(getLocalizedValue(choice.label, languageCode));
-                          } else {
-                            removeItem(getLocalizedValue(choice.label, languageCode));
+                {questionChoices.map((choice, idx) => {
+                  if (!choice || choice.id === "other") return;
+                  return (
+                    <label
+                      key={choice.id}
+                      tabIndex={idx + 1}
+                      className={cn(
+                        value.includes(getLocalizedValue(choice.label, languageCode))
+                          ? "border-border bg-input-selected-bg z-10"
+                          : "border-border",
+                        "text-heading bg-input-bg focus-within:border-brand hover:bg-input-bg-selected focus:bg-input-bg-selected rounded-custom relative flex cursor-pointer flex-col border p-4 focus:outline-none"
+                      )}
+                      onKeyDown={(e) => {
+                        // Accessibility: if spacebar was pressed pass this down to the input
+                        if (e.key === " ") {
+                          e.preventDefault();
+                          document.getElementById(choice.id)?.click();
+                          document.getElementById(choice.id)?.focus();
+                        }
+                      }}
+                      autoFocus={idx === 0 && !isInIframe}>
+                      <span className="flex items-center text-sm">
+                        <input
+                          type="checkbox"
+                          id={choice.id}
+                          name={question.id}
+                          tabIndex={-1}
+                          value={getLocalizedValue(choice.label, languageCode)}
+                          className="border-brand text-brand h-4 w-4 border focus:ring-0 focus:ring-offset-0"
+                          aria-labelledby={`${choice.id}-label`}
+                          onChange={(e) => {
+                            if ((e.target as HTMLInputElement)?.checked) {
+                              addItem(getLocalizedValue(choice.label, languageCode));
+                            } else {
+                              removeItem(getLocalizedValue(choice.label, languageCode));
+                            }
+                          }}
+                          checked={
+                            Array.isArray(value) &&
+                            value.includes(getLocalizedValue(choice.label, languageCode))
                           }
-                        }}
-                        checked={
-                          Array.isArray(value) &&
-                          value.includes(getLocalizedValue(choice.label, languageCode))
-                        }
-                        required={
-                          question.required && Array.isArray(value) && value.length
-                            ? false
-                            : question.required
-                        }
-                      />
-                      <span id={`${choice.id}-label`} className="ml-3 font-medium">
-                        {getLocalizedValue(choice.label, languageCode)}
+                          required={
+                            question.required && Array.isArray(value) && value.length
+                              ? false
+                              : question.required
+                          }
+                        />
+                        <span id={`${choice.id}-label`} className="ml-3 font-medium">
+                          {getLocalizedValue(choice.label, languageCode)}
+                        </span>
                       </span>
-                    </span>
-                  </label>
-                ))}
+                    </label>
+                  );
+                })}
                 {otherOption && (
                   <label
                     tabIndex={questionChoices.length + 1}
diff --git a/packages/surveys/src/components/questions/MultipleChoiceSingleQuestion.tsx b/packages/surveys/src/components/questions/MultipleChoiceSingleQuestion.tsx
index cfb7630c7d..031bd35e42 100644
--- a/packages/surveys/src/components/questions/MultipleChoiceSingleQuestion.tsx
+++ b/packages/surveys/src/components/questions/MultipleChoiceSingleQuestion.tsx
@@ -5,7 +5,7 @@ import { QuestionMedia } from "@/components/general/QuestionMedia";
 import { Subheader } from "@/components/general/Subheader";
 import { ScrollableContainer } from "@/components/wrappers/ScrollableContainer";
 import { getUpdatedTtc, useTtc } from "@/lib/ttc";
-import { cn, shuffleQuestions } from "@/lib/utils";
+import { cn, getShuffledChoicesIds } from "@/lib/utils";
 import { useEffect, useMemo, useRef, useState } from "preact/hooks";
 
 import { getLocalizedValue } from "@formbricks/lib/i18n/utils";
@@ -47,18 +47,27 @@ export const MultipleChoiceSingleQuestion = ({
   const choicesContainerRef = useRef<HTMLDivElement | null>(null);
   const isMediaAvailable = question.imageUrl || question.videoUrl;
 
+  const shuffledChoicesIds = useMemo(() => {
+    if (question.shuffleOption) {
+      return getShuffledChoicesIds(question.choices, question.shuffleOption);
+    } else return question.choices.map((choice) => choice.id);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [question.shuffleOption, question.choices.length, question.choices[question.choices.length - 1].id]);
+
   useTtc(question.id, ttc, setTtc, startTime, setStartTime, question.id === currentQuestionId);
 
   const questionChoices = useMemo(() => {
     if (!question.choices) {
       return [];
     }
-    const choicesWithoutOther = question.choices.filter((choice) => choice.id !== "other");
-    if (question.shuffleOption) {
-      return shuffleQuestions(choicesWithoutOther, question.shuffleOption);
-    }
-    return choicesWithoutOther;
-  }, [question.choices, question.shuffleOption]);
+    if (question.shuffleOption === "none" || question.shuffleOption === undefined) return question.choices;
+    return shuffledChoicesIds.map((choiceId) => {
+      const choice = question.choices.find((choice) => {
+        return choice.id === choiceId;
+      });
+      return choice;
+    });
+  }, [question.choices, question.shuffleOption, shuffledChoicesIds]);
 
   const otherOption = useMemo(
     () => question.choices.find((choice) => choice.id === "other"),
@@ -77,7 +86,7 @@ export const MultipleChoiceSingleQuestion = ({
     }
 
     const isOtherSelected =
-      value !== undefined && !questionChoices.some((choice) => choice.label[languageCode] === value);
+      value !== undefined && !questionChoices.some((choice) => choice?.label[languageCode] === value);
     setOtherSelected(isOtherSelected);
   }, [isFirstQuestion, languageCode, otherOption, question.id, questionChoices, value]);
 
@@ -116,45 +125,50 @@ export const MultipleChoiceSingleQuestion = ({
               <legend className="sr-only">Options</legend>
 
               <div className="bg-survey-bg relative space-y-2" role="radiogroup" ref={choicesContainerRef}>
-                {questionChoices.map((choice, idx) => (
-                  <label
-                    tabIndex={idx + 1}
-                    key={choice.id}
-                    className={cn(
-                      value === choice.label ? "border-brand z-10" : "border-border",
-                      "text-heading bg-input-bg focus-within:border-brand focus-within:bg-input-bg-selected hover:bg-input-bg-selected rounded-custom relative flex cursor-pointer flex-col border p-4 focus:outline-none"
-                    )}
-                    onKeyDown={(e) => {
-                      // Accessibility: if spacebar was pressed pass this down to the input
-                      if (e.key === " ") {
-                        e.preventDefault();
-                        document.getElementById(choice.id)?.click();
-                        document.getElementById(choice.id)?.focus();
-                      }
-                    }}
-                    autoFocus={idx === 0 && !isInIframe}>
-                    <span className="flex items-center text-sm">
-                      <input
-                        tabIndex={-1}
-                        type="radio"
-                        id={choice.id}
-                        name={question.id}
-                        value={choice.label}
-                        className="border-brand text-brand h-4 w-4 border focus:ring-0 focus:ring-offset-0"
-                        aria-labelledby={`${choice.id}-label`}
-                        onChange={() => {
-                          setOtherSelected(false);
-                          onChange({ [question.id]: getLocalizedValue(choice.label, languageCode) });
-                        }}
-                        checked={value === getLocalizedValue(choice.label, languageCode)}
-                        required={question.required && idx === 0}
-                      />
-                      <span id={`${choice.id}-label`} className="ml-3 font-medium">
-                        {getLocalizedValue(choice.label, languageCode)}
+                {questionChoices.map((choice, idx) => {
+                  if (!choice || choice.id === "other") return;
+                  return (
+                    <label
+                      tabIndex={idx + 1}
+                      key={choice.id}
+                      className={cn(
+                        value === getLocalizedValue(choice.label, languageCode)
+                          ? "border-brand z-10"
+                          : "border-border",
+                        "text-heading bg-input-bg focus-within:border-brand focus-within:bg-input-bg-selected hover:bg-input-bg-selected rounded-custom relative flex cursor-pointer flex-col border p-4 focus:outline-none"
+                      )}
+                      onKeyDown={(e) => {
+                        // Accessibility: if spacebar was pressed pass this down to the input
+                        if (e.key === " ") {
+                          e.preventDefault();
+                          document.getElementById(choice.id)?.click();
+                          document.getElementById(choice.id)?.focus();
+                        }
+                      }}
+                      autoFocus={idx === 0 && !isInIframe}>
+                      <span className="flex items-center text-sm">
+                        <input
+                          tabIndex={-1}
+                          type="radio"
+                          id={choice.id}
+                          name={question.id}
+                          value={getLocalizedValue(choice.label, languageCode)}
+                          className="border-brand text-brand h-4 w-4 border focus:ring-0 focus:ring-offset-0"
+                          aria-labelledby={`${choice.id}-label`}
+                          onChange={() => {
+                            setOtherSelected(false);
+                            onChange({ [question.id]: getLocalizedValue(choice.label, languageCode) });
+                          }}
+                          checked={value === getLocalizedValue(choice.label, languageCode)}
+                          required={question.required && idx === 0}
+                        />
+                        <span id={`${choice.id}-label`} className="ml-3 font-medium">
+                          {getLocalizedValue(choice.label, languageCode)}
+                        </span>
                       </span>
-                    </span>
-                  </label>
-                ))}
+                    </label>
+                  );
+                })}
                 {otherOption && (
                   <label
                     tabIndex={questionChoices.length + 1}
diff --git a/packages/surveys/src/lib/utils.ts b/packages/surveys/src/lib/utils.ts
index 6a1c56bc0a..7bd071ba72 100644
--- a/packages/surveys/src/lib/utils.ts
+++ b/packages/surveys/src/lib/utils.ts
@@ -1,4 +1,4 @@
-import { TSurvey } from "@formbricks/types/surveys";
+import { TSurvey, TSurveyChoice } from "@formbricks/types/surveys";
 
 export const cn = (...classes: string[]) => {
   return classes.filter(Boolean).join(" ");
@@ -11,24 +11,28 @@ const shuffle = (array: any[]) => {
   }
 };
 
-export const shuffleQuestions = (array: any[], shuffleOption: string) => {
-  const arrayCopy = [...array];
-  const otherIndex = arrayCopy.findIndex((element) => element.id === "other");
-  const otherElement = otherIndex !== -1 ? arrayCopy.splice(otherIndex, 1)[0] : null;
+export const getShuffledChoicesIds = (choices: TSurveyChoice[], shuffleOption: string): string[] => {
+  const otherOption = choices.find((choice) => {
+    return choice.id === "other";
+  });
+  const shuffledChoices = otherOption ? [...choices.filter((choice) => choice.id !== "other")] : [...choices];
 
   if (shuffleOption === "all") {
-    shuffle(arrayCopy);
+    shuffle(shuffledChoices);
   } else if (shuffleOption === "exceptLast") {
-    const lastElement = arrayCopy.pop();
-    shuffle(arrayCopy);
-    arrayCopy.push(lastElement);
+    if (otherOption) {
+      shuffle(shuffledChoices);
+    } else {
+      const lastElement = shuffledChoices.pop();
+      if (lastElement) {
+        shuffle(shuffledChoices);
+        shuffledChoices.push(lastElement);
+      }
+    }
   }
+  if (otherOption) shuffledChoices.push(otherOption);
 
-  if (otherElement) {
-    arrayCopy.push(otherElement);
-  }
-
-  return arrayCopy;
+  return shuffledChoices.map((choice) => choice.id);
 };
 
 export const calculateElementIdx = (survey: TSurvey, currentQustionIdx: number): number => {