-
Notifications
You must be signed in to change notification settings - Fork 26
Issueid #232034 fix: Sometimes it take time to load icons #237
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.
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
base: all-1.3-feedback-change
Are you sure you want to change the base?
Changes from 7 commits
5b84a39
b92f545
554a072
cfca254
bcefb1e
36fe010
7be4324
a0a667d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
import { useEffect } from "react"; | ||
|
||
export function useMediaCache(dbName = "MediaCacheDB", storeName = "media") { | ||
useEffect(() => { | ||
initializeDB(dbName, storeName); | ||
}, [dbName, storeName]); | ||
|
||
const initializeDB = (dbName, storeName) => { | ||
const openDB = indexedDB.open(dbName, 1); | ||
|
||
openDB.onupgradeneeded = () => { | ||
const db = openDB.result; | ||
if (!db.objectStoreNames.contains(storeName)) { | ||
db.createObjectStore(storeName, { keyPath: "key" }); | ||
} | ||
}; | ||
|
||
openDB.onerror = (e) => { | ||
console.error("IndexedDB initialization error:", e.target.error); | ||
}; | ||
}; | ||
|
||
const saveMedia = async (key, blob) => { | ||
const db = await openDatabase(dbName); | ||
return await putMedia(db, storeName, { key, blob }); | ||
}; | ||
|
||
const getMedia = async (key) => { | ||
const db = await openDatabase(dbName); | ||
return await fetchMedia(db, storeName, key); | ||
}; | ||
|
||
const cacheMedia = async (key, url) => { | ||
try { | ||
const cachedBlob = await getMedia(key); | ||
if (cachedBlob) { | ||
return URL.createObjectURL(cachedBlob); | ||
} | ||
return await fetchAndCacheMedia(key, url); | ||
} catch (error) { | ||
console.error(`Error caching media for key "${key}":`, error); | ||
throw error; | ||
} | ||
}; | ||
Comment on lines
+33
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prevent memory leaks and add cache management The caching implementation has potential memory leak issues and lacks important safeguards:
Apply these critical fixes: const cacheMedia = async (key, url) => {
try {
const cachedBlob = await getMedia(key);
if (cachedBlob) {
- return URL.createObjectURL(cachedBlob);
+ const objectUrl = URL.createObjectURL(cachedBlob);
+ // Store URL for cleanup
+ mediaUrls.set(key, objectUrl);
+ return objectUrl;
}
- return await fetchAndCacheMedia(key, url);
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 5000);
+
+ try {
+ return await fetchAndCacheMedia(key, url, controller.signal);
+ } finally {
+ clearTimeout(timeoutId);
+ }
} catch (error) {
+ if (error.name === 'AbortError') {
+ throw new Error(`Fetch timeout for media "${key}"`);
+ }
console.error(`Error caching media for key "${key}":`, error);
throw error;
}
}; Add these utility functions for proper resource management: const mediaUrls = new Map();
const cleanupObjectUrl = (key) => {
const url = mediaUrls.get(key);
if (url) {
URL.revokeObjectURL(url);
mediaUrls.delete(key);
}
};
// Add this to the returned object
return {
cacheMedia,
cleanup: () => {
mediaUrls.forEach((url) => URL.revokeObjectURL(url));
mediaUrls.clear();
}
}; |
||
|
||
const openDatabase = (dbName) => { | ||
return new Promise((resolve, reject) => { | ||
const openDB = indexedDB.open(dbName, 1); | ||
|
||
openDB.onsuccess = () => resolve(openDB.result); | ||
openDB.onerror = () => reject(openDB.error); | ||
}); | ||
}; | ||
|
||
const putMedia = (db, storeName, media) => { | ||
return new Promise((resolve, reject) => { | ||
const tx = db.transaction(storeName, "readwrite"); | ||
const store = tx.objectStore(storeName); | ||
const putRequest = store.put(media); | ||
|
||
putRequest.onsuccess = () => resolve(true); | ||
putRequest.onerror = () => reject(putRequest.error); | ||
|
||
tx.oncomplete = () => db.close(); | ||
}); | ||
}; | ||
|
||
const fetchMedia = (db, storeName, key) => { | ||
return new Promise((resolve, reject) => { | ||
const tx = db.transaction(storeName, "readonly"); | ||
const store = tx.objectStore(storeName); | ||
const getRequest = store.get(key); | ||
|
||
getRequest.onsuccess = () => resolve(getRequest.result?.blob || null); | ||
getRequest.onerror = () => reject(getRequest.error); | ||
|
||
tx.oncomplete = () => db.close(); | ||
}); | ||
}; | ||
|
||
const fetchAndCacheMedia = async (key, url) => { | ||
const response = await fetch(url); | ||
const blob = await response.blob(); | ||
await saveMedia(key, blob); | ||
return URL.createObjectURL(blob); | ||
}; | ||
Comment on lines
+81
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling and add validation in fetchAndCacheMedia The function needs better error handling and validation:
Apply these improvements: const fetchAndCacheMedia = async (key, url) => {
- const response = await fetch(url);
- const blob = await response.blob();
- await saveMedia(key, blob);
- return URL.createObjectURL(blob);
+ const MAX_RETRIES = 3;
+ const MAX_BLOB_SIZE = 5 * 1024 * 1024; // 5MB
+
+ for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
+ try {
+ const response = await fetch(url);
+ if (!response.ok) {
+ throw new Error(`HTTP error! status: ${response.status}`);
+ }
+
+ const blob = await response.blob();
+ if (blob.size > MAX_BLOB_SIZE) {
+ throw new Error('Media file too large');
+ }
+
+ await saveMedia(key, blob);
+ const objectUrl = URL.createObjectURL(blob);
+ mediaUrls.set(key, objectUrl);
+ return objectUrl;
+ } catch (error) {
+ if (attempt === MAX_RETRIES) throw error;
+ await new Promise(resolve => setTimeout(resolve, 1000 * attempt));
+ }
+ }
};
|
||
|
||
return { cacheMedia }; | ||
} | ||
ajinkyapandetekdi marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -18,6 +18,7 @@ import textureImage from "../../assets/images/textureImage.png"; | |||||||
import timer from "../../assets/images/timer.svg"; | ||||||||
import playButton from "../../assets/listen.png"; | ||||||||
import pauseButton from "../../assets/pause.png"; | ||||||||
import { useMediaCache } from "../Hooks/useMediaCache"; | ||||||||
ajinkyapandetekdi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
import { | ||||||||
GreenTick, | ||||||||
HeartBlack, | ||||||||
|
@@ -46,55 +47,88 @@ import { useEffect, useState, useRef } from "react"; | |||||||
import { useNavigate } from "react-router-dom"; | ||||||||
|
||||||||
const MainLayout = (props) => { | ||||||||
const [mediaUrls, setMediaUrls] = useState({}); | ||||||||
const { cacheMedia } = useMediaCache(); | ||||||||
|
||||||||
const mediaFiles = [ | ||||||||
{ key: "practicebgstone", url: practicebgstone }, | ||||||||
{ key: "practicebgstone2", url: practicebgstone2 }, | ||||||||
{ key: "practicebgstone3", url: practicebgstone3 }, | ||||||||
{ key: "practicebg", url: practicebg }, | ||||||||
{ key: "practicebg2", url: practicebg2 }, | ||||||||
{ key: "practicebg3", url: practicebg3 }, | ||||||||
{ key: "gameWon", url: gameWon }, | ||||||||
{ key: "gameLost", url: gameLost }, | ||||||||
{ key: "textureImage", url: textureImage }, | ||||||||
{ key: "timer", url: timer }, | ||||||||
{ key: "playButton", url: playButton }, | ||||||||
{ key: "playButton", url: playButton }, | ||||||||
Comment on lines
+64
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove duplicate entry for playButton. The { key: "playButton", url: playButton },
- { key: "playButton", url: playButton }, 📝 Committable suggestion
Suggested change
|
||||||||
{ key: "clouds", url: clouds }, | ||||||||
{ key: "catLoading", url: catLoading }, | ||||||||
]; | ||||||||
|
||||||||
useEffect(() => { | ||||||||
const cacheAllMedia = async () => { | ||||||||
const urls = {}; | ||||||||
for (const media of mediaFiles) { | ||||||||
const cachedUrl = await cacheMedia(media.key, media.url); | ||||||||
urls[media.key] = cachedUrl; | ||||||||
} | ||||||||
setMediaUrls(urls); | ||||||||
}; | ||||||||
|
||||||||
cacheAllMedia(); | ||||||||
}, []); | ||||||||
ajinkyapandetekdi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
||||||||
const levelsImages = { | ||||||||
1: { | ||||||||
milestone: <LevelOne />, | ||||||||
backgroundAddOn: practicebgstone, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone, | ||||||||
background: practicebg, | ||||||||
}, | ||||||||
2: { | ||||||||
milestone: <LevelTwo />, | ||||||||
backgroundAddOn: practicebgstone2, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone2, | ||||||||
background: practicebg2, | ||||||||
}, | ||||||||
3: { | ||||||||
milestone: <LevelThree />, | ||||||||
backgroundAddOn: practicebgstone3, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone3, | ||||||||
background: practicebg3, | ||||||||
}, | ||||||||
4: { | ||||||||
milestone: <LevelFour />, | ||||||||
backgroundAddOn: practicebgstone, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone, | ||||||||
background: practicebg3, | ||||||||
backgroundColor: `${levelConfig[4].color}60`, | ||||||||
}, | ||||||||
5: { | ||||||||
milestone: <LevelFive />, | ||||||||
backgroundAddOn: practicebgstone3, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone3, | ||||||||
background: practicebg3, | ||||||||
backgroundColor: `${levelConfig[5].color}60`, | ||||||||
}, | ||||||||
6: { | ||||||||
milestone: <LevelSix />, | ||||||||
backgroundAddOn: practicebgstone3, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone3, | ||||||||
background: practicebg3, | ||||||||
backgroundColor: `${levelConfig[6].color}60`, | ||||||||
}, | ||||||||
7: { | ||||||||
milestone: <LevelSeven />, | ||||||||
backgroundAddOn: practicebgstone3, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone3, | ||||||||
background: practicebg3, | ||||||||
backgroundColor: `${levelConfig[7].color}60`, | ||||||||
}, | ||||||||
8: { | ||||||||
milestone: <LevelEight />, | ||||||||
backgroundAddOn: practicebgstone3, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone3, | ||||||||
background: practicebg3, | ||||||||
backgroundColor: `${levelConfig[8].color}60`, | ||||||||
}, | ||||||||
9: { | ||||||||
milestone: <LevelNine />, | ||||||||
backgroundAddOn: practicebgstone3, | ||||||||
backgroundAddOn: mediaUrls.practicebgstone3, | ||||||||
background: practicebg3, | ||||||||
backgroundColor: `${levelConfig[9].color}60`, | ||||||||
}, | ||||||||
|
@@ -327,7 +361,7 @@ const MainLayout = (props) => { | |||||||
flexDirection: "column", | ||||||||
justifyContent: "center", | ||||||||
alignItems: "center", | ||||||||
backgroundImage: `url(${cardBackground || textureImage})`, | ||||||||
backgroundImage: `url(${cardBackground || mediaUrls.textureImage})`, | ||||||||
backgroundSize: "contain", | ||||||||
backgroundRepeat: "round", | ||||||||
boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)", | ||||||||
|
@@ -337,7 +371,7 @@ const MainLayout = (props) => { | |||||||
> | ||||||||
<Box> | ||||||||
<img | ||||||||
src={catLoading} | ||||||||
src={mediaUrls?.catLoading} | ||||||||
alt="catLoading" | ||||||||
// sx={{ height: "58px", width: "58px" }} | ||||||||
/> | ||||||||
|
@@ -356,7 +390,9 @@ const MainLayout = (props) => { | |||||||
display: "flex", | ||||||||
flexDirection: "column", | ||||||||
justifyContent: "space-between", | ||||||||
backgroundImage: `url(${cardBackground || textureImage})`, | ||||||||
backgroundImage: `url(${ | ||||||||
cardBackground || mediaUrls?.textureImage | ||||||||
})`, | ||||||||
backgroundRepeat: "no-repeat", | ||||||||
backgroundSize: "cover", | ||||||||
boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)", | ||||||||
|
@@ -374,7 +410,7 @@ const MainLayout = (props) => { | |||||||
{showTimer && ( | ||||||||
<Box sx={{ position: "absolute" }}> | ||||||||
<img | ||||||||
src={timer} | ||||||||
src={mediaUrls?.timer} | ||||||||
alt="timer" | ||||||||
style={{ height: "58px", width: "58px" }} | ||||||||
/> | ||||||||
|
@@ -706,7 +742,9 @@ const MainLayout = (props) => { | |||||||
display: "flex", | ||||||||
flexDirection: "column", | ||||||||
justifyContent: "space-between", | ||||||||
backgroundImage: `url(${cardBackground || textureImage})`, | ||||||||
backgroundImage: `url(${ | ||||||||
cardBackground || mediaUrls.textureImage | ||||||||
})`, | ||||||||
backgroundSize: "contain", | ||||||||
backgroundRepeat: "round", | ||||||||
boxShadow: "0px 4px 20px -1px rgba(0, 0, 0, 0.00)", | ||||||||
|
@@ -766,7 +804,7 @@ const MainLayout = (props) => { | |||||||
> | ||||||||
{!gameOverData?.userWon && ( | ||||||||
<img | ||||||||
src={clouds} | ||||||||
src={mediaUrls?.clouds} | ||||||||
ajinkyapandetekdi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
alt="clouds" | ||||||||
style={{ zIndex: -999 }} | ||||||||
/> | ||||||||
|
@@ -782,7 +820,7 @@ const MainLayout = (props) => { | |||||||
> | ||||||||
{gameOverData?.userWon ? ( | ||||||||
<img | ||||||||
src={gameWon} | ||||||||
src={mediaUrls?.gameWon} | ||||||||
alt="gameWon" | ||||||||
style={{ zIndex: 9999, height: 340 }} | ||||||||
/> | ||||||||
|
Uh oh!
There was an error while loading. Please reload this page.