All 1.3.1 dev #249
All 1.3.1 dev #249
Conversation
Issueid #231942 feat: audio delay right after completing level in pra…
Changing span into typography
…' into release-1.3-apicall-optimisation
Issueid #231949 feat: Show loading until next content is not loaded
…tion Release 1.3 apicall optimisation
All 1.3 feedback change
#00000 zoom icon visble event after image is not there
All 1.3 feedback change
All 1.3 feedback change
Issueid #232740 fix: While switching the iframe telemetry events are …
All 1.3 feedback change
…e lost and earn], fill in the blanks and jumble the words mechanism
Issueid #232843 fix: Audio sync issue for low network - Showcase [liv…
…29 Triggered When Not Required
Issueid #232821 fix: Unexpected API Call: /pointer/getPoints/43477152…
All 1.3 feedback change
|
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
src/App.jsOops! 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. WalkthroughThis pull request introduces several key modifications across the project, primarily focusing on audio preloading, telemetry service improvements, and workflow configuration updates. The changes span multiple files, including GitHub Actions workflows, component logic, and utility functions. The most significant additions include a new Changes
Possibly related PRs
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: 6
🔭 Outside diff range comments (2)
.github/workflows/all-dev-tn.yml (1)
Line range hint
89-93: Security: Remove debug printing of sensitive informationThe debug step prints environment variables that might contain sensitive information. While the actual secret values are not exposed (due to GitHub's secret masking), it's best practice to avoid debugging sensitive variable names in CI logs.
Consider removing or modifying the debug step:
- - name: Debug Environment Variables - run: | - echo "REACT_APP_AWS_S3_BUCKET_NAME: $REACT_APP_AWS_S3_BUCKET_NAME" - echo "AWS_REGION: $AWS_REGION" - echo "secrate": ${{ vars.REACT_APP_AWS_S3_BUCKET_NAME }} + - name: Verify Deployment Configuration + run: | + echo "Deploying to region: $AWS_REGION"src/components/Practice/Mechanics3.jsx (1)
Line range hint
268-309: Improve image handling logic.Consider the following improvements:
- The image path validation is fragile.
- The gradient overlay styles are duplicated from Mechanics5.jsx.
Extract the common image handling logic into a shared component to avoid duplication:
// src/components/common/ImageWithZoom.jsx const ImageWithZoom = ({ image, onZoom }) => { const isValidImagePath = (imagePath) => { try { return imagePath && new URL(imagePath).pathname.split('/').length > 4; } catch { return false; } }; const gradientOverlayStyles = { position: "absolute", top: 0, left: 0, right: 0, height: "40px", background: "linear-gradient(to bottom, rgba(0, 0, 0, 0.4), transparent)", borderTopLeftRadius: "20px", borderTopRightRadius: "20px", display: "flex", alignItems: "center", paddingLeft: "8px", }; if (!isValidImagePath(image)) { return null; } return ( <Box sx={{ position: "relative", cursor: "zoom-in" }}> <img onClick={onZoom} src={image} style={{ borderRadius: "20px", maxWidth: "100%", height: "clamp(150px, 20vw, 220px)", }} alt="content" /> <Box sx={gradientOverlayStyles}> <ZoomInIcon onClick={onZoom} sx={{ color: "white", fontSize: "22px", cursor: "pointer", }} /> </Box> </Box> ); };
🧹 Nitpick comments (10)
.gitignore (1)
13-13: Clean up redundant .gitignore entriesThe new
*.env*pattern makes the existing.enventry redundant as it will match all environment files. Additionally, thebuilddirectory appears twice in the file.Consider cleaning up the file by:
- Removing the redundant
.enventry- Keeping only one
buildentry- Adding comments to explain what the
*.env*pattern covers-#env -.env -#build -build +# Environment files +*.env* +# Build output +buildsrc/App.js (1)
77-77: Simplify conditional call using optional chainingThe conditional check can be simplified using optional chaining for better readability.
Apply this diff:
- window.telemetry && window.telemetry.syncEvents && window.telemetry.syncEvents(); + window.telemetry?.syncEvents?.();🧰 Tools
🪛 Biome (1.9.4)
[error] 77-77: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/Practice/Mechanics4.jsx (1)
122-122: Consider consolidating audio playback logic.The audio playback logic is scattered. Consider creating a dedicated audio manager.
+const playAudioEffect = (audioSrc) => { + if (audioSrc) { + const audio = new Audio(audioSrc); + audio.play(); + } +}; -let audio = new Audio(isSelected ? removeSoundAudio : addSoundAudio); -audio.play(); +playAudioEffect(isSelected ? removeSoundAudio : addSoundAudio);Also applies to: 142-143
src/components/AssesmentEnd/AssesmentEnd.jsx (1)
47-56: Improve session ID handling and pointer details fetching.Consider the following improvements:
- The double negative condition
!(localStorage.getItem("contentSessionId") !== null)is hard to read.- The session ID initialization could be moved to a separate function for better reusability.
Apply this diff to improve readability:
-if (!sessionId) { - sessionId = uniqueId(); - localStorage.setItem("sessionId", sessionId); -} -if (process.env.REACT_APP_IS_APP_IFRAME !== "true" && localStorage.getItem("contentSessionId") !== null) { +const initializeSessionId = () => { + if (!sessionId) { + sessionId = uniqueId(); + localStorage.setItem("sessionId", sessionId); + } + return sessionId; +}; + +sessionId = initializeSessionId(); + +const shouldFetchPointerDetails = process.env.REACT_APP_IS_APP_IFRAME !== "true" && + localStorage.getItem("contentSessionId") === null; + +if (shouldFetchPointerDetails) {src/components/DiscoverSentance/DiscoverSentance.jsx (3)
44-47: Improve telemetry sync handling.The telemetry sync should be more robust by handling potential errors and ensuring the sync completes.
Apply this diff to improve error handling:
-let audio = new Audio(levelCompleteAudioSrc); -audio.play(); -callConfetti(); -window.telemetry?.syncEvents && window.telemetry.syncEvents(); +const playAudioAndSync = async () => { + try { + const audio = new Audio(levelCompleteAudioSrc); + await audio.play(); + callConfetti(); + if (window.telemetry?.syncEvents) { + await window.telemetry.syncEvents(); + } + } catch (error) { + console.error('Error playing audio or syncing telemetry:', error); + } +}; + +playAudioAndSync();🧰 Tools
🪛 Biome (1.9.4)
[error] 47-47: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
125-136: Remove commented out code.If this code is no longer needed, it should be removed. If it might be needed later, consider adding a TODO comment explaining why it's kept.
155-170: Improve session handling logic.The session handling logic could be more readable and maintainable:
- The double negative condition is hard to read.
- The points handling logic could be extracted into a separate function.
Apply this diff to improve readability:
-if (!(localStorage.getItem("contentSessionId") !== null)) { +const handlePoints = async (lang) => { + const isExternalContent = localStorage.getItem("contentSessionId") !== null; + + if (!isExternalContent) { + const pointsRes = await axios.post( + `${process.env.REACT_APP_LEARNER_AI_ORCHESTRATION_HOST}/${config.URLS.ADD_POINTER}`, + { + userId: localStorage.getItem("virtualId"), + sessionId: localStorage.getItem("sessionId"), + points: 1, + language: lang, + milestone: "m0", + } + ); + setPoints(pointsRes?.data?.result?.totalLanguagePoints || 0); + } else { + send(5); + } +}; + +await handlePoints(lang);src/components/Practice/Mechanics5.jsx (1)
188-223: Improve image handling logic.Consider the following improvements:
- The image path validation
image?.split("/")?.[4]is fragile and relies on a specific path format.- The gradient overlay styles could be extracted into a reusable constant.
Apply this diff to improve maintainability:
+const isValidImagePath = (imagePath) => { + try { + return imagePath && new URL(imagePath).pathname.split('/').length > 4; + } catch { + return false; + } +}; +const gradientOverlayStyles = { + position: "absolute", + top: 0, + left: 0, + right: 0, + height: "40px", + background: "linear-gradient(to bottom, rgba(0, 0, 0, 0.4), transparent)", + borderTopLeftRadius: "20px", + borderTopRightRadius: "20px", + display: "flex", + alignItems: "center", + paddingLeft: "8px", +}; -{image?.split("/")?.[4] && ( +{isValidImagePath(image) && ( <Box sx={{ position: "relative", cursor: "zoom-in" }}> <img src={image} style={{ borderRadius: "20px", maxWidth: "100%", height: "250px", }} alt="contentImage" onClick={() => setZoomOpen(true)} /> <Box - sx={{ - position: "absolute", - top: 0, - left: 0, - right: 0, - height: "40px", - background: "linear-gradient(to bottom, rgba(0, 0, 0, 0.4), transparent)", - borderTopLeftRadius: "20px", - borderTopRightRadius: "20px", - display: "flex", - alignItems: "center", - paddingLeft: "8px", - }} + sx={gradientOverlayStyles} >src/components/Practice/Mechanics3.jsx (1)
92-93: Improve audio playback error handling.The audio playback should handle potential errors that may occur during playback.
Apply this diff to improve error handling:
-let audio = new Audio(isSoundCorrect ? correctSoundAudio : wrongSoundAudio); -audio.play(); +const playSound = async (soundSrc) => { + try { + const audio = new Audio(soundSrc); + await audio.play(); + } catch (error) { + console.error('Error playing sound:', error); + } +}; + +await playSound(isSoundCorrect ? correctSoundAudio : wrongSoundAudio); -let audio = new Audio(removeSoundAudio); -audio.play(); +await playSound(removeSoundAudio);Also applies to: 104-105
src/index.css (1)
1-9: Consider using transform for better performance.While the current implementation works, using transform for animations provides better performance as it triggers GPU acceleration:
.loader { width: 250px; height: 20px; margin-top: 80px; border-radius: 20px; background: - linear-gradient(rgb(80, 216, 1) 0 0) 0/0% no-repeat lightblue; + linear-gradient(rgb(80, 216, 1) 0 0) 0/0% no-repeat lightblue; - animation: l2 1s infinite steps(10); + animation: l2 1s infinite steps(10); + transform: translateZ(0); + will-change: background-size; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/workflows/all-dev-tn-new.yml(1 hunks).github/workflows/all-dev-tn.yml(1 hunks).gitignore(1 hunks)src/App.js(2 hunks)src/components/Assesment/Assesment.jsx(2 hunks)src/components/AssesmentEnd/AssesmentEnd.jsx(2 hunks)src/components/DiscoverEnd/DiscoverEnd.jsx(4 hunks)src/components/DiscoverSentance/DiscoverSentance.jsx(4 hunks)src/components/Layouts.jsx/MainLayout.jsx(3 hunks)src/components/Practice/Mechanics3.jsx(6 hunks)src/components/Practice/Mechanics4.jsx(4 hunks)src/components/Practice/Mechanics5.jsx(1 hunks)src/hooks/usePreloadAudio.jsx(1 hunks)src/index.css(1 hunks)src/services/telementryService.js(3 hunks)src/utils/AudioCompare.js(7 hunks)src/utils/VoiceAnalyser.js(6 hunks)src/views/Practice/Practice.jsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/index.css (1)
Pattern **/*.css: "Review the CSS code against the google css style guide and point out any mismatches. Ensure that:
- The code adheres to best practices associated with CSS.
- The code adheres to best practices recommended by lighthouse or similar tools for performance.
- The code adheres to similar naming conventions for classes, ids."
🪛 Biome (1.9.4)
src/App.js
[error] 77-77: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/DiscoverSentance/DiscoverSentance.jsx
[error] 47-47: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/views/Practice/Practice.jsx
[error] 80-80: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (27)
.github/workflows/all-dev-tn.yml (1)
6-6: LGTM: Branch name updated for version 1.3.1The branch name has been correctly updated to reflect the new version.
.github/workflows/all-dev-tn-new.yml (3)
6-6: LGTM: Branch name updated for feedback changesThe branch name has been appropriately updated from hotfix to feedback changes.
Line range hint
89-93: Security: Remove debug printing of sensitive informationSimilar to the other workflow file, this debug step should be removed or modified to avoid logging sensitive information.
Line range hint
1-104: Verify deployment configuration differencesThe workflow deploys to a different S3 bucket and uses a different CloudFront distribution compared to
all-dev-tn.yml. Please verify that these differences are intentional.Run this script to compare the deployment configurations:
✅ Verification successful
Deployment configuration differences are intentional
The new workflow deploys to a development-specific S3 bucket (
all-tn-app-dev) and uses a dedicated CloudFront distribution, which appears to be a deliberate configuration for the new deployment pipeline.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare deployment configurations between workflows echo "Comparing S3 bucket paths:" rg -A 1 "Deploy to S3 Bucket" .github/workflows/all-dev-tn*.yml echo "Comparing CloudFront distributions:" rg -A 4 "Cloudfront Invalidation" .github/workflows/all-dev-tn*.ymlLength of output: 1423
src/App.js (1)
76-79: Ensure telemetry events are reliably sent before page unloadAsynchronous operations in the
beforeunloadevent may not complete before the page is unloaded, potentially causing telemetry data to be lost.Consider verifying if
window.telemetry.syncEvents()completes execution before the unload event finishes. If not, you might need to use thenavigator.sendBeaconAPI for reliable data transmission during page unload.🧰 Tools
🪛 Biome (1.9.4)
[error] 77-77: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/DiscoverEnd/DiscoverEnd.jsx (3)
32-32: LGTM! Audio preloading implementation.The use of
usePreloadAudiohook improves reliability by ensuring the audio is ready before playback.
134-134: LGTM! Event handler optimization.Direct reference to
handleProfileBackimproves code clarity and performance.
36-39: Verify audio playback handling.The audio playback is now conditional on
levelCompleteAudioSrcbeing available, which is good. However, consider cleaning up the Audio instance after playback to prevent memory leaks.if (levelCompleteAudioSrc) { let audio = new Audio(levelCompleteAudioSrc); + audio.addEventListener('ended', () => { + audio = null; + }); audio.play(); }src/utils/AudioCompare.js (3)
16-16: LGTM! Loading state management.Adding
showLoaderstate improves user feedback during recording operations.
215-220: LGTM! PropTypes validation.Good addition of PropTypes for type safety and documentation.
58-80: Review the timeout duration for recording cleanup.The 500ms timeout for cleanup might be too short for some devices or browsers. Consider making this configurable or increasing it to ensure proper cleanup.
- }, 500); + const CLEANUP_TIMEOUT = 1000; // 1 second + }, CLEANUP_TIMEOUT);src/components/Practice/Mechanics4.jsx (1)
55-58: LGTM! Audio preloading implementation.Good use of
usePreloadAudiohook for all sound effects improves performance and reliability.src/components/AssesmentEnd/AssesmentEnd.jsx (2)
19-21: LGTM!The imports are correctly ordered and necessary for the functionality.
28-28: LGTM! Audio preloading improves performance.The use of
usePreloadAudiohook and conditional audio playback is a good improvement for better performance and reliability.Also applies to: 32-35
src/components/DiscoverSentance/DiscoverSentance.jsx (1)
17-17: LGTM! Audio preloading setup is correct.The import and usage of
usePreloadAudiohook for level completion audio is properly implemented.Also applies to: 41-42
src/components/Practice/Mechanics3.jsx (1)
21-21: LGTM! Audio preloading setup is correct.The import and usage of
usePreloadAudiohook for multiple sound effects is properly implemented.Also applies to: 68-70
src/utils/VoiceAnalyser.js (4)
36-36: LGTM!The import statement is correctly placed and follows the established import order.
67-67: LGTM!The state variable is correctly initialized using the useState hook.
81-82: LGTM!The audio files are correctly preloaded using the usePreloadAudio hook, which improves performance by loading the audio files in advance.
286-291: LGTM!The useEffect hook correctly manages the enableAfterLoad state based on props.originalText changes.
src/components/Assesment/Assesment.jsx (2)
601-606: LGTM!The pointer details are correctly fetched only when both conditions are met:
- Application is not running in an iframe
- contentSessionId exists in localStorage
Line range hint
640-645: LGTM!The pointer details are correctly fetched only when all conditions are met:
- Application is not running in an iframe
- virtualId exists
- contentSessionId exists in localStorage
src/index.css (1)
11-15: LGTM!The keyframes are correctly defined for the loader animation.
src/views/Practice/Practice.jsx (4)
25-25: LGTM!The import statement is correctly placed and follows the established import order.
74-75: LGTM!The level complete audio is correctly preloaded using the usePreloadAudio hook.
216-235: LGTM!The point assignment logic correctly handles different scenarios:
- When contentSessionId exists: assigns 1 point and sends score 5 for showcase
- Otherwise: makes an API call to add points and updates the state with the response
77-80: 🛠️ Refactor suggestionUse preloaded audio directly instead of creating a new Audio instance.
Creating a new Audio instance defeats the purpose of preloading. Use the preloaded audio directly:
-const audio = new Audio(levelCompleteAudioSrc); -audio.play(); +if (levelCompleteAudioSrc) { + levelCompleteAudioSrc.play(); +} callConfetti(); window.telemetry?.syncEvents && window.telemetry.syncEvents();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 80-80: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| @@ -752,7 +795,7 @@ const MainLayout = (props) => { | |||
| > | |||
| <Stack justifyContent="center" alignItems="center"> | |||
| <img | |||
| src={gameLost} | |||
| src={`https://raw.githubusercontent.com/Sunbird-ALL/all-learner-ai-app/refs/heads/all-1.3/src/assets/images/gameLost.svg`} | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded image URL with imported asset
The src attribute is using a hardcoded URL pointing to GitHub's raw content, which is not optimal for production use. This can lead to performance issues and potential broken links if the repository changes.
Apply this diff to replace the hardcoded URL with an imported asset:
+ import gameLost from "../../assets/images/gameLost.svg";
...
- src={`https://raw.githubusercontent.com/Sunbird-ALL/all-learner-ai-app/refs/heads/all-1.3/src/assets/images/gameLost.svg`}
+ src={gameLost}Committable suggestion skipped: line range outside the PR's diff.
| return () => { | ||
| Object.values(audioCache).forEach((audioUrl) => | ||
| URL.revokeObjectURL(audioUrl) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Fix potential memory leak due to stale audioCache in cleanup function
The cleanup function in the useEffect depends on audioCache, but since audioCache is not included in the dependency array, the cleanup function may reference a stale value of audioCache. This can lead to the preloaded audio URLs not being revoked properly, resulting in memory leaks.
Apply this diff to fix the issue by using a ref to store the audioCache, ensuring the cleanup function has access to the latest values:
+ const audioCacheRef = useRef({});
...
useEffect(() => {
const preloadAudio = async () => {
try {
const urls = [LevelCompleteAudio, gameLoseAudio];
const cache = {};
for (const url of urls) {
const response = await fetch(url);
const audioBlob = await response.blob();
const audioUrl = URL.createObjectURL(audioBlob);
cache[url] = audioUrl;
}
setAudioCache(cache);
+ audioCacheRef.current = cache;
} catch (error) {
console.error("Error preloading audio:", error);
}
};
preloadAudio();
// Cleanup cached audio URLs on unmount
return () => {
- Object.values(audioCache).forEach((audioUrl) =>
+ Object.values(audioCacheRef.current).forEach((audioUrl) =>
URL.revokeObjectURL(audioUrl)
);
};
}, []);Committable suggestion skipped: line range outside the PR's diff.
| return () => { | ||
| if (audioSrc) { | ||
| URL.revokeObjectURL(audioSrc); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Ensure proper cleanup of preloaded audio to prevent memory leaks
The cleanup function may not revoke the correct audioSrc if audioUrl changes rapidly because it relies on the current state of audioSrc, which might have changed. This can lead to memory leaks due to unreleased object URLs.
To fix this issue, capture the current audioSrc within the effect, so the cleanup function references the correct URL:
useEffect(() => {
+ let currentAudioSrc = null;
const preloadAudio = async () => {
try {
const response = await fetch(audioUrl);
const audioBlob = await response.blob();
const audioObjectUrl = URL.createObjectURL(audioBlob);
setAudioSrc(audioObjectUrl);
+ currentAudioSrc = audioObjectUrl;
} catch (error) {
console.error("Error preloading audio:", error);
}
};
preloadAudio();
return () => {
- if (audioSrc) {
- URL.revokeObjectURL(audioSrc);
+ if (currentAudioSrc) {
+ URL.revokeObjectURL(currentAudioSrc);
}
};
}, [audioUrl]);Committable suggestion skipped: line range outside the PR's diff.
| @@ -3,6 +3,7 @@ import { CsTelemetryModule } from '@project-sunbird/client-services/telemetry'; | |||
| import { uniqueId } from './utilService'; | |||
| import { jwtDecode } from '../../node_modules/jwt-decode/build/cjs/index'; | |||
|
|
|||
| let startTime; // Variable to store the timestamp when the start event is raised | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more robust timestamp storage.
Using a global variable for startTime could lead to race conditions in concurrent scenarios.
-let startTime;
+const timeStore = {
+ startTime: null,
+ setStartTime(time) {
+ this.startTime = time;
+ },
+ getStartTime() {
+ return this.startTime;
+ }
+};📝 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.
| let startTime; // Variable to store the timestamp when the start event is raised | |
| const timeStore = { | |
| startTime: null, | |
| setStartTime(time) { | |
| this.startTime = time; | |
| }, | |
| getStartTime() { | |
| return this.startTime; | |
| } | |
| }; // Variable to store the timestamp when the start event is raised |
| try { | ||
| const endTime = Date.now(); // Record the end time | ||
| const duration = ((endTime - startTime) / 1000).toFixed(2); // Calculate duration in seconds | ||
|
|
||
| CsTelemetryModule.instance.telemetryService.raiseEndTelemetry({ | ||
| edata: { | ||
| type: 'content', | ||
| mode: 'play', | ||
| pageid: url, | ||
| summary: data?.summary || {}, | ||
| duration: duration, // Log the calculated duration | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| console.error("Error in end telemetry event:", error); | ||
| } |
There was a problem hiding this comment.
Improve error handling and duration calculation.
The duration calculation is now more accurate, but there's no validation for missing startTime.
try {
const endTime = Date.now();
+ if (!startTime) {
+ console.warn('Start time not set, using fallback duration');
+ return;
+ }
const duration = ((endTime - startTime) / 1000).toFixed(2);📝 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.
| try { | |
| const endTime = Date.now(); // Record the end time | |
| const duration = ((endTime - startTime) / 1000).toFixed(2); // Calculate duration in seconds | |
| CsTelemetryModule.instance.telemetryService.raiseEndTelemetry({ | |
| edata: { | |
| type: 'content', | |
| mode: 'play', | |
| pageid: url, | |
| summary: data?.summary || {}, | |
| duration: duration, // Log the calculated duration | |
| }, | |
| }); | |
| } catch (error) { | |
| console.error("Error in end telemetry event:", error); | |
| } | |
| try { | |
| const endTime = Date.now(); | |
| if (!startTime) { | |
| console.warn('Start time not set, using fallback duration'); | |
| return; | |
| } | |
| const duration = ((endTime - startTime) / 1000).toFixed(2); // Calculate duration in seconds | |
| CsTelemetryModule.instance.telemetryService.raiseEndTelemetry({ | |
| edata: { | |
| type: 'content', | |
| mode: 'play', | |
| pageid: url, | |
| summary: data?.summary || {}, | |
| duration: duration, // Log the calculated duration | |
| }, | |
| }); | |
| } catch (error) { | |
| console.error("Error in end telemetry event:", error); | |
| } |
| @@ -620,7 +631,7 @@ function VoiceAnalyser(props) { | |||
| } else { | |||
| isLiveLost = false; | |||
| } | |||
| const audio = new Audio(isLiveLost ? livesCut : livesAdd); | |||
| const audio = new Audio(isLiveLost ? livesCutAudio : livesAddAudio); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use preloaded audio directly instead of creating a new Audio instance.
Creating a new Audio instance defeats the purpose of preloading. Use the preloaded audio directly:
-const audio = new Audio(isLiveLost ? livesCutAudio : livesAddAudio);
-audio.play();
+const audioToPlay = isLiveLost ? livesCutAudio : livesAddAudio;
+if (audioToPlay) {
+ audioToPlay.play();
+}Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
.gitignoreto handle environment variable filesDocumentation