create Progress bar component and use on card and details page,logout button add and functionality #30
Conversation
|
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
npm warn config production Use 📝 WalkthroughWalkthroughThe pull request introduces several enhancements across multiple files in the shared library and content micro-frontend. The changes primarily focus on adding progress tracking, status indicators, and implementing a logout functionality. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant TopAppBar
participant Menu
participant LocalStorage
participant LoginPage
User->>TopAppBar: Click Logout Icon
TopAppBar->>Menu: Open Logout Menu
User->>Menu: Confirm Logout
Menu->>LocalStorage: Clear Access Token
LocalStorage-->>Menu: Token Cleared
Menu->>LoginPage: Redirect User
This sequence diagram illustrates the new logout flow introduced in the content page, showing how a user interacts with the logout functionality through the top app bar and menu. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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
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: 8
🔭 Outside diff range comments (1)
libs/shared-lib/src/lib/Layout/Layout.tsx (1)
Line range hint
1-1: Remove @ts-nocheck and fix type errors properly.Using @ts-nocheck disables all TypeScript type checking for the file, which defeats the purpose of using TypeScript. This can lead to runtime errors that could have been caught during development.
Remove the @ts-nocheck directive and properly type check the code. I can help identify and fix the type errors if needed.
🧹 Nitpick comments (7)
libs/shared-lib/src/lib/Progress/Progress.tsx (2)
5-12: Add JSDoc documentation for the interface and its properties.Adding documentation will improve code maintainability and help other developers understand the purpose of each prop.
+/** + * Props for the Progress component + */ interface CommonCircularProgressProps { + /** The variant to use. It can be 'determinate' or 'indeterminate' */ variant?: 'determinate' | 'indeterminate'; + /** The value of the progress indicator for the determinate variant */ value?: number; + /** The size of the circle in pixels */ size?: number; + /** The thickness of the circle */ thickness?: number; + /** The color of the component */ color?: string; + /** The system prop that allows defining system overrides as well as additional CSS styles */ sx?: SxProps<Theme>; }
14-34: Add prop validation and improve type safety.Consider these improvements for better type safety and validation:
+import { PropTypes } from '@mui/material'; + export const Progress: React.FC<CommonCircularProgressProps> = ({ variant = 'determinate', value = 0, size = 40, thickness = 3.6, - color = 'primary', + color = 'primary.main', sx = {}, }) => { + // Validate value is between 0 and 100 for determinate variant + if (variant === 'determinate' && (value < 0 || value > 100)) { + console.warn('Progress value should be between 0 and 100'); + } + return ( <CircularProgress variant={variant} value={value} size={size} thickness={thickness} sx={{ color, ...sx, }} /> ); }; + +Progress.propTypes = { + variant: PropTypes.oneOf(['determinate', 'indeterminate']), + value: PropTypes.number, + size: PropTypes.number, + thickness: PropTypes.number, + color: PropTypes.string, + sx: PropTypes.object, +};mfes/content/src/pages/details/[identifier].tsx (1)
Line range hint
63-74: Optimize performance for large lists.The current implementation renders all children at once, which could impact performance with large datasets.
Consider implementing:
- Virtualization for large lists
- Pagination or infinite scroll
- Lazy loading of child items
Example implementation using react-window:
import { FixedSizeList } from 'react-window'; const renderNestedChildren = (children: any) => { if (!Array.isArray(children)) { return null; } return ( <FixedSizeList height={400} width="100%" itemCount={children.length} itemSize={50} > {({ index, style }) => ( <CommonCollapse style={style} key={children[index].id} identifier={children[index].identifier} title={children[index].name} data={children[index]?.children} defaultExpanded={false} progress={children[index].progress ?? 0} status={children[index].status ?? 'Not started'} /> )} </FixedSizeList> ); };libs/shared-lib/src/lib/Card/CommonCard.tsx (2)
66-141: Consider extracting the progress overlay into a separate component.The progress overlay implementation is duplicated in multiple places. Consider creating a reusable
ProgressOverlaycomponent to improve maintainability and reduce code duplication.+// Create a new component ProgressOverlay.tsx +interface ProgressOverlayProps { + progress: number; + status?: string; + showStatus?: boolean; +} + +export const ProgressOverlay: React.FC<ProgressOverlayProps> = ({ + progress, + status, + showStatus, +}) => ( + <Box + sx={{ + position: 'absolute', + height: '40px', + top: 0, + width: '100%', + display: 'flex', + alignItems: 'center', + background: 'rgba(0, 0, 0, 0.5)', + }} + > + <Progress + variant="determinate" + value={100} + size={30} + thickness={5} + sx={{ + color: '#fff8fb', + position: 'absolute', + left: '10px', + }} + /> + <Progress + variant="determinate" + value={progress} + size={30} + thickness={5} + sx={{ + color: progress === 100 ? '#21A400' : '#FFB74D', + position: 'absolute', + left: '10px', + }} + /> + {showStatus && ( + <Typography + sx={{ + fontSize: '12px', + fontWeight: 'bold', + marginLeft: '12px', + color: progress === 100 ? '#21A400' : '#FFB74D', + position: 'absolute', + left: '50px', + }} + > + {status} + </Typography> + )} + </Box> +); // Use in CommonCard -{progress !== undefined && ( - <Box> - ... // Current progress overlay implementation - </Box> -)} +{progress !== undefined && ( + <ProgressOverlay + progress={progress} + status={status} + showStatus={actions?.toString().toLowerCase() === 'resource'} + /> +)}
88-96: Consider using theme variables for consistent styling.Magic numbers and hardcoded colors are used in the styling. Consider:
- Using theme spacing for consistent margins/paddings
- Defining color constants or using theme palette
- Creating styled components for repeated styles
+// Create theme constants +const THEME = { + colors: { + progress: { + background: 'rgba(0, 0, 0, 0.5)', + completed: '#21A400', + inProgress: '#FFB74D', + base: '#fff8fb', + }, + }, + spacing: { + progress: { + height: '40px', + iconSize: '30px', + thickness: 5, + leftOffset: '10px', + }, + }, +}; // Use in styles -background: 'rgba(0, 0, 0, 0.5)', +background: THEME.colors.progress.background,Also applies to: 104-107, 115-118, 122-128
libs/shared-lib/src/lib/Layout/Layout.tsx (2)
182-183: Consider memoizing selectedValues access.Accessing selectedValues[filterCode] on every render could be optimized using useMemo for better performance, especially with multiple filters.
- const currentSelectedValues = selectedValues[filterCode] || []; + const currentSelectedValues = React.useMemo( + () => selectedValues[filterCode] || [], + [selectedValues, filterCode] + );
Line range hint
1-605: Consider splitting FilterDialog into a separate component file.The FilterDialog component has grown complex enough to warrant its own file. This would:
- Improve maintainability and testability
- Reduce the cognitive load when working with the Layout component
- Allow for better separation of concerns
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
libs/shared-lib/src/index.ts(1 hunks)libs/shared-lib/src/lib/Card/CommonCard.tsx(5 hunks)libs/shared-lib/src/lib/Header/TopAppBar.tsx(2 hunks)libs/shared-lib/src/lib/Layout/Layout.tsx(5 hunks)libs/shared-lib/src/lib/Progress/Progress.tsx(1 hunks)mfes/content/src/components/CommonCollapse.tsx(9 hunks)mfes/content/src/pages/content-details/[identifier].tsx(1 hunks)mfes/content/src/pages/content.tsx(8 hunks)mfes/content/src/pages/details/[identifier].tsx(2 hunks)
🔇 Additional comments (7)
libs/shared-lib/src/index.ts (1)
26-26: Potential naming conflict with Progress exports.The file exports both
Progress/CircularandProgress/Progress. This could lead to confusion when importing. Consider:
- Renaming one of the components for clarity
- Consolidating both progress components into a single module
mfes/content/src/pages/content-details/[identifier].tsx (1)
105-105: LGTM! The image width adjustment improves visual balance.The change from 100% to 80% width provides better visual spacing and prevents the image from stretching too wide.
libs/shared-lib/src/lib/Card/CommonCard.tsx (1)
25-26: LGTM! Well-typed props for progress tracking.The new props are properly typed with clear constraints for the status values.
mfes/content/src/pages/content.tsx (2)
114-114: LGTM! YouTube video support added.The mime type for YouTube videos is properly added to the supported content types.
309-325: LGTM! Well-implemented logout menu.The menu implementation follows Material-UI best practices with proper positioning and event handling.
mfes/content/src/components/CommonCollapse.tsx (1)
26-27: LGTM! Well-defined interfaces for progress tracking.The progress and status props are properly typed and consistent across components.
Also applies to: 44-45
libs/shared-lib/src/lib/Layout/Layout.tsx (1)
150-153: LGTM! Dialog styling is consistent with the design system.The background color #FEF7FF matches the design system's color palette.
| @@ -459,6 +465,7 @@ export const Layout: React.FC<LayoutProps> = ({ | |||
| bgcolor="#FDF7FF" | |||
| actionIcons={topAppBarIcons} | |||
| menuIconClick={() => setIsDrawerOpen(true)} | |||
| onLogoutClick={(event) => action.onLogoutClick(event)} | |||
There was a problem hiding this comment.
Fix undefined 'action' reference in onLogoutClick handler.
There's a critical bug where action is undefined. This will cause a runtime error when the logout button is clicked.
- onLogoutClick={(event) => action.onLogoutClick(event)}
+ onLogoutClick={(event) => showTopAppBar?.actionIcons?.[0]?.onLogoutClick?.(event)}Note: The fix assumes the logout action is the first icon. Consider passing the logout handler directly through props for better maintainability.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
libs/shared-lib/src/lib/Header/TopAppBar.tsx (1)
13-16: 🛠️ Refactor suggestionConsider using a more flexible action handling pattern.
The current implementation tightly couples the
ActionIconinterface to logout functionality. A more generic approach would be better for maintainability and reusability.Consider this alternative:
interface ActionIcon { icon: React.ReactNode; ariaLabel: string; anchorEl?: HTMLElement | null; - onLogoutClick: ( - event: React.MouseEvent<HTMLButtonElement, MouseEvent> - ) => void; + onClick: (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void; + actionType: 'logout' | 'menu' | 'profile' | 'other'; }This change:
- Makes the interface more generic and reusable
- Maintains type safety through actionType
- Allows for future action types without interface changes
mfes/content/src/pages/content.tsx (2)
71-78:⚠️ Potential issueImprove token cleanup and error handling during logout.
The current implementation only removes 'accToken', but there might be other auth-related items that should be cleared. Also, the navigation could fail silently.
Consider this enhancement:
const handleClose = () => { setAnchorEl(null); + try { + // Clear all auth-related items + ['accToken', 'refToken', 'userRole', 'userName'].forEach(key => + localStorage.removeItem(key) + ); + router.push(`${process.env.NEXT_PUBLIC_LOGIN}`); + } catch (error) { + console.error('Logout failed:', error); + // Show error notification to user + } };
154-155: 🛠️ Refactor suggestionConsider persisting progress state.
Currently, cards are initialized with hardcoded "Not started" status and 0 progress.
Consider:
- Fetching initial progress from an API
- Persisting progress updates
- Showing loading state while fetching progress
+ interface Progress { + status: 'Not started' | 'Completed' | 'In progress'; + progress: number; + } + + const [progressMap, setProgressMap] = useState<Record<string, Progress>>({}); + + useEffect(() => { + // Fetch progress for visible content + const fetchProgress = async () => { + try { + const progress = await fetch(`/api/progress?ids=${contentData.map(i => i.identifier).join(',')}`); + setProgressMap(progress); + } catch (error) { + console.error('Failed to fetch progress:', error); + } + }; + fetchProgress(); + }, [contentData]); <CommonCard // ... other props - status={'Not started'} - progress={0} + status={progressMap[item?.identifier]?.status ?? 'Not started'} + progress={progressMap[item?.identifier]?.progress ?? 0} />
🧹 Nitpick comments (2)
libs/shared-lib/src/lib/Header/TopAppBar.tsx (1)
117-135: Enhance menu configuration and type safety.The current Menu implementation is hardcoded to show only a logout option. Consider making it more flexible and type-safe.
Consider this enhancement:
+ interface MenuItem { + label: string; + onClick: () => void; + icon?: React.ReactNode; + } interface CommonAppBarProps { // ... existing props + menuItems?: MenuItem[]; onMenuClose?: () => void; } // In the Menu component: - <MenuItem onClick={onMenuClose}>Logout</MenuItem> + {menuItems?.map((item, index) => ( + <MenuItem key={index} onClick={item.onClick}> + {item.icon && <ListItemIcon>{item.icon}</ListItemIcon>} + <ListItemText primary={item.label} /> + </MenuItem> + ))}mfes/content/src/pages/content.tsx (1)
311-327: Consolidate Menu component implementation.The Menu component implementation is duplicated from TopAppBar. Consider extracting it into a shared component.
+ // In libs/shared-lib/src/lib/Menu/LogoutMenu.tsx + interface LogoutMenuProps { + anchorEl: HTMLElement | null; + onClose: () => void; + } + + export const LogoutMenu: React.FC<LogoutMenuProps> = ({ anchorEl, onClose }) => ( + <Menu + id="menu-appbar" + anchorEl={anchorEl} + anchorOrigin={{ + vertical: 'top', + horizontal: 'right', + }} + keepMounted + transformOrigin={{ + vertical: 'top', + horizontal: 'right', + }} + open={Boolean(anchorEl)} + onClose={onClose} + > + <MenuItem onClick={onClose}>Logout</MenuItem> + </Menu> + ); - <Menu> - {/* ... duplicate implementation ... */} - </Menu> + <LogoutMenu anchorEl={anchorEl} onClose={handleClose} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/shared-lib/src/lib/Header/TopAppBar.tsx(5 hunks)libs/shared-lib/src/lib/Layout/Layout.tsx(6 hunks)mfes/content/src/pages/content.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/shared-lib/src/lib/Layout/Layout.tsx
|
Summary by CodeRabbit
Based on the comprehensive pull request summary, here are the concise release notes:
New Features
Progresscomponent for visual progress tracking.Improvements
User Experience