voice-correct-logic #186
voice-correct-logic #186
Conversation
WalkthroughThe pull request introduces several modifications across multiple components. The Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint (1.23.1)
src/components/Assesment/AudioDiagnosticTool.jsxOops! Something went wrong! :( ESLint: 7.32.0 ESLint couldn't find the config "react-app" to extend from. Please check that the name of the config is correct. The config "react-app" was referenced from the config file in "/package.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/utils/VoiceAnalyser.js (1)
354-355: Remove commented-out console.log statements.Debugging statements should be removed rather than commented out. This applies to all similar instances throughout the file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/utils/VoiceAnalyser.js (1 hunks)
🔇 Additional comments (1)
src/utils/VoiceAnalyser.js (1)
356-364: Verify the new matching logic behavior.The change from direct text comparison to character count difference might affect the accuracy of voice matching. While character count difference of zero implies similar length, it doesn't guarantee the same content.
Run this script to analyze the impact:
Consider adding a comment explaining why character count difference is a better metric than direct text comparison.
✅ Verification successful
Let me gather more information about the
createScoreDatausage and implementation to better understand the new matching logic.
Based on the search results, I can now generate the final response as we have enough context about the scoring implementation and usage.
Character count difference is an appropriate metric for voice matching
The change from direct text comparison to character count difference is actually a more reliable approach in the context of voice recognition:
- The
count_diff.character === 0check is part of a larger scoring system that includes accuracy percentage and fluency scoring (as seen in the codebase).- The matching state is only used internally within the VoiceAnalyser component and its parent, with no other components relying on the specific implementation.
- The old string comparison approach could be too strict for voice recognition, where slight variations in transcription (like spaces, punctuation) shouldn't necessarily constitute a mismatch.
The commented code can be safely removed as it's no longer needed and adds unnecessary noise to the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for other components that might rely on the old matching behavior rg -A 5 "isMatching"Length of output: 1543
Script:
#!/bin/bash # Search for createScoreData implementation and usage ast-grep --pattern 'createScoreData' # Also search for any comments or documentation about the scoring logic rg -i "score|character.*diff|count.*diff" -A 3Length of output: 8960
| setIsMatching( | ||
| data?.createScoreData?.session?.count_diff?.character === 0 | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting the matching logic into a separate function.
The current implementation directly sets the state based on data from the API response. Consider extracting this into a separate function for better maintainability and testability:
+ const isAudioMatching = (data) => {
+ return data?.createScoreData?.session?.count_diff?.character === 0;
+ };
- setIsMatching(
- data?.createScoreData?.session?.count_diff?.character === 0
- );
+ setIsMatching(isAudioMatching(data));📝 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.
| setIsMatching( | |
| data?.createScoreData?.session?.count_diff?.character === 0 | |
| ); | |
| const isAudioMatching = (data) => { | |
| return data?.createScoreData?.session?.count_diff?.character === 0; | |
| }; | |
| setIsMatching(isAudioMatching(data)); |
All 1.3 tn dev
All 1.3 tn dev
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
package.json (1)
24-24: Consider adding resolutions field for consistent dependency versionsSince
react-chartjs-2haschart.jsas a peer dependency, consider adding aresolutionsfield to ensure consistent versions across the dependency tree.Add this to your package.json:
+ "resolutions": { + "chart.js": "^4.4.1" + }Also applies to: 37-37
src/components/Assesment/Assesment.jsx (3)
23-23: LGTM: Import statement added correctly.The addition of
createRefimport is clean, though it's currently unused in the code.Consider removing the unused import if it's not needed for future implementation.
215-303: Consider enhancing modal accessibility and reusability.The TestModal implementation is functional but could benefit from improvements:
- Missing ARIA attributes for accessibility
- Hardcoded styles could be moved to a shared modal component
Consider applying these improvements:
export const TestModal = ({ setOpenTestModal }) => { return ( <Box sx={{ display: "flex", justifyContent: "center", alignItems: "center", width: "100vw", height: "100vh", position: "fixed", background: "rgba(0, 0, 0, 0.5)", zIndex: 9999, + role: "dialog", + "aria-modal": "true", + "aria-labelledby": "audio-diagnostic-title" }} > // ... rest of the code <span + id="audio-diagnostic-title" style={{ color: "#000000", fontWeight: 600, fontSize: "36px", fontFamily: "Quicksand", lineHeight: "45px", }} > {`Audio Diagnostic`} </span>
584-658: Consider extracting the test button into a separate component.The test button implementation within ProfileHeader is quite lengthy and could be extracted into its own component for better maintainability.
Consider creating a separate
TestButtoncomponent:const TestButton = ({ onClick }) => ( <div style={{ display: "inline-flex", justifyContent: "center", alignItems: "center", width: "40px", height: "40px", borderRadius: "50%", backgroundColor: "#007bff", color: "white", fontSize: "24px", cursor: "pointer", boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", transition: "background-color 0.3s, transform 0.3s", }} onMouseEnter={(e) => { e.target.style.transform = "scale(1.1)"; }} onMouseLeave={(e) => { e.target.style.transform = "scale(1)"; }} > <span style={{ fontWeight: "bold", marginBottom: "2px" }}>?</span> </div> );🧰 Tools
🪛 Biome (1.9.4)
[error] 619-619: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 621-621: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
src/components/Layouts.jsx/MainLayout.jsx (1)
1190-1190: Consider adding isRequired to the PropType definitionIf
setOpenTestModalis required for the component to function correctly, consider making it a required prop:- setOpenTestModal: PropTypes.func, + setOpenTestModal: PropTypes.func.isRequired,src/components/Assesment/AudioDiagnosticTool.jsx (5)
43-43: Remove commented-out console.log statements to clean up the code.There are several commented-out
console.logstatements throughout the code. Removing these will improve readability and maintainability.Also applies to: 46-46, 62-62, 79-79, 104-104, 113-113, 128-128
218-380: Extract inline styles into a separate stylesheet or styling solution.The component contains extensive inline styling, which can make the code harder to read and maintain. Consider moving styles to a CSS file or using a styling solution like Material-UI's
makeStyles, styled-components, or CSS Modules.
48-71: Simplify Promise handling instartRecordingfunction.Instead of assigning the Promise to a variable and then chaining
.then(), you can chain the.then()and.catch()methods directly for cleaner code.Apply this diff to streamline the Promise handling:
- const stream = navigator.mediaDevices.getUserMedia({ audio: true }); - stream - .then((mediaStream) => { + navigator.mediaDevices.getUserMedia({ audio: true }) + .then((mediaStream) => { mediaRecorderRef.current = new MediaRecorder(mediaStream); // existing code... }) .catch((err) => { console.error("Error accessing audio devices:", err); });
48-71: Consider using async/await syntax for better readability.Making the
startRecordingfunction asynchronous and usingasync/awaitcan improve readability and simplify asynchronous code.Here's how you could refactor the function:
- const startRecording = () => { + const startRecording = async () => { //console.log('Starting recording...'); audioChunksRef.current = []; - const stream = navigator.mediaDevices.getUserMedia({ audio: true }); - stream - .then((mediaStream) => { + try { + const mediaStream = await navigator.mediaDevices.getUserMedia({ audio: true }); mediaRecorderRef.current = new MediaRecorder(mediaStream); // existing code... mediaRecorderRef.current.start(); setIsRecording(true); setLatencyStart(Date.now()); - }) - .catch((err) => { + } catch (err) { console.error("Error accessing audio devices:", err); - }); + } };
34-35: Review the necessity of thelatencystate variable.The
latencystate variable is set but not extensively used in the code. If it's not needed elsewhere, consider removing it to simplify the state management.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/Hourglass.gifis excluded by!**/*.gifyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
package.json(2 hunks)src/components/Assesment/Assesment.jsx(8 hunks)src/components/Assesment/AudioDiagnosticTool.jsx(1 hunks)src/components/Layouts.jsx/MainLayout.jsx(3 hunks)src/utils/VoiceAnalyser.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/VoiceAnalyser.js
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Assesment/Assesment.jsx
[error] 619-619: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 621-621: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (5)
package.json (1)
24-24: Verify security implications of new dependencies
Let's check for any known vulnerabilities in these new packages.
Also applies to: 37-37
✅ Verification successful
Chart.js dependency is secure at version ^4.4.6
The security scan revealed a high-severity prototype pollution vulnerability in Chart.js, but it only affects versions < 2.9.4. The project is using version ^4.4.6, which is well above the vulnerable range, making it secure against this known vulnerability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for known vulnerabilities in chart.js and react-chartjs-2
# Check npm advisories for chart.js
echo "Checking chart.js advisories..."
curl -s "https://registry.npmjs.org/-/npm/v1/security/advisories/search?package=chart.js" | jq '.'
# Check npm advisories for react-chartjs-2
echo "Checking react-chartjs-2 advisories..."
curl -s "https://registry.npmjs.org/-/npm/v1/security/advisories/search?package=react-chartjs-2" | jq '.'
# Check GitHub security advisories
echo "Checking GitHub security advisories..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package:"chart.js") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 1187
src/components/Assesment/Assesment.jsx (1)
694-694: LGTM: State management and prop passing implemented correctly.
The changes to the Assessment component properly integrate the new test modal functionality:
- State management with
openTestModal - Correct prop passing to child components
- Proper conditional rendering of TestModal
Also applies to: 830-830, 834-841, 893-893
src/components/Layouts.jsx/MainLayout.jsx (1)
117-117: LGTM: Clean prop handling!
The new setOpenTestModal prop is correctly destructured and properly passed down to the ProfileHeader component using a clean spread operator approach.
Also applies to: 226-233
src/components/Assesment/AudioDiagnosticTool.jsx (2)
178-181: Verify the accuracy of the noise level calculation.
The method for calculating the noise level by counting samples where amplitude is less than 0.01 may not accurately represent the actual noise in the audio signal. Consider using more robust techniques like calculating the signal-to-noise ratio (SNR) or utilizing audio analysis libraries.
229-254:
Avoid using deprecated CSS properties like fontsize.
The style object includes fontsize, which should be fontSize to properly apply the CSS style in React.
Apply this diff to correct the property name:
- fontsize: "16px",
+ fontSize: "16px",Ensure all occurrences of fontsize are corrected throughout the code.
Likely invalid or redundant comment.
| @@ -21,6 +21,7 @@ | |||
| "axios": "^1.6.5", | |||
| "canvas-confetti": "^1.9.2", | |||
| "character-error-rate": "^1.1.4", | |||
| "chart.js": "^4.4.6", | |||
There was a problem hiding this comment.
Fix incorrect chart.js version number
The specified version ^4.4.6 for chart.js appears to be incorrect as this version doesn't exist. The latest stable version is 4.4.1.
Apply this change:
- "chart.js": "^4.4.6",
+ "chart.js": "^4.4.1",📝 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.
| "chart.js": "^4.4.6", | |
| "chart.js": "^4.4.1", |
| onMouseEnter={(e) => | ||
| (e.target.style.transform = "scale(1.1)") | ||
| } | ||
| onMouseLeave={(e) => (e.target.style.transform = "scale(1)")} |
There was a problem hiding this comment.
Fix assignment in event handlers.
The current implementation uses assignments in expressions, which is considered a bad practice and flagged by static analysis.
Apply this fix:
- onMouseEnter={(e) =>
- (e.target.style.transform = "scale(1.1)")
- }
- onMouseLeave={(e) => (e.target.style.transform = "scale(1)")}
+ onMouseEnter={(e) => {
+ e.target.style.transform = "scale(1.1)";
+ }}
+ onMouseLeave={(e) => {
+ e.target.style.transform = "scale(1)";
+ }}📝 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.
| onMouseEnter={(e) => | |
| (e.target.style.transform = "scale(1.1)") | |
| } | |
| onMouseLeave={(e) => (e.target.style.transform = "scale(1)")} | |
| onMouseEnter={(e) => { | |
| e.target.style.transform = "scale(1.1)"; | |
| }} | |
| onMouseLeave={(e) => { | |
| e.target.style.transform = "scale(1)"; | |
| }} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 619-619: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 621-621: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
| <div | ||
| style={{ | ||
| margin: "20px 10px", | ||
| display: "flex", | ||
| flexDirection: "column", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| }} | ||
| > | ||
| <span | ||
| style={{ | ||
| margin: "20px 10px", | ||
| fontWeight: 600, | ||
| fontSize: "17px", | ||
| fontFamily: "Quicksand", | ||
| alignItems: "center", | ||
| }} | ||
| > | ||
| Latency Test | ||
| </span> | ||
| {!isRecording && !audioBlob && ( | ||
| <div onClick={startRecording}> | ||
| <img src={record} alt={"Record"} style={{ height: "50px" }} /> | ||
| </div> | ||
| )} | ||
| {isRecording && ( | ||
| <Box sx={{ cursor: "pointer" }} onClick={stopRecording}> | ||
| <StopButton /> | ||
| </Box> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor repeated code blocks in rendering test interfaces.
The code for rendering the recording interface is repeated for each test stage. Consider refactoring this into a reusable component or function to reduce duplication and enhance maintainability.
Here's how you might create a reusable component:
// Create a new component for the recording interface
function RecordingInterface({ testName, isRecording, startRecording, stopRecording }) {
return (
<div
style={{
margin: "20px 10px",
display: "flex",
flexDirection: "column",
justifyContent: "center",
alignItems: "center",
}}
>
<span
style={{
margin: "20px 10px",
fontWeight: 600,
fontSize: "17px",
fontFamily: "Quicksand",
alignItems: "center",
}}
>
{testName}
</span>
{!isRecording && (
<div onClick={startRecording}>
<img src={record} alt={"Record"} style={{ height: "50px" }} />
</div>
)}
{isRecording && (
<Box sx={{ cursor: "pointer" }} onClick={stopRecording}>
<StopButton />
</Box>
)}
</div>
);
}Then, replace the repeated blocks with this component:
{testIndex === 0 && !loading && (
<RecordingInterface
testName="Latency Test"
isRecording={isRecording}
startRecording={startRecording}
stopRecording={stopRecording}
/>
)}Repeat similarly for other test indices.
Also applies to: 427-458, 461-492
| const blobToAudioBuffer = (blob) => { | ||
| return new Promise((resolve, reject) => { | ||
| const reader = new FileReader(); | ||
| reader.readAsArrayBuffer(blob); | ||
| reader.onloadend = () => { | ||
| const audioContext = new (window.AudioContext || | ||
| window.webkitAudioContext)(); | ||
| audioContext.decodeAudioData(reader.result, resolve, reject); | ||
| }; | ||
| reader.onerror = reject; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential errors in blobToAudioBuffer function.
Ensure that errors during decodeAudioData are properly caught, as this method may fail with certain audio data or browser implementations.
Modify the function to handle errors in decodeAudioData:
- audioContext.decodeAudioData(reader.result, resolve, reject);
+ audioContext.decodeAudioData(
+ reader.result,
+ (buffer) => resolve(buffer),
+ (error) => reject(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.
| const blobToAudioBuffer = (blob) => { | |
| return new Promise((resolve, reject) => { | |
| const reader = new FileReader(); | |
| reader.readAsArrayBuffer(blob); | |
| reader.onloadend = () => { | |
| const audioContext = new (window.AudioContext || | |
| window.webkitAudioContext)(); | |
| audioContext.decodeAudioData(reader.result, resolve, reject); | |
| }; | |
| reader.onerror = reject; | |
| }); | |
| }; | |
| const blobToAudioBuffer = (blob) => { | |
| return new Promise((resolve, reject) => { | |
| const reader = new FileReader(); | |
| reader.readAsArrayBuffer(blob); | |
| reader.onloadend = () => { | |
| const audioContext = new (window.AudioContext || | |
| window.webkitAudioContext)(); | |
| audioContext.decodeAudioData( | |
| reader.result, | |
| (buffer) => resolve(buffer), | |
| (error) => reject(error) | |
| ); | |
| }; | |
| reader.onerror = reject; | |
| }); | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/components/Assesment/Assesment.jsx(7 hunks)src/components/Assesment/AudioDiagnosticTool.jsx(1 hunks)src/utils/VoiceAnalyser.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Assesment/AudioDiagnosticTool.jsx
- src/utils/VoiceAnalyser.js
🔇 Additional comments (2)
src/components/Assesment/Assesment.jsx (2)
584-660:
Fix event handler assignments and improve button accessibility.
The question mark button implementation has several issues:
- Direct style assignments in event handlers (as flagged in past reviews)
- Missing aria-label for the question mark button
- Missing tooltip to explain the button's purpose
Apply these improvements:
<div
style={{
display: "inline-flex",
justifyContent: "center",
alignItems: "center",
width: "40px",
height: "40px",
borderRadius: "50%",
backgroundColor: "#007bff",
color: "white",
fontSize: "24px",
cursor: "pointer",
boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)",
transition: "background-color 0.3s, transform 0.3s",
}}
- onMouseEnter={(e) => {
- e.target.style.transform = "scale(1.1)";
- }}
- onMouseLeave={(e) => {
- e.target.style.transform = "scale(1)";
- }}
+ sx={{
+ "&:hover": {
+ transform: "scale(1.1)"
+ }
+ }}
+ aria-label="Open audio diagnostic"
+ role="button"
>696-696: Verify state management and prop drilling.
The changes introduce new state (openTestModal) and props that are passed through multiple levels. Consider using React Context or a state management solution to avoid prop drilling.
Let's verify the component hierarchy and prop usage:
Also applies to: 832-832, 836-843, 895-895
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Assesment/Assesment.jsx(7 hunks)src/components/Assesment/AudioDiagnosticTool.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Assesment/AudioDiagnosticTool.jsx
🔇 Additional comments (3)
src/components/Assesment/Assesment.jsx (3)
47-47: LGTM!
The import statement for AudioDiagnosticTool is correctly added.
697-697: LGTM!
The state management and prop passing for the TestModal integration is implemented correctly.
Also applies to: 833-833, 837-844, 896-896
215-303: 🛠️ Refactor suggestion
Enhance modal accessibility and keyboard navigation.
The TestModal component needs accessibility improvements for better user experience.
Apply these improvements:
<Box
sx={{
display: "flex",
justifyContent: "center",
alignItems: "center",
width: "100vw",
height: "100vh",
position: "fixed",
background: "rgba(0, 0, 0, 0.5)",
zIndex: 9999,
+ role: "dialog",
+ "aria-modal": true,
+ "aria-labelledby": "audio-diagnostic-title"
}}
>
<Box
sx={{
width: "750px",
minHeight: "424px",
borderRadius: "20px",
display: "flex",
flexDirection: "column",
alignItems: "center",
backgroundImage: `url(${textureImage})`,
backgroundSize: "contain",
backgroundRepeat: "round",
boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)",
backdropFilter: "blur(25px)",
+ outline: "none",
+ tabIndex: -1
}}
+ onKeyDown={(e) => {
+ if (e.key === "Escape") {
+ setOpenTestModal(false);
+ }
+ }}
>Likely invalid or redundant comment.
| <div | ||
| style={{ | ||
| display: "flex", | ||
| flexDirection: "row", | ||
| alignItems: "center", | ||
| }} | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <SelectLanguageButton /> | ||
| <Box sx={{ position: "absolute", top: 9, left: 20 }}> | ||
| <span | ||
| <Box | ||
| mr={{ xs: "15px", sm: "15px" }} | ||
| onClick={() => | ||
| setOpenTestModal | ||
| ? setOpenTestModal(true) | ||
| : setOpenMessageDialog({ | ||
| message: "go to homescreen to change language", | ||
| dontShowHeader: true, | ||
| }) | ||
| } | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <div | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 700, | ||
| fontSize: { xs: "14px", sm: "16px" }, | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "25px", | ||
| display: "inline-flex", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| width: "40px", | ||
| height: "40px", | ||
| borderRadius: "50%", | ||
| backgroundColor: "#007bff", | ||
| color: "white", | ||
| fontSize: "24px", | ||
| cursor: "pointer", | ||
| boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", | ||
| transition: "background-color 0.3s, transform 0.3s", | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.target.style.transform = "scale(1.1)"; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.target.style.transform = "scale(1)"; | ||
| }} | ||
| role="button" | ||
| > | ||
| {languages?.find((elem) => elem.lang === language)?.name || | ||
| "Select Language"} | ||
| </span> | ||
| <span style={{ fontWeight: "bold", marginBottom: "2px" }}> | ||
| ? | ||
| </span> | ||
| </div> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| <Box | ||
| mr={{ xs: "10px", sm: "90px" }} | ||
| onClick={() => | ||
| setOpenLangModal | ||
| ? setOpenLangModal(true) | ||
| : setOpenMessageDialog({ | ||
| message: "go to homescreen to change language", | ||
| dontShowHeader: true, | ||
| }) | ||
| } | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <SelectLanguageButton /> | ||
| <Box sx={{ position: "absolute", top: 9, left: 20 }}> | ||
| <span | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 700, | ||
| fontSize: { xs: "14px", sm: "16px" }, | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "25px", | ||
| }} | ||
| > | ||
| {languages?.find((elem) => elem.lang === language)?.name || | ||
| "Select Language"} | ||
| </span> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| </div> |
There was a problem hiding this comment.
Fix assignment in event handlers and enhance button accessibility.
The current implementation uses assignments in expressions and lacks proper button accessibility.
Apply these improvements:
-onMouseEnter={(e) => {
- e.target.style.transform = "scale(1.1)";
-}}
-onMouseLeave={(e) => {
- e.target.style.transform = "scale(1)";
-}}
+sx={{
+ transform: "scale(1)",
+ transition: "transform 0.3s",
+ "&:hover": {
+ transform: "scale(1.1)"
+ }
+}}
+aria-label="Open audio diagnostic"
+tabIndex={0}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
src/components/Assesment/AudioDiagnosticTool.jsx (2)
30-40: Consider using useReducer for complex state management.The component manages multiple related states that could be consolidated using
useReducerto improve maintainability and state updates consistency.Consider refactoring the state management like this:
- const [isRecording, setIsRecording] = useState(false); - const [audioBlob, setAudioBlob] = useState(null); - const [audioUrl, setAudioUrl] = useState(null); - const [latencyData, setLatencyData] = useState([]); - const [latency, setLatency] = useState(null); - const [testResults, setTestResults] = useState([]); - const [testIndex, setTestIndex] = useState(0); - const [loading, setLoading] = useState(false); + const [state, dispatch] = useReducer(reducer, { + isRecording: false, + audioBlob: null, + audioUrl: null, + latencyData: [], + latency: null, + testResults: [], + testIndex: 0, + loading: false + });
175-178: Extract magic numbers into named constants.The noise threshold of 0.01 should be a named constant for better maintainability.
+const NOISE_THRESHOLD = 0.01; + const getNoiseLevel = (channelData) => { - const noise = channelData.filter((value) => Math.abs(value) < 0.01).length; + const noise = channelData.filter((value) => Math.abs(value) < NOISE_THRESHOLD).length; return noise / channelData.length; };
| const startRecording = () => { | ||
| //console.log('Starting recording...'); | ||
| audioChunksRef.current = []; | ||
| const stream = navigator.mediaDevices.getUserMedia({ audio: true }); | ||
|
|
||
| stream | ||
| .then((mediaStream) => { | ||
| mediaRecorderRef.current = new MediaRecorder(mediaStream); | ||
| mediaRecorderRef.current.ondataavailable = (event) => { | ||
| audioChunksRef.current.push(event.data); | ||
| }; | ||
| mediaRecorderRef.current.onstop = () => { | ||
| const audioBlob = new Blob(audioChunksRef.current, { | ||
| type: "audio/wav", | ||
| }); | ||
| setAudioBlob(audioBlob); | ||
| setAudioUrl(URL.createObjectURL(audioBlob)); | ||
| //console.log('Recording stopped, audioBlob set.'); | ||
| }; | ||
| mediaRecorderRef.current.start(); | ||
| setIsRecording(true); | ||
| setLatencyStart(Date.now()); | ||
| }) | ||
| .catch((err) => { | ||
| console.error("Error accessing audio devices:", err); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Add media stream cleanup to prevent resource leaks.
The media stream should be properly cleaned up when recording stops or component unmounts.
Add cleanup logic:
const startRecording = () => {
+ let mediaStream;
audioChunksRef.current = [];
const stream = navigator.mediaDevices.getUserMedia({ audio: true });
stream
.then((stream) => {
+ mediaStream = stream;
mediaRecorderRef.current = new MediaRecorder(mediaStream);
// ... rest of the code
})
.catch((err) => {
console.error("Error accessing audio devices:", err);
});
+
+ return () => {
+ if (mediaStream) {
+ mediaStream.getTracks().forEach(track => track.stop());
+ }
+ };
};Also add cleanup in useEffect:
useEffect(() => {
return () => {
if (mediaRecorderRef.current?.state === 'recording') {
stopRecording();
}
};
}, []);| <div onClick={startRecording} role="button" tabIndex="0"> | ||
| <img src={record} alt={"Record"} style={{ height: "50px" }} /> | ||
| </div> |
There was a problem hiding this comment.
Improve accessibility of click handlers.
The click handlers need proper keyboard handling and ARIA labels for better accessibility.
Update the click handlers:
-<div onClick={startRecording} role="button" tabIndex="0">
+<div
+ onClick={startRecording}
+ onKeyPress={(e) => e.key === 'Enter' && startRecording()}
+ role="button"
+ tabIndex="0"
+ aria-label="Start recording"
+>
<img src={record} alt={"Record"} style={{ height: "50px" }} />
</div>Also applies to: 446-448, 480-482
| const analyzeAudioBuffer = (audioBuffer) => { | ||
| const channelData = audioBuffer.getChannelData(0); // Assuming mono audio | ||
| const noiseLevel = getNoiseLevel(channelData); | ||
| const noiseLevelRating = normalizeNoiseLevel(noiseLevel); | ||
| const noiseLevelDescription = getDescription( | ||
| noiseLevelRating, | ||
| "noise level" | ||
| ); | ||
| return { | ||
| noiseLevel: noiseLevelRating, | ||
| noiseLevelDescription, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Add channel count validation in analyzeAudioBuffer.
The function assumes mono audio without validation, which could lead to errors with stereo input.
Add channel validation:
const analyzeAudioBuffer = (audioBuffer) => {
+ if (audioBuffer.numberOfChannels === 0) {
+ throw new Error('Invalid audio buffer: no channels found');
+ }
+ // For stereo, we could average both channels or use the first one
+ const channel = audioBuffer.numberOfChannels > 1 ?
+ averageChannels(audioBuffer) : audioBuffer.getChannelData(0);
const noiseLevel = getNoiseLevel(channelData);
// ... rest of the code
};Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Assesment/Assesment.jsx(7 hunks)
🔇 Additional comments (2)
src/components/Assesment/Assesment.jsx (2)
47-47: Verify AudioDiagnosticTool component existence.
Let's ensure the imported component exists in the specified location.
✅ Verification successful
Import statement for AudioDiagnosticTool component is valid
The component exists at the correct relative path src/components/Assesment/AudioDiagnosticTool.jsx, making the import statement valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if AudioDiagnosticTool component exists
fd "AudioDiagnosticTool.jsx" --type f
Length of output: 86
698-698: Verify prop drilling path for setOpenTestModal.
The implementation looks correct. Let's verify the prop drilling path for setOpenTestModal through MainLayout.
Also applies to: 834-834, 838-845, 897-897
✅ Verification successful
Let me gather more information about the prop drilling path by checking the ProfileHeader component and its usage.
Let me try a different approach to check the component hierarchy and prop usage.
Prop drilling path for setOpenTestModal is correctly implemented
The verification shows that the prop drilling path for setOpenTestModal is properly implemented:
- The state is initialized in
Assesment.jsxusinguseState - It's passed down to
ProfileHeadercomponent as a prop TestModalcomponent receives it directly- The prop is properly typed with PropTypes in
MainLayout
The implementation follows React's best practices for state management and prop drilling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check MainLayout component for setOpenTestModal prop
fd "MainLayout.jsx" --type f --exec rg -A 5 "setOpenTestModal"
Length of output: 452
Script:
#!/bin/bash
# Check ProfileHeader component and its usage
fd "ProfileHeader" --type f --exec rg -A 5 "setOpenTestModal"
# Check for TestModal usage
fd "TestModal" --type f --exec rg -A 5 "setOpenTestModal"
Length of output: 115
Script:
#!/bin/bash
# Search for ProfileHeader component definition and props
ast-grep --pattern 'const ProfileHeader = ({ $$$ }) => {
$$$
}'
# Search for TestModal component definition and props
ast-grep --pattern 'const TestModal = ({ $$$ }) => {
$$$
}'
# Search for any other usage of setOpenTestModal
rg "setOpenTestModal" -A 3
Length of output: 28342
| export const TestModal = ({ setOpenTestModal }) => { | ||
| return ( | ||
| <Box | ||
| sx={{ | ||
| display: "flex", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| width: "100vw", | ||
| height: "100vh", | ||
| position: "fixed", | ||
| background: "rgba(0, 0, 0, 0.5)", | ||
| zIndex: 9999, | ||
| }} | ||
| > | ||
| <Box | ||
| sx={{ | ||
| width: "750px", | ||
| minHeight: "424px", | ||
| borderRadius: "20px", | ||
| display: "flex", | ||
| flexDirection: "column", | ||
| alignItems: "center", | ||
| backgroundImage: `url(${textureImage})`, | ||
| backgroundSize: "contain", | ||
| backgroundRepeat: "round", | ||
| boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)", | ||
| backdropFilter: "blur(25px)", | ||
| }} | ||
| > | ||
| <Box | ||
| sx={{ width: "100%", display: "flex", justifyContent: "flex-end" }} | ||
| mt={2} | ||
| mb={1} | ||
| mr={3} | ||
| > | ||
| <Box | ||
| onClick={() => { | ||
| setOpenTestModal(false); | ||
| }} | ||
| sx={{ | ||
| cursor: "pointer", | ||
| background: "red", | ||
| width: "25px", | ||
| height: "25px", | ||
| borderRadius: "50%", | ||
| display: "flex", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| border: "2px solid black", // Add black border | ||
| }} | ||
| > | ||
| <span | ||
| style={{ | ||
| color: "#FFFFFF", | ||
| fontWeight: 600, | ||
| fontSize: "12px", | ||
| fontFamily: "Quicksand", | ||
| display: "flex", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| }} | ||
| > | ||
| X | ||
| </span> | ||
| </Box> | ||
| </Box> | ||
|
|
||
| <Box mt="32px"> | ||
| <span | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 600, | ||
| fontSize: "36px", | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "45px", | ||
| }} | ||
| > | ||
| {`Audio Diagnostic`} | ||
| </span> | ||
| </Box> | ||
| <Box sx={{ width: "100%", display: "flex", justifyContent: "center" }}> | ||
| <Box sx={{ display: "flex", justifyContent: "center" }}> | ||
| <AudioDiagnosticTool /> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance modal accessibility and error handling.
The modal implementation needs improvements in accessibility and error handling:
- Add ARIA attributes for modal dialog
- Improve close button accessibility
- Add error boundary for AudioDiagnosticTool
Apply these improvements:
export const TestModal = ({ setOpenTestModal }) => {
return (
<Box
sx={{
display: "flex",
justifyContent: "center",
alignItems: "center",
width: "100vw",
height: "100vh",
position: "fixed",
background: "rgba(0, 0, 0, 0.5)",
zIndex: 9999,
+ role: "dialog",
+ "aria-modal": true,
+ "aria-labelledby": "audio-diagnostic-title"
}}
>
<Box
sx={{
width: "750px",
minHeight: "424px",
borderRadius: "20px",
display: "flex",
flexDirection: "column",
alignItems: "center",
backgroundImage: `url(${textureImage})`,
backgroundSize: "contain",
backgroundRepeat: "round",
boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)",
backdropFilter: "blur(25px)",
+ outline: "none",
+ tabIndex: -1
}}
+ onKeyDown={(e) => {
+ if (e.key === "Escape") {
+ setOpenTestModal(false);
+ }
+ }}
>
<Box
sx={{ width: "100%", display: "flex", justifyContent: "flex-end" }}
mt={2}
mb={1}
mr={3}
>
<Box
onClick={() => {
setOpenTestModal(false);
}}
sx={{
cursor: "pointer",
background: "red",
width: "25px",
height: "25px",
borderRadius: "50%",
display: "flex",
justifyContent: "center",
alignItems: "center",
border: "2px solid black",
+ "&:focus": {
+ outline: "2px solid #000",
+ outlineOffset: "2px"
+ }
}}
+ role="button"
+ aria-label="Close audio diagnostic"
+ tabIndex={0}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ setOpenTestModal(false);
+ }
+ }}
>
<span
style={{
color: "#FFFFFF",
fontWeight: 600,
fontSize: "12px",
fontFamily: "Quicksand",
display: "flex",
alignItems: "center",
justifyContent: "center",
}}
>
X
</span>
</Box>
</Box>
<Box mt="32px">
<span
style={{
color: "#000000",
fontWeight: 600,
fontSize: "36px",
fontFamily: "Quicksand",
lineHeight: "45px",
}}
+ id="audio-diagnostic-title"
>
{`Audio Diagnostic`}
</span>
</Box>
<Box sx={{ width: "100%", display: "flex", justifyContent: "center" }}>
<Box sx={{ display: "flex", justifyContent: "center" }}>
+ <ErrorBoundary fallback={<div>Error loading audio diagnostic tool</div>}>
<AudioDiagnosticTool />
+ </ErrorBoundary>
</Box>
</Box>
</Box>
</Box>
);
};Committable suggestion skipped: line range outside the PR's diff.
| <div | ||
| style={{ | ||
| display: "flex", | ||
| flexDirection: "row", | ||
| alignItems: "center", | ||
| }} | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <SelectLanguageButton /> | ||
| <Box sx={{ position: "absolute", top: 9, left: 20 }}> | ||
| <span | ||
| <Box | ||
| mr={{ xs: "15px", sm: "15px" }} | ||
| onClick={() => | ||
| setOpenTestModal | ||
| ? setOpenTestModal(true) | ||
| : setOpenMessageDialog({ | ||
| message: "go to homescreen to change language", | ||
| dontShowHeader: true, | ||
| }) | ||
| } | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <div | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 700, | ||
| fontSize: { xs: "14px", sm: "16px" }, | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "25px", | ||
| display: "inline-flex", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| width: "40px", | ||
| height: "40px", | ||
| borderRadius: "50%", | ||
| backgroundColor: "#007bff", | ||
| color: "white", | ||
| fontSize: "24px", | ||
| cursor: "pointer", | ||
| boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", | ||
| transition: "background-color 0.3s, transform 0.3s", | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.target.style.transform = "scale(1.1)"; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.target.style.transform = "scale(1)"; | ||
| }} | ||
| role="button" | ||
| tabIndex="0" | ||
| > | ||
| {languages?.find((elem) => elem.lang === language)?.name || | ||
| "Select Language"} | ||
| </span> | ||
| <span style={{ fontWeight: "bold", marginBottom: "2px" }}> | ||
| ? | ||
| </span> | ||
| </div> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| <Box | ||
| mr={{ xs: "10px", sm: "90px" }} | ||
| onClick={() => | ||
| setOpenLangModal | ||
| ? setOpenLangModal(true) | ||
| : setOpenMessageDialog({ | ||
| message: "go to homescreen to change language", | ||
| dontShowHeader: true, | ||
| }) | ||
| } | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <SelectLanguageButton /> | ||
| <Box sx={{ position: "absolute", top: 9, left: 20 }}> | ||
| <span | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 700, | ||
| fontSize: { xs: "14px", sm: "16px" }, | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "25px", | ||
| }} | ||
| > | ||
| {languages?.find((elem) => elem.lang === language)?.name || | ||
| "Select Language"} | ||
| </span> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test modal button implementation.
The test modal button needs improvements:
- Replace inline style manipulations with MUI's sx prop
- Enhance button accessibility
Apply these improvements:
<div
style={{
display: "inline-flex",
justifyContent: "center",
alignItems: "center",
width: "40px",
height: "40px",
borderRadius: "50%",
backgroundColor: "#007bff",
color: "white",
fontSize: "24px",
cursor: "pointer",
boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)",
- transition: "background-color 0.3s, transform 0.3s",
}}
- onMouseEnter={(e) => {
- e.target.style.transform = "scale(1.1)";
- }}
- onMouseLeave={(e) => {
- e.target.style.transform = "scale(1)";
- }}
+ sx={{
+ transform: "scale(1)",
+ transition: "transform 0.3s",
+ "&:hover": {
+ transform: "scale(1.1)"
+ },
+ "&:focus": {
+ outline: "2px solid #000",
+ outlineOffset: "2px"
+ }
+ }}
role="button"
- tabIndex="0"
+ tabIndex={0}
+ aria-label="Open audio diagnostic"
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ setOpenTestModal(true);
+ }
+ }}
>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Assesment/Assesment.jsx(7 hunks)src/components/Assesment/AudioDiagnosticTool.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Assesment/AudioDiagnosticTool.jsx
🔇 Additional comments (2)
src/components/Assesment/Assesment.jsx (2)
703-703: LGTM!
The state management and prop passing for the test modal functionality is implemented correctly.
Also applies to: 839-839, 843-850, 902-902
215-303: 🛠️ Refactor suggestion
Enhance modal accessibility and error handling.
The modal implementation needs improvements in accessibility and error handling:
- Add ARIA attributes for modal dialog
- Improve close button accessibility
- Add error boundary for AudioDiagnosticTool
Apply these improvements:
export const TestModal = ({ setOpenTestModal }) => {
return (
<Box
sx={{
display: "flex",
justifyContent: "center",
alignItems: "center",
width: "100vw",
height: "100vh",
position: "fixed",
background: "rgba(0, 0, 0, 0.5)",
zIndex: 9999,
+ role: "dialog",
+ "aria-modal": true,
+ "aria-labelledby": "audio-diagnostic-title"
}}
>
<Box
sx={{
width: "750px",
minHeight: "424px",
borderRadius: "20px",
display: "flex",
flexDirection: "column",
alignItems: "center",
backgroundImage: `url(${textureImage})`,
backgroundSize: "contain",
backgroundRepeat: "round",
boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)",
backdropFilter: "blur(25px)",
+ outline: "none",
+ tabIndex: -1
}}
+ onKeyDown={(e) => {
+ if (e.key === "Escape") {
+ setOpenTestModal(false);
+ }
+ }}
>
<Box
sx={{ width: "100%", display: "flex", justifyContent: "flex-end" }}
mt={2}
mb={1}
mr={3}
>
<Box
onClick={() => {
setOpenTestModal(false);
}}
sx={{
cursor: "pointer",
background: "red",
width: "25px",
height: "25px",
borderRadius: "50%",
display: "flex",
justifyContent: "center",
alignItems: "center",
border: "2px solid black",
+ "&:focus": {
+ outline: "2px solid #000",
+ outlineOffset: "2px"
+ }
}}
+ role="button"
+ aria-label="Close audio diagnostic"
+ tabIndex={0}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ setOpenTestModal(false);
+ }
+ }}
>
<span
style={{
color: "#FFFFFF",
fontWeight: 600,
fontSize: "12px",
fontFamily: "Quicksand",
display: "flex",
alignItems: "center",
justifyContent: "center",
}}
>
X
</span>
</Box>
</Box>
<Box mt="32px">
<span
style={{
color: "#000000",
fontWeight: 600,
fontSize: "36px",
fontFamily: "Quicksand",
lineHeight: "45px",
}}
+ id="audio-diagnostic-title"
>
{`Audio Diagnostic`}
</span>
</Box>
<Box sx={{ width: "100%", display: "flex", justifyContent: "center" }}>
<Box sx={{ display: "flex", justifyContent: "center" }}>
+ <ErrorBoundary fallback={<div>Error loading audio diagnostic tool</div>}>
<AudioDiagnosticTool />
+ </ErrorBoundary>
</Box>
</Box>
</Box>
</Box>
);
};Likely invalid or redundant comment.
| <div | ||
| style={{ | ||
| display: "flex", | ||
| flexDirection: "row", | ||
| alignItems: "center", | ||
| }} | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <SelectLanguageButton /> | ||
| <Box sx={{ position: "absolute", top: 9, left: 20 }}> | ||
| <span | ||
| <Box | ||
| mr={{ xs: "15px", sm: "15px" }} | ||
| onClick={() => | ||
| setOpenTestModal | ||
| ? setOpenTestModal(true) | ||
| : setOpenMessageDialog({ | ||
| message: "go to homescreen to change language", | ||
| dontShowHeader: true, | ||
| }) | ||
| } | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <div | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 700, | ||
| fontSize: { xs: "14px", sm: "16px" }, | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "25px", | ||
| display: "inline-flex", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| width: "40px", | ||
| height: "40px", | ||
| borderRadius: "50%", | ||
| backgroundColor: "#007bff", | ||
| color: "white", | ||
| fontSize: "24px", | ||
| cursor: "pointer", | ||
| boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", | ||
| transition: "background-color 0.3s, transform 0.3s", | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.target.style.transform = "scale(1.1)"; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.target.style.transform = "scale(1)"; | ||
| }} | ||
| role="button" | ||
| tabIndex="0" | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") { | ||
| e.target.style.transform = "scale(1)"; | ||
| } | ||
| }} | ||
| > | ||
| {languages?.find((elem) => elem.lang === language)?.name || | ||
| "Select Language"} | ||
| </span> | ||
| <span style={{ fontWeight: "bold", marginBottom: "2px" }}> | ||
| ? | ||
| </span> | ||
| </div> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| <Box | ||
| mr={{ xs: "10px", sm: "90px" }} | ||
| onClick={() => | ||
| setOpenLangModal | ||
| ? setOpenLangModal(true) | ||
| : setOpenMessageDialog({ | ||
| message: "go to homescreen to change language", | ||
| dontShowHeader: true, | ||
| }) | ||
| } | ||
| > | ||
| <Box sx={{ position: "relative", cursor: "pointer" }}> | ||
| <SelectLanguageButton /> | ||
| <Box sx={{ position: "absolute", top: 9, left: 20 }}> | ||
| <span | ||
| style={{ | ||
| color: "#000000", | ||
| fontWeight: 700, | ||
| fontSize: { xs: "14px", sm: "16px" }, | ||
| fontFamily: "Quicksand", | ||
| lineHeight: "25px", | ||
| }} | ||
| > | ||
| {languages?.find((elem) => elem.lang === language)?.name || | ||
| "Select Language"} | ||
| </span> | ||
| </Box> | ||
| </Box> | ||
| </Box> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test modal button implementation.
The test modal button needs improvements:
- Replace inline style manipulations with MUI's sx prop
- Enhance button accessibility
Apply these improvements:
<div
style={{
display: "inline-flex",
justifyContent: "center",
alignItems: "center",
width: "40px",
height: "40px",
borderRadius: "50%",
backgroundColor: "#007bff",
color: "white",
fontSize: "24px",
cursor: "pointer",
boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)",
- transition: "background-color 0.3s, transform 0.3s",
}}
- onMouseEnter={(e) => {
- e.target.style.transform = "scale(1.1)";
- }}
- onMouseLeave={(e) => {
- e.target.style.transform = "scale(1)";
- }}
+ sx={{
+ transform: "scale(1)",
+ transition: "transform 0.3s",
+ "&:hover": {
+ transform: "scale(1.1)"
+ },
+ "&:focus": {
+ outline: "2px solid #000",
+ outlineOffset: "2px"
+ }
+ }}
role="button"
- tabIndex="0"
+ tabIndex={0}
+ aria-label="Open audio diagnostic"
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ setOpenTestModal(true);
+ }
+ }}
>Committable suggestion skipped: line range outside the PR's diff.
|
gouravmore
changed the base branch from
all-1.3-test-rig-dev
to
all-1.3-tn-dev-hotfix
voice-correct-logic
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
VoiceAnalysercomponent to include new properties for better clarity.MainLayoutcomponent to include the newsetOpenTestModalprop.