Add InfoDialog component and integrate with Redux for state management#488
Add InfoDialog component and integrate with Redux for state management#488rahulharpal1603 merged 4 commits intoAOSSIE-Org:mainfrom Hemil36:InfoDialog-Feature
Conversation
WalkthroughAdds a Redux-controlled global InfoDialog component (with 'info'/'error' variants), registers an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Settings as SettingsPage
participant Store as Redux Store\n(infoDialog slice)
participant App as App Shell
participant Dialog as InfoDialog
User->>Settings: Click "Check for Updates"
Settings->>Settings: perform update check
alt No updates
Settings->>Store: dispatch showInfoDialog({title, message, variant:'info'})
Store-->>App: state.infoDialog updated (isOpen=true)
App->>Dialog: Render with isOpen=true, title, message, variant
User->>Dialog: Click "Close"
Dialog->>Store: dispatch hideInfoDialog()
Store-->>App: state.infoDialog updated (isOpen=false)
App-->>Dialog: Unmount/close
else Updates available
Settings->>Settings: open existing UpdateDialog
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/SettingsPage/Settings.tsx (1)
82-96: Hide loader on failure and surface errors.If
checkForUpdates()throws, loader remains visible and users get no feedback. Wrap in try/catch/finally.Apply:
- const onCheckUpdatesClick = () => { - let checkUpdates = async () => { - dispatch(showLoader('Checking for updates...')); - const hasUpdate = await checkForUpdates(); - if (hasUpdate) { - setUpdateDialogOpen(true); - } else { - // Show info dialog when no updates are available - dispatch(showInfoDialog({ - title: 'No Updates Available', - message: 'Your application is already up to date with the latest version.' - })); - } - dispatch(hideLoader()); - }; - checkUpdates(); - }; + const onCheckUpdatesClick = async () => { + dispatch(showLoader('Checking for updates...')); + try { + const hasUpdate = await checkForUpdates(); + if (hasUpdate) { + setUpdateDialogOpen(true); + } else { + dispatch(showInfoDialog({ + title: 'No Updates Available', + message: 'Your application is already up to date with the latest version.' + })); + } + } catch (err) { + showErrorDialog('Update check failed', err); + } finally { + dispatch(hideLoader()); + } + };
🧹 Nitpick comments (7)
frontend/src/features/infoDialogSlice.ts (2)
24-28: Reset state via return to avoid field-by-field clearing.Returning a fresh copy of initialState is simpler and less error-prone if fields grow.
Apply:
- hideInfoDialog(state) { - state.isOpen = false; - state.title = ''; - state.message = ''; - }, + hideInfoDialog() { + return { ...initialState }; + },
3-7: Export the state interface for reuse in selectors/tests.Helps consumers type state without importing from component props.
-interface InfoDialogState { +export interface InfoDialogState { isOpen: boolean; title: string; message: string; }frontend/src/App.tsx (1)
8-8: Optionally lazy-load InfoDialog to trim initial bundle.The dialog shows infrequently; defer its code until needed.
-import { InfoDialog } from './components/Dialog/InfoDialog'; +const InfoDialog = React.lazy(() => + import('./components/Dialog/InfoDialog').then(m => ({ default: m.InfoDialog })) +);- <InfoDialog isOpen={isOpen} title={title} message={infoMessage} /> + <React.Suspense fallback={null}> + <InfoDialog isOpen={isOpen} title={title} message={infoMessage} /> + </React.Suspense>Also applies to: 21-21
frontend/src/components/Dialog/InfoDialog.tsx (4)
42-44: Prevent accidental form submit and improve a11y on Close button.Add
type="button"and an explicit aria-label.- <Button onClick={handleClose}> + <Button type="button" onClick={handleClose} aria-label="Close information dialog"> Close </Button>
37-39: Allow richer message content (lists/links) by wideningmessagetoReactNode.Current
stringlimits reusability. Switching toReactNodekeeps safety (React escapes strings) and flexibility.Update the type (outside this file):
// frontend/src/types/infoDialog.ts export interface InfoDialogProps { isOpen: boolean; title: string; message: React.ReactNode; }No changes required here; JSX already supports nodes.
31-46: Testing hook: add stable selectors to ease RTL/E2E tests.Optional but helpful for asserting title/message and dispatch on close.
- <DialogContent className="sm:max-w-md"> + <DialogContent className="sm:max-w-md" data-testid="info-dialog"> ... - <Button type="button" onClick={handleClose} aria-label="Close information dialog"> + <Button type="button" onClick={handleClose} aria-label="Close information dialog" data-testid="info-dialog-close">
13-14: Use typed Redux dispatch in InfoDialog.tsx
ImportAppDispatchfrom@/app/storeand apply it touseDispatchfor compile-time safety:import { useDispatch } from 'react-redux'; import { hideInfoDialog } from '@/features/infoDialogSlice'; +import type { AppDispatch } from '@/app/store'; … - const dispatch = useDispatch(); + const dispatch = useDispatch<AppDispatch>();
AppDispatchis already exported infrontend/src/app/store.ts(line 17).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
frontend/src/App.tsx(1 hunks)frontend/src/app/store.ts(1 hunks)frontend/src/components/Dialog/InfoDialog.tsx(1 hunks)frontend/src/features/infoDialogSlice.ts(1 hunks)frontend/src/pages/SettingsPage/Settings.tsx(2 hunks)frontend/src/types/infoDialog.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/App.tsx (2)
frontend/src/app/store.ts (1)
RootState(15-15)frontend/src/components/Dialog/InfoDialog.tsx (1)
InfoDialog(16-49)
frontend/src/pages/SettingsPage/Settings.tsx (1)
frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(19-23)
frontend/src/components/Dialog/InfoDialog.tsx (2)
frontend/src/types/infoDialog.ts (1)
InfoDialogProps(1-5)frontend/src/features/infoDialogSlice.ts (1)
hideInfoDialog(24-28)
🔇 Additional comments (6)
frontend/src/features/infoDialogSlice.ts (1)
19-23: Solid, minimal Redux state for the dialog.Payload is typed and the state remains fully serializable. No concerns.
frontend/src/App.tsx (1)
13-21: Integration looks correct and non-intrusive.State is selected and passed to InfoDialog as props; placement alongside GlobalLoader is appropriate.
frontend/src/app/store.ts (1)
4-11: Reducer registration is correct.infoDialog is added under a clear key; RootState inference updates automatically.
frontend/src/types/infoDialog.ts (1)
1-5: Props shape is clear and aligns with usage.No changes required now.
frontend/src/pages/SettingsPage/Settings.tsx (1)
88-93: Good UX: show info dialog when app is up to date.Message/title are concise and user-friendly.
frontend/src/components/Dialog/InfoDialog.tsx (1)
27-49: Well-structured, accessible dialog; control flow looks solid.Controlled open state, close-on-overlay/ESC via onOpenChange, and explicit Close button are all correct and align with the PR objectives.
…log, and Settings components
… and update related state management
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/SettingsPage/Settings.tsx (1)
82-101: Ensure loader is always cleared; handle update-check failuresIf
checkForUpdates()throws, the loader remains visible and users get no feedback. Wrap in try/catch/finally and surface an error dialog.Apply:
- let checkUpdates = async () => { - dispatch(showLoader('Checking for updates...')); - const hasUpdate = await checkForUpdates(); - if (hasUpdate) { - setUpdateDialogOpen(true); - } else { - // Show info dialog when no updates are available - dispatch( - showInfoDialog({ - title: 'No Updates Available', - message: - 'Your application is already up to date with the latest version.', - variant: 'info', - }), - ); - } - dispatch(hideLoader()); - }; - checkUpdates(); + const checkUpdates = async () => { + dispatch(showLoader('Checking for updates...')); + try { + const hasUpdate = await checkForUpdates(); + if (hasUpdate) { + setUpdateDialogOpen(true); + } else { + dispatch( + showInfoDialog({ + title: 'No Updates Available', + message: + 'Your application is already up to date with the latest version.', + }), + ); + } + } catch (err) { + showErrorDialog('Update Check Failed', err); + } finally { + dispatch(hideLoader()); + } + }; + void checkUpdates();Also optional: the slice defaults
variantto'info', so passing it is redundant.
🧹 Nitpick comments (4)
frontend/src/pages/SettingsPage/Settings.tsx (4)
121-129: Don’t ship test-only dialog behavior to prodGuard the test helper to no-op in production.
const testErrorDialog = () => { + if (isProd()) return; dispatch( showInfoDialog({ title: 'Error Test', message: 'This is a test error message to verify the error styling.', variant: 'error', }) ); };
136-142: Minor: drop explicitvariant: 'info'The slice sets
'info'by default; keep payload minimal.showInfoDialog({ title: 'Cache Refreshed', message: 'The application cache has been successfully refreshed.', - variant: 'info', }),
146-152: Reuse centralized error handler for cache failuresLeverage
showErrorDialogto propagate real error messages consistently.- dispatch( - showInfoDialog({ - title: 'Cache Refresh Error', - message: 'Failed to refresh the application cache. Please try again.', - variant: 'error', - }), - ); + showErrorDialog('Cache Refresh Error', error);
283-283: Hide “Test Error Dialog” button in productionPrevent exposing internal test controls to end users.
- <Button onClick={testErrorDialog}>Test Error Dialog</Button> + {!isProd() && <Button onClick={testErrorDialog}>Test Error Dialog</Button>}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
frontend/src/App.tsx(1 hunks)frontend/src/components/Dialog/InfoDialog.tsx(1 hunks)frontend/src/features/infoDialogSlice.ts(1 hunks)frontend/src/pages/SettingsPage/Settings.tsx(5 hunks)frontend/src/types/infoDialog.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/types/infoDialog.ts
- frontend/src/App.tsx
- frontend/src/components/Dialog/InfoDialog.tsx
- frontend/src/features/infoDialogSlice.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/SettingsPage/Settings.tsx (2)
frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(15-27)frontend/src/services/cacheService.ts (1)
deleteCache(3-11)
🔇 Additional comments (1)
frontend/src/pages/SettingsPage/Settings.tsx (1)
20-20: Import looks correctRedux action import aligns with the new slice API.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
frontend/src/pages/SettingsPage/Settings.tsx (4)
7-7: Delete unused legacy ErrorDialog import.
Now redundant after switching to InfoDialog.- import ErrorDialog from '@/components/Album/Error';
43-46: Remove unusederrorDialogContentstate.
No longer needed once legacy dialog is removed.- const [errorDialogContent, setErrorDialogContent] = useState<{ - title: string; - description: string; - } | null>(null);
274-277: Remove<ErrorDialog />render block.
Prevents two dialogs and dead UI paths.- <ErrorDialog - content={errorDialogContent} - onClose={() => setErrorDialogContent(null)} - />
173-177: Remove legacy ErrorDialog state update to avoid double modals.
This keeps two dialog systems in play; drop the legacy path.- // Also set the legacy error dialog content for backward compatibility - setErrorDialogContent({ - title, - description: errorMessage, - });
🧹 Nitpick comments (3)
frontend/src/pages/SettingsPage/Settings.tsx (3)
104-121: Deduplicate new paths before merge to prevent accidental duplicates.
Covers the case wherenewPathscontains internal duplicates.- const handleFolderPathChange = async (newPaths: string[]) => { - const duplicatePaths = newPaths.filter((path) => - currentPaths.includes(path), - ); + const handleFolderPathChange = async (newPaths: string[]) => { + const uniqueNewPaths = Array.from(new Set(newPaths)); + const duplicatePaths = uniqueNewPaths.filter((path) => + currentPaths.includes(path), + ); if (duplicatePaths.length > 0) { showErrorDialog( 'Duplicate Paths', new Error( `The following paths are already selected: ${duplicatePaths.join(', ')}`, ), ); return; } - generateThumbnailsAPI([...currentPaths, ...newPaths]); - setCurrentPaths([...currentPaths, ...newPaths]); + const mergedPaths = [...currentPaths, ...uniqueNewPaths]; + generateThumbnailsAPI(mergedPaths); + setCurrentPaths(mergedPaths); await deleteCache(); };
258-261: UseonChangefor controlled checkbox; avoid stale state flips.
React convention and clearer intent.- onClick={() => { - setCheck(!check); - setAutoFolder(check ? 'false' : 'true'); - }} + onChange={(e) => { + const next = e.currentTarget.checked; + setCheck(next); + setAutoFolder(next ? 'true' : 'false'); + }}
295-295: Simplify boolean expression.- showCloseButton={false || !isDownloading} + showCloseButton={!isDownloading}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/pages/SettingsPage/Settings.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/SettingsPage/Settings.tsx (1)
frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(15-27)
🔇 Additional comments (5)
frontend/src/pages/SettingsPage/Settings.tsx (5)
20-20: InfoDialog slice import — good integration.
Import aligns with the slice API; consistent with Redux wiring noted in the PR.
127-133: LGTM: success path uses InfoDialog (info).
Consistent UX for cache refresh confirmations.
137-143: LGTM: error path uses InfoDialog (error).
Clear user feedback on cache failures.
162-172: Good: centralized error presentation via InfoDialog.
Error message extraction and dispatch look correct.
16-16: Ignore typo warning: the filefrontend/src/hooks/useQueryExtensio.tsexists and exportsusePictoMutation, so the import is valid.Likely an incorrect or invalid review comment.
|
LGTM! Thanks @Hemil36 |
2 participants
InfoDialogcomponent, implementing Redux state management for dialog visibility and content, and integrating the dialog into the app and settings page.Info Dialog Feature Implementation
InfoDialogcomponent incomponents/Dialog/InfoDialog.tsxthat displays a modal with a title, message, and close button, using the Lucide React icon library and custom UI components.features/infoDialogSlice.tsto manage the dialog's open state, title, and message, with actions to show and hide the dialog.InfoDialogPropsinterface intypes/infoDialog.tsfor type safety of dialog properties.Integration with Application State
infoDialogreducer in the Redux store (app/store.ts) and connected its state to the mainAppcomponent, rendering the dialog when appropriate.Usage Example
Screen.Recording.2025-08-28.164836.mp4
pages/SettingsPage/Settings.tsx) to show the info dialog when no updates are available, providing user feedback.Summary by CodeRabbit
New Features
Settings
Other