Refactor LanguageSelectionGrid component to include loading state - #265
Refactor LanguageSelectionGrid component to include loading state#265VishnuKrishnathu wants to merge 4 commits into
Conversation
- Reintroduced useChatStorage and useSiteStorage hooks. - Added loading indicators for flow languages while data is being fetched. - Improved conditional rendering logic for language options based on loading state.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request modifies the language selection grid component to implement loading state handling. It captures the loading status from a query hook and implements conditional rendering: skeleton placeholders display during loading, fetched flow languages appear when ready, and a fallback language list shows only if no flow languages exist after loading completes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (1)
src/components/LanguageSelectionGrid.jsx (1)
79-89: Consider reducing duplicated skeleton markup.The four identical placeholder blocks are maintainable now, but a tiny map-based render keeps this cleaner for future style/layout tweaks.
♻️ Optional cleanup
- isFlowLanguagesLoading && ( - <> - <div className="bg-[`#e3e9f0`] animate-pulse w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center"> - </div> - <div className="bg-[`#e3e9f0`] animate-pulse w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center"> - </div> - <div className="bg-[`#e3e9f0`] animate-pulse w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center"> - </div> - <div className="bg-[`#e3e9f0`] animate-pulse w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center"> - </div> - </> - ) + isFlowLanguagesLoading && + Array.from({ length: 4 }).map((_, idx) => ( + <div + key={`lang-skeleton-${idx}`} + className="bg-[`#e3e9f0`] animate-pulse w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center" + /> + ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LanguageSelectionGrid.jsx` around lines 79 - 89, Replace the four duplicated skeleton placeholder divs in LanguageSelectionGrid.jsx (rendered when isFlowLanguagesLoading is true) with a concise map-based render: create an array of four items and map over it to return the same placeholder div for each item, ensure you pass a unique key (e.g., index) and preserve the existing className/structure so styles and animation remain unchanged; update the JSX inside the isFlowLanguagesLoading conditional to use this mapped array instead of repeating the markup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/LanguageSelectionGrid.jsx`:
- Around line 92-99: The current render checks only !isFlowLanguagesLoading and
flowLanguages, which fails when flowLanguages is {languages: []} or missing
languages; update the conditional rendering to first guard that
flowLanguages.languages is a non-empty array (e.g., use
flowLanguages?.languages?.length > 0) before mapping in the JSX where
flowLanguages.languages.map(...) is used (refer to flowLanguages,
flowLanguages.languages, isFlowLanguagesLoading, languageList,
handleLanguageClick, languageValueMap), and add a fallback branch that renders
languageList when flowLanguages is falsy or when flowLanguages.languages is
empty to avoid runtime errors and an empty grid.
---
Nitpick comments:
In `@src/components/LanguageSelectionGrid.jsx`:
- Around line 79-89: Replace the four duplicated skeleton placeholder divs in
LanguageSelectionGrid.jsx (rendered when isFlowLanguagesLoading is true) with a
concise map-based render: create an array of four items and map over it to
return the same placeholder div for each item, ensure you pass a unique key
(e.g., index) and preserve the existing className/structure so styles and
animation remain unchanged; update the JSX inside the isFlowLanguagesLoading
conditional to use this mapped array instead of repeating the markup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd838192-4835-454f-843f-8e2c110fe0fe
📒 Files selected for processing (1)
src/components/LanguageSelectionGrid.jsx
| {!isFlowLanguagesLoading && flowLanguages && | ||
| flowLanguages.languages.map(lang => ( | ||
| <div key={lang} className="div14-lang w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center" onClick={() => handleLanguageClick(lang)}> | ||
| <button className="w-full">{languageValueMap[lang]}</button> | ||
| </div> | ||
| ))} | ||
| {!flowLanguages && | ||
| {!isFlowLanguagesLoading && !flowLanguages && | ||
| languageList |
There was a problem hiding this comment.
Fallback condition misses empty-language responses.
Line 98 only checks !flowLanguages. If API returns { languages: [] }, neither list renders and users get an empty grid. Also, flowLanguages truthy with missing languages can break mapping at Line 93.
✅ Suggested fix
- {!isFlowLanguagesLoading && flowLanguages &&
- flowLanguages.languages.map(lang => (
+ {!isFlowLanguagesLoading && (flowLanguages?.languages?.length ?? 0) > 0 &&
+ flowLanguages.languages.map(lang => (
<div key={lang} className="div14-lang w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center" onClick={() => handleLanguageClick(lang)}>
<button className="w-full">{languageValueMap[lang]}</button>
</div>
))}
- {!isFlowLanguagesLoading && !flowLanguages &&
+ {!isFlowLanguagesLoading && (flowLanguages?.languages?.length ?? 0) === 0 &&
languageList
.filter(lang => !lang.excludeFor.includes(urlFlow || usecaseType))
.map(lang => (📝 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.
| {!isFlowLanguagesLoading && flowLanguages && | |
| flowLanguages.languages.map(lang => ( | |
| <div key={lang} className="div14-lang w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center" onClick={() => handleLanguageClick(lang)}> | |
| <button className="w-full">{languageValueMap[lang]}</button> | |
| </div> | |
| ))} | |
| {!flowLanguages && | |
| {!isFlowLanguagesLoading && !flowLanguages && | |
| languageList | |
| {!isFlowLanguagesLoading && (flowLanguages?.languages?.length ?? 0) > 0 && | |
| flowLanguages.languages.map(lang => ( | |
| <div key={lang} className="div14-lang w-full text-center vertical-center m-0 h-[100px] flex items-center justify-center" onClick={() => handleLanguageClick(lang)}> | |
| <button className="w-full">{languageValueMap[lang]}</button> | |
| </div> | |
| ))} | |
| {!isFlowLanguagesLoading && (flowLanguages?.languages?.length ?? 0) === 0 && | |
| languageList |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/LanguageSelectionGrid.jsx` around lines 92 - 99, The current
render checks only !isFlowLanguagesLoading and flowLanguages, which fails when
flowLanguages is {languages: []} or missing languages; update the conditional
rendering to first guard that flowLanguages.languages is a non-empty array
(e.g., use flowLanguages?.languages?.length > 0) before mapping in the JSX where
flowLanguages.languages.map(...) is used (refer to flowLanguages,
flowLanguages.languages, isFlowLanguagesLoading, languageList,
handleLanguageClick, languageValueMap), and add a fallback branch that renders
languageList when flowLanguages is falsy or when flowLanguages.languages is
empty to avoid runtime errors and an empty grid.
There was a problem hiding this comment.
Languages will never be empty
There was a problem hiding this comment.
@VishnuKrishnathu, understood! If the API contract guarantees that the languages array is always non-empty, then the empty-grid concern doesn't apply. I'll note this for future reviews.
(The optional chaining on flowLanguages?.languages is still a low-cost defensive guard against unexpected API shape changes, but I leave that to your discretion.)
(^ο½₯ω・^)
🧠 Learnings used
Learnt from: VishnuKrishnathu
Repo: ELEVATE-Project/mohini-app-frontend PR: 244
File: src/components/LanguageSelectionGrid.jsx:47-68
Timestamp: 2026-02-16T06:38:02.503Z
Learning: In LanguageSelectionGrid.jsx (src/components/LanguageSelectionGrid.jsx), the `setPreviousUrl` and `setStorageFlow` calls are intentionally only set for specific route_mapping flows (ParentPerceptionSurvey, ListeningActivity) before navigation. For the COMMON_CHAT navigation path, these should NOT be set, as users should not be able to navigate back to /home or the current page. This is expected behavior.
- Introduced 'StudyTeacherInterview' and 'BiharTeachersBot' session flows in session constants. - Updated dynamic voice chat logic to include 'BiharTeachersBot' in conditions for showing confirm and new session buttons.
…oice chat logic to reflect changes in session flow conditions.
- Changed background styles of loading indicators to improve visual appearance with shadow and rounded corners. - Ensured consistent styling across multiple loading divs for better user experience during data fetching.
1 participant
Summary by CodeRabbit