Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #229206 feat: For fill in the blanks mechanic,answer option sho… #187

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Issue #229206 feat: For fill in the blanks mechanic,answer option sho… #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 12 additions & 167 deletions src/components/Practice/Mechanics3.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,111 +68,9 @@ const Mechanics2 = ({
});

const lang = getLocalData("lang");
let wordToCheck = type === "audio" ? parentWords : wordToFill;

//console.log('Mechanics3', answer);

useEffect(() => {
const initializeFillInTheBlank = async () => {
if (type === "fillInTheBlank" && parentWords?.length) {
let wordsArr = parentWords.split(" ");
let randomIndex = Math.floor(Math.random() * wordsArr.length);
try {
await getSimilarWords(wordsArr[randomIndex]);
setWordToFill(wordsArr[randomIndex]);
// wordsArr[randomIndex] = "dash";
setSentences(wordsArr);
setSelectedWord("");
} catch (error) {
console.error("Error in initializeFillInTheBlank:", error);
}
}
};
initializeFillInTheBlank();
}, [contentId, parentWords]);

useEffect(() => {
const initializeAudio = async () => {
if (type === "audio" && parentWords) {
setDisabledWords(true);
setSelectedWord("");
try {
await getSimilarWords(parentWords);
} catch (error) {
console.error("Error in initializeAudio:", error);
}
}
};
initializeAudio();
}, [contentId, parentWords]);

const getSimilarWords = async (wordForFindingHomophones) => {
const lang = getLocalData("lang");
// const isFillInTheBlanks = type === "fillInTheBlank";
const wordToSimilar = wordForFindingHomophones
? wordForFindingHomophones
: parentWords;

if (lang === "en") {
const finder = new HomophonesFinder();
const homophones = await finder.find(wordToSimilar);
let wordsArr = [homophones[8], wordToSimilar, homophones[6]];
setWords(randomizeArray(wordsArr));
} else {
let wordsToShow = [];
if (type == "audio") {
wordsToShow = allWords?.filter((elem) => elem != wordToSimilar);
}
if (type == "fillInTheBlank") {
wordsToShow = allWords
?.join(" ")
?.split(" ")
.filter((elem) => elem !== wordToSimilar && elem.length > 2);
}

wordsToShow = randomizeArray(wordsToShow).slice(0, 2);
wordsToShow.push(wordToSimilar);
setWords(randomizeArray(wordsToShow));
}
};

const handleWord = (word, removeWord) => {
if (removeWord) {
setWords([...words, word]);
setSelectedWord("");
setEnableNext(false);
} else {
let wordsArr = [...words];

if (type !== "audio") {
let index = wordsArr?.findIndex((elem) => elem === word);
if (index !== -1) {
wordsArr?.splice(index, 1);
}
}

if (selectedWord && type !== "audio") {
wordsArr.push(selectedWord);
}

// if (type === "audio") {
const isSoundCorrect = word === wordToCheck;
let audio = new Audio(isSoundCorrect ? correctSound : wrongSound);
if (!isSoundCorrect) {
setEnableNext(false);
}
audio.play();
setShake(true);
setTimeout(() => {
setShake(false);
}, 800);
// }

setWords(wordsArr);
setSelectedWord(word);
}
};

useEffect(() => {
if (!enableNext) {
setAnswer({ text: "", audio_url: "", image_url: "", isAns: false });
Expand Down Expand Up @@ -224,20 +122,11 @@ const Mechanics2 = ({
? 0
: currrentProgress / duration;

const getEnableButton = () => {
if (type === "fillInTheBlank") {
return enableNext;
}
if (type === "audio") {
return selectedWord === wordToCheck;
}
return false;
};
return (
<MainLayout
background={background}
handleNext={handleNext}
enableNext={getEnableButton()}
enableNext={enableNext}
showTimer={showTimer}
points={points}
pageName={"m3"}
Expand Down Expand Up @@ -354,17 +243,15 @@ const Mechanics2 = ({
xs: "relative", // For extra small screens
sm: "relative", // For small screens
md: "relative", // For medium screens
lg: "absolute", // Change as needed for large screens
xl: "absolute", // Change as needed for extra-large screens
lg: "relative", // Change as needed for large screens
xl: "relative", // Change as needed for extra-large screens
},
left: {
xs: 0, // For extra small screens
sm: 0, // For small screens
md: "-40px", // Adjust position for medium screens
lg: "40px",
},
mt: {
lg: "300px",
lg: "0px",
xl: "0px",
},
}}
>
Expand All @@ -374,7 +261,7 @@ const Mechanics2 = ({
style={{
borderRadius: "20px",
maxWidth: "100%",
height: "250px",
height: "clamp(150px, 20vw, 220px)",
}}
alt=""
/>
Expand All @@ -385,7 +272,8 @@ const Mechanics2 = ({
sx={{
display: "flex",
justifyContent: "center",
mt: { xs: "20px", sm: "40px" }, // Add margin-top to create space below the image
mt: { xs: "20px", sm: "40px" },
width: "75%",
}}
>
<Typography
Expand All @@ -402,7 +290,7 @@ const Mechanics2 = ({
>
{answer?.text !== "" ? (
<>
{parentWords?.split("_____")[0]} {/* Before the blank */}
{parentWords?.split(/_+/)[0]}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider handling multiple blanks

The current split implementation assumes a single blank. Consider enhancing it to support multiple blanks in the future.

- {parentWords?.split(/_+/)[0]}
- {parentWords?.split(/_+/)[1]}
+ {parentWords?.split(/(_+)/).map((part, index) => {
+   if (part.match(/^_+$/)) {
+     return answer?.text || part;
+   }
+   return part;
+ })}

Also applies to: 313-313

<span
className={!answer.isAns && shake ? "shakeImage" : ""}
style={{
Expand All @@ -422,7 +310,7 @@ const Mechanics2 = ({
>
{answer?.text}
</span>
{parentWords?.split("_____")[1]} {/* After the blank */}
{parentWords?.split(/_+/)[1]}
</>
) : (
<>{parentWords}</>
Expand All @@ -438,52 +326,9 @@ const Mechanics2 = ({
justifyContent: "center",
marginTop: "20px",
marginBottom: "30px",
flexWrap: "wrap",
}}
>
{type === "audio" &&
words?.map((elem, ind) => (
<Box
key={ind}
className={`${
type === "audio" && selectedWord === elem
? selectedWord === parentWords
? `audioSelectedWord`
: `audioSelectedWrongWord ${shake ? "shakeImage" : ""}`
: ""
}`}
onClick={() => handleWord(elem)}
sx={{
textAlign: "center",
px: "25px",
py: "12px",
// background: "transparent",
m: 1,
textTransform: "none",
borderRadius: "12px",
border: `1px solid rgba(51, 63, 97, 0.10)`,
background: "#FFF",
cursor: "pointer",
opacity: disabledWords ? 0.25 : 1,
pointerEvents: disabledWords ? "none" : "initial",
}}
>
<span
style={{
color:
type === "audio" && selectedWord === elem
? selectedWord === parentWords
? "#58CC02"
: "#C30303"
: "#333F61",
fontWeight: 600,
fontSize: "32px",
fontFamily: "Quicksand",
}}
>
{elem}
</span>
</Box>
))}
<>
{type === "fillInTheBlank" &&
Array.isArray(options) &&
Expand Down Expand Up @@ -550,7 +395,7 @@ const Mechanics2 = ({
dontShowListen={type === "image" || isDiscover}
// updateStory={updateStory}
originalText={parentWords}
enableNext={getEnableButton()}
enableNext={enableNext}
handleNext={handleNext}
audioLink={audio ? audio : null}
{...{
Expand Down
1 change: 1 addition & 0 deletions src/utils/AudioCompare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ const AudioRecorder = (props) => {
<Box
sx={{
marginLeft: props.isShowCase ? "" : "35px",
cursor: "pointer",
}}
>
{props.recordedAudio ? (
Expand Down
99 changes: 68 additions & 31 deletions src/utils/VoiceAnalyser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ function VoiceAnalyser(props) {
const [currentIndex, setCurrentIndex] = useState();
const [temp_audio, set_temp_audio] = useState(null);
const [isStudentAudioPlaying, setIsStudentAudioPlaying] = useState(false);
const [temp_Student_audio, set_temp_Student_audio] = useState(null);
const { callUpdateLearner } = props;
const lang = getLocalData("lang");
const { livesData, setLivesData } = props;
Expand Down Expand Up @@ -107,7 +108,11 @@ function VoiceAnalyser(props) {
audio.addEventListener("canplaythrough", () => {
set_temp_audio(audio);
setPauseAudio(val);
audio.play();
if (val) {
audio.play();
} else {
audio.pause();
}
Comment on lines +111 to +115
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider adding error handling for audio play/pause operations.

While the code handles loading errors, it should also handle play/pause operation failures.

Apply this diff to add error handling:

 if (val) {
-  audio.play();
+  audio.play().catch(error => {
+    console.error("Failed to play audio:", error);
+    setPauseAudio(false);
+    alert("Failed to play the audio. Please try again.");
+  });
 } else {
-  audio.pause();
+  try {
+    audio.pause();
+  } catch (error) {
+    console.error("Failed to pause audio:", error);
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (val) {
audio.play();
} else {
audio.pause();
}
if (val) {
audio.play().catch(error => {
console.error("Failed to play audio:", error);
setPauseAudio(false);
alert("Failed to play the audio. Please try again.");
});
} else {
try {
audio.pause();
} catch (error) {
console.error("Failed to pause audio:", error);
}
}

});

audio.addEventListener("error", (e) => {
Expand All @@ -127,20 +132,50 @@ function VoiceAnalyser(props) {
}
try {
const audio = new Audio(recordedAudio);

if (val) {
audio.play();
setIsStudentAudioPlaying(true);
audio.onended = () => setIsStudentAudioPlaying(false);
} else {
audio.pause();
audio.addEventListener("canplaythrough", () => {
setIsStudentAudioPlaying(val);
set_temp_Student_audio(audio);
if (val) {
audio.play();
audio.onended = () => setIsStudentAudioPlaying(false);
} else {
audio.pause();
}
});
audio.addEventListener("error", (e) => {
console.error("Audio failed to load", e);
setIsStudentAudioPlaying(false);
}
alert("Failed to load the audio. Please try again.");
});
Comment on lines +135 to +149
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add cleanup for audio resources.

The audio element should be properly cleaned up to prevent memory leaks.

Apply this diff to add cleanup:

 const audio = new Audio(recordedAudio);
+let isCleanedUp = false;
+
+const cleanup = () => {
+  if (!isCleanedUp) {
+    audio.removeEventListener("canplaythrough", handler);
+    audio.removeEventListener("error", errorHandler);
+    audio.pause();
+    isCleanedUp = true;
+  }
+};
+
+const handler = () => {
  setIsStudentAudioPlaying(val);
  set_temp_Student_audio(audio);
  if (val) {
    audio.play();
    audio.onended = () => setIsStudentAudioPlaying(false);
  } else {
    audio.pause();
  }
+};
+
+const errorHandler = (e) => {
  console.error("Audio failed to load", e);
  setIsStudentAudioPlaying(false);
  alert("Failed to load the audio. Please try again.");
+  cleanup();
};

-audio.addEventListener("canplaythrough", () => {
-  setIsStudentAudioPlaying(val);
-  set_temp_Student_audio(audio);
-  if (val) {
-    audio.play();
-    audio.onended = () => setIsStudentAudioPlaying(false);
-  } else {
-    audio.pause();
-  }
-});
+audio.addEventListener("canplaythrough", handler);
-audio.addEventListener("error", (e) => {
-  console.error("Audio failed to load", e);
-  setIsStudentAudioPlaying(false);
-  alert("Failed to load the audio. Please try again.");
-});
+audio.addEventListener("error", errorHandler);
+
+return cleanup;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
audio.addEventListener("canplaythrough", () => {
setIsStudentAudioPlaying(val);
set_temp_Student_audio(audio);
if (val) {
audio.play();
audio.onended = () => setIsStudentAudioPlaying(false);
} else {
audio.pause();
}
});
audio.addEventListener("error", (e) => {
console.error("Audio failed to load", e);
setIsStudentAudioPlaying(false);
}
alert("Failed to load the audio. Please try again.");
});
let isCleanedUp = false;
const cleanup = () => {
if (!isCleanedUp) {
audio.removeEventListener("canplaythrough", handler);
audio.removeEventListener("error", errorHandler);
audio.pause();
isCleanedUp = true;
}
};
const handler = () => {
setIsStudentAudioPlaying(val);
set_temp_Student_audio(audio);
if (val) {
audio.play();
audio.onended = () => setIsStudentAudioPlaying(false);
} else {
audio.pause();
}
};
const errorHandler = (e) => {
console.error("Audio failed to load", e);
setIsStudentAudioPlaying(false);
alert("Failed to load the audio. Please try again.");
cleanup();
};
audio.addEventListener("canplaythrough", handler);
audio.addEventListener("error", errorHandler);
return cleanup;

} catch (err) {
console.log(err);
}
};

useEffect(() => {
if (temp_Student_audio !== null) {
if (!isStudentAudioPlaying) {
temp_Student_audio.pause();
props.setVoiceAnimate(false);
} else {
temp_Student_audio.play();
props.setVoiceAnimate(true);
}
temp_Student_audio.onended = function () {
setPauseAudio(false);
props.setVoiceAnimate(false);
};
temp_Student_audio.addEventListener("ended", () =>
setIsStudentAudioPlaying(false)
);
}
return () => {
if (temp_Student_audio !== null) {
temp_Student_audio.pause();
}
};
}, [temp_Student_audio]);
Comment on lines +155 to +177
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve event listener cleanup in useEffect.

The current implementation might lead to memory leaks if the component unmounts while audio is playing.

Apply this diff to improve cleanup:

 useEffect(() => {
   if (temp_Student_audio !== null) {
+    const handleEnded = () => {
+      setPauseAudio(false);
+      props.setVoiceAnimate(false);
+      setIsStudentAudioPlaying(false);
+    };
+
     if (!isStudentAudioPlaying) {
       temp_Student_audio.pause();
       props.setVoiceAnimate(false);
     } else {
       temp_Student_audio.play();
       props.setVoiceAnimate(true);
     }
-    temp_Student_audio.onended = function () {
-      setPauseAudio(false);
-      props.setVoiceAnimate(false);
-    };
-    temp_Student_audio.addEventListener("ended", () =>
-      setIsStudentAudioPlaying(false)
-    );
+    temp_Student_audio.addEventListener("ended", handleEnded);
+
+    return () => {
+      temp_Student_audio.removeEventListener("ended", handleEnded);
+      temp_Student_audio.pause();
+    };
   }
-  return () => {
-    if (temp_Student_audio !== null) {
-      temp_Student_audio.pause();
-    }
-  };
 }, [temp_Student_audio]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (temp_Student_audio !== null) {
if (!isStudentAudioPlaying) {
temp_Student_audio.pause();
props.setVoiceAnimate(false);
} else {
temp_Student_audio.play();
props.setVoiceAnimate(true);
}
temp_Student_audio.onended = function () {
setPauseAudio(false);
props.setVoiceAnimate(false);
};
temp_Student_audio.addEventListener("ended", () =>
setIsStudentAudioPlaying(false)
);
}
return () => {
if (temp_Student_audio !== null) {
temp_Student_audio.pause();
}
};
}, [temp_Student_audio]);
useEffect(() => {
if (temp_Student_audio !== null) {
const handleEnded = () => {
setPauseAudio(false);
props.setVoiceAnimate(false);
setIsStudentAudioPlaying(false);
};
if (!isStudentAudioPlaying) {
temp_Student_audio.pause();
props.setVoiceAnimate(false);
} else {
temp_Student_audio.play();
props.setVoiceAnimate(true);
}
temp_Student_audio.addEventListener("ended", handleEnded);
return () => {
temp_Student_audio.removeEventListener("ended", handleEnded);
temp_Student_audio.pause();
};
}
}, [temp_Student_audio]);


useEffect(() => {
if (temp_audio !== null) {
if (!pauseAudio) {
Expand Down Expand Up @@ -671,28 +706,30 @@ function VoiceAnalyser(props) {
}
})()
)}
<Box sx={{ display: "flex", justifyContent: "center", mt: 4 }}>
{props.enableNext && (
<Box
sx={{ cursor: "pointer" }}
onClick={() => {
if (
props.pageName === "wordsorimage" ||
props.pageName === "m5"
) {
props.updateStoredData(recordedAudio, isMatching);
}
if (props.setIsNextButtonCalled) {
props.setIsNextButtonCalled(true);
} else {
props.handleNext();
}
}}
>
<NextButtonRound />
</Box>
)}
</Box>
{!loader && (
<Box sx={{ display: "flex", justifyContent: "center", mt: 4 }}>
{props.enableNext && (
<Box
sx={{ cursor: "pointer" }}
onClick={() => {
if (
props.pageName === "wordsorimage" ||
props.pageName === "m5"
) {
props.updateStoredData(recordedAudio, isMatching);
}
if (props.setIsNextButtonCalled) {
props.setIsNextButtonCalled(true);
} else {
props.handleNext();
}
}}
>
<NextButtonRound />
</Box>
)}
</Box>
)}
Comment on lines +709 to +732
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider extracting Next button logic to a separate component.

The Next button rendering logic contains complex conditions and could benefit from being extracted into a dedicated component for better maintainability.

Create a new component:

// NextButton.js
const NextButton = ({ 
  loader,
  enableNext,
  pageName,
  updateStoredData,
  recordedAudio,
  isMatching,
  setIsNextButtonCalled,
  handleNext 
}) => {
  if (loader) return null;
  
  const handleClick = () => {
    if (pageName === "wordsorimage" || pageName === "m5") {
      updateStoredData(recordedAudio, isMatching);
    }
    if (setIsNextButtonCalled) {
      setIsNextButtonCalled(true);
    } else {
      handleNext();
    }
  };

  return enableNext ? (
    <Box sx={{ display: "flex", justifyContent: "center", mt: 4 }}>
      <Box sx={{ cursor: "pointer" }} onClick={handleClick}>
        <NextButtonRound />
      </Box>
    </Box>
  ) : null;
};

Then simplify the render:

-{!loader && (
-  <Box sx={{ display: "flex", justifyContent: "center", mt: 4 }}>
-    {props.enableNext && (
-      <Box
-        sx={{ cursor: "pointer" }}
-        onClick={() => {
-          if (
-            props.pageName === "wordsorimage" ||
-            props.pageName === "m5"
-          ) {
-            props.updateStoredData(recordedAudio, isMatching);
-          }
-          if (props.setIsNextButtonCalled) {
-            props.setIsNextButtonCalled(true);
-          } else {
-            props.handleNext();
-          }
-        }}
-      >
-        <NextButtonRound />
-      </Box>
-    )}
-  </Box>
-)}
+<NextButton
+  loader={loader}
+  enableNext={props.enableNext}
+  pageName={props.pageName}
+  updateStoredData={props.updateStoredData}
+  recordedAudio={recordedAudio}
+  isMatching={isMatching}
+  setIsNextButtonCalled={props.setIsNextButtonCalled}
+  handleNext={props.handleNext}
+/>

</div>
);
}
Expand Down