feat: dark theme support #148
Conversation
|
@AmitChauhan63390 looks great, just take a look at discord to make some fixes |
|
Did as you told me on discord. Hope you like it |
|
@RohitR311 do take a look |
rob1.mp4 |
There was a problem hiding this comment.
at a lot of places, the theme has been reset to default MUI theme. @RohitR311 can you take it from here
WalkthroughThe pull request introduces a comprehensive theme management system for the application, focusing on implementing dark mode functionality across various components. The changes span multiple files, introducing a new Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (25)
src/components/molecules/RunContent.tsx (1)
78-121: Refactor tab styling to improve maintainability and consistencyWhile the dark theme implementation looks good, there are a few improvements that could enhance maintainability:
- Use theme tokens instead of hardcoded colors
- Extract duplicate tab styles to reduce code duplication
- Consider adding smooth transitions for hover effects
Here's a suggested refactor:
<Tabs value={tab} onChange={(e, newTab) => setTab(newTab)} aria-label="run-content-tabs" sx={{ '& .MuiTabs-indicator': { - backgroundColor: '#FF00C3', + backgroundColor: theme => theme.palette.primary.main, }, '& .MuiTab-root': { '&.Mui-selected': { - color: '#FF00C3', + color: theme => theme.palette.primary.main, }, } }} > + {[ + { label: "Output Data", value: 'output' }, + { label: "Log", value: 'log' } + ].map(({ label, value }) => ( <Tab - label="Output Data" - value='output' + key={value} + label={label} + value={value} sx={{ color: (theme) => theme.palette.mode === 'dark' ? '#fff' : '#000', '&:hover': { - color: '#FF00C3' + color: theme => theme.palette.primary.main, + transition: theme => theme.transitions.create('color'), }, '&.Mui-selected': { - color: '#FF00C3', + color: theme => theme.palette.primary.main, } }} /> + ))} - <Tab - label="Log" - value='log' - sx={{...}} // Removed duplicate styling - />src/components/molecules/InterpretationLog.tsx (1)
127-129: Consider using theme tokens for colors and removing empty lineA few suggestions to improve the styling:
- Replace the hard-coded color
#ff00c3with a theme token for better maintainability and theme consistency- Remove the empty line in the styling object (line 129)
sx={{ borderRadius: ' 0 0 10px 10px', - color: 'white', position: 'absolute', - background: '#ff00c3', + background: theme.palette.primary.main, border: 'none', padding: '10px 20px', width: '900px', overflow: 'hidden', textAlign: 'left', justifyContent: 'flex-start', '&:hover': { - backgroundColor: '#ff00c3', + backgroundColor: theme.palette.primary.dark, }, }}src/components/organisms/BrowserContent.tsx (3)
143-143: Move inline styles to styled componentConsider moving the inline styles to the
BrowserContentWrapperstyled component for better maintainability and consistency with the project's styling approach.-<div id="browser" style={{ display: "flex", flexDirection: "column" }} > +<div id="browser">Then add to BrowserContentWrapper:
const BrowserContentWrapper = styled.div` position: relative; width: 100vw; height: 100vh; overflow: hidden; + display: flex; + flex-direction: column; `;
163-167: Consider theme-aware styling and mobile viewport handlingTwo suggestions for improvement:
- Since this PR implements dark theme support, consider adding theme-aware styling:
-const BrowserContentWrapper = styled.div` +const BrowserContentWrapper = styled.div<{ theme: Theme }>` position: relative; width: 100vw; height: 100vh; overflow: hidden; + background-color: ${({ theme }) => theme.palette.background.default}; + color: ${({ theme }) => theme.palette.text.primary}; `;
- The use of
100vhcan cause issues on mobile browsers due to dynamic viewport height behavior with toolbars. Consider using alternative approaches:const BrowserContentWrapper = styled.div` position: relative; width: 100vw; - height: 100vh; + height: 100%; + min-height: 100vh; + min-height: 100dvh; /* dynamic viewport height */ overflow: hidden; `;
Theme implementation needs attention in browser components
The verification reveals several components that need theme-related updates:
- BrowserWindow uses inline styles without theme integration
- BrowserTabs imports useThemeMode but doesn't appear to use it for styling
- BrowserContent uses basic styling without theme consideration
Specific locations requiring theme implementation:
- BrowserWindow's container div and modal styles should use theme colors
- BrowserTabs should implement theme-aware styled components
- BrowserContent's container should use theme-aware styling for consistent dark mode support
🔗 Analysis chain
Line range hint
18-161: Verify theme implementation completenessGiven that this PR implements dark theme support, we should verify if this component needs additional theme-related changes for child components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if BrowserNavBar and BrowserWindow components implement theme support ast-grep --pattern 'const $_ = styled.$_` $$$ ${({ theme }) => $$$} $$$ `' src/components/molecules/BrowserNavBar.tsx src/components/organisms/BrowserWindow.tsx # Check for useTheme or useThemeMode hook usage rg -l "useTheme|useThemeMode" src/components/molecules/BrowserNavBar.tsx src/components/organisms/BrowserWindow.tsxLength of output: 329
Script:
#!/bin/bash # Let's check the actual implementation of BrowserNavBar and BrowserWindow # to see their theme-related code # Check BrowserNavBar implementation rg -A 5 "useTheme|theme" src/components/molecules/BrowserNavBar.tsx # Check BrowserWindow styled components and theme usage rg -A 5 "styled\.|theme" src/components/organisms/BrowserWindow.tsx # Check if there are any theme-related styles in BrowserTabs rg -A 5 "styled\.|theme" src/components/molecules/BrowserTabs.tsxLength of output: 800
Script:
#!/bin/bash # Let's check the styled components in BrowserTabs and their theme implementation rg -A 10 "styled\." src/components/molecules/BrowserTabs.tsx # Check if BrowserWindow has any styling that might need theme support rg -A 10 "styled\.|className|style" src/components/organisms/BrowserWindow.tsx # Check if there are any color-related styles in the BrowserContent itself rg -A 5 "background|color|border|style=" src/components/organisms/BrowserContent.tsxLength of output: 3144
src/components/molecules/BrowserTabs.tsx (3)
126-138: Adjust CloseButton styles to adapt to theme modeThe
CloseButtoncomponent uses hardcoded colors that may not align with the current theme. Adjust thebackgroundColorand hover styles based onisDarkModefor consistent theming.Apply this diff to make the
CloseButtonstyles adapt to the theme mode:sx={{ height: '28px', width: '28px', padding: '4px', - backgroundColor: '#3A3A3A', // Synced dark gray background + backgroundColor: `${isDarkMode ? '#3A3A3A' : '#FFFFFF'}`, // Adjust background color based on theme borderRadius: '50%', '&:hover': { - backgroundColor: '#505050', // Synced hover color for close button - color: '#FFFFFF', + backgroundColor: `${isDarkMode ? '#505050' : '#F0F0F0'}`, // Adjust hover background color + color: `${isDarkMode ? '#FFFFFF' : '#000000'}`, // Adjust hover text color }, '&.Mui-disabled': { opacity: 0.4, // Lower opacity for disabled state }, transition: 'background-color 0.3s ease, color 0.3s ease', }}
42-42: Consider making width responsiveSetting a fixed width can impact responsiveness. Use relative units like percentages to ensure the component adjusts to different screen sizes.
Apply this diff to make the width responsive:
- width: '900px', // Fixed width + width: '100%', // Make width responsive
97-108: Remove commented-out codeThere is a block of commented-out code related to the
IconButton. Remove unused code to keep the codebase clean and maintainable.src/components/molecules/NavBar.tsx (1)
215-225: AdjustdiscardButtonstyles to adapt to theme modeThe
discardButtonstyles use hardcoded colors that might not provide optimal contrast in dark mode. Adjust thebackgroundandcolorproperties based ondarkModefor better visibility and theme consistency.Modify the
discardButtonstyle to be a function that acceptsdarkMode:- discardButton: { + discardButton: (darkMode: boolean) => ({ borderRadius: '5px', padding: '8px', - background: 'red', + background: darkMode ? '#ff4d4d' : 'red', color: 'white', marginRight: '10px', '&:hover': { color: 'white', backgroundColor: '#ff0000' } - } + }),Update the usage in the component:
<IconButton onClick={goToMainMenu} - sx={styles.discardButton} + sx={styles.discardButton(darkMode)} >src/App.tsx (1)
3-3: Remove unused importcreateThemeThe
createThemeimport is no longer used after removingThemeProvider. Remove unused imports to clean up the code.Apply this diff to remove the unused import:
-import { createTheme } from "@mui/material/styles";src/context/theme-provider.tsx (1)
5-75: Consider extracting theme configuration to separate filesThe light and dark theme configurations contain duplicated values and could benefit from better organization.
Consider:
- Extract shared values to a common configuration:
// src/theme/constants.ts export const THEME_COLORS = { primary: '#ff00c3', primaryHover: '#e600b3', }
- Create separate files for light and dark themes:
// src/theme/lightTheme.ts import { THEME_COLORS } from './constants'; export const lightTheme = createTheme({...})src/components/molecules/ActionSettings.tsx (1)
9-9: Consider using theme context instead of prop drillingThe component could directly use the theme context instead of receiving darkMode as a prop.
- interface ActionSettingsProps { - action: string; - darkMode?: boolean; - } + interface ActionSettingsProps { + action: string; + } - export const ActionSettings = ({ action, darkMode = false }: ActionSettingsProps) => { + export const ActionSettings = ({ action }: ActionSettingsProps) => { + const { darkMode } = useThemeMode();Also applies to: 12-12
src/components/molecules/BrowserRecordingSave.tsx (1)
Line range hint
14-22: Add error handling for stopRecordingThe navigation function doesn't properly handle API errors.
const goToMainMenu = async () => { if (browserId) { + try { await stopRecording(browserId); notify('warning', 'Current Recording was terminated'); setBrowserId(null); + } catch (error) { + console.error('Failed to stop recording:', error); + notify('error', 'Failed to stop recording'); + return; + } } navigate('/'); };src/pages/Login.tsx (1)
Line range hint
63-71: Implement dark theme supportThe login form uses a hard-coded white background color which doesn't align with the dark theme implementation. Consider using theme-aware colors:
sx={{ textAlign: "center", - backgroundColor: "#ffffff", + backgroundColor: theme.palette.background.paper, padding: 6, borderRadius: 5, - boxShadow: "0px 20px 40px rgba(0, 0, 0, 0.2), 0px -5px 10px rgba(0, 0, 0, 0.15)", + boxShadow: theme.shadows[4], display: "flex", flexDirection: "column", alignItems: "center", maxWidth: 400, width: "100%", }}Don't forget to import and use the
useThemehook:+import { useTheme } from "@mui/material/styles"; const Login = () => { + const theme = useTheme(); // ... rest of the componentsrc/components/organisms/MainMenu.tsx (2)
25-27: Centralize theme color constantsConsider moving the color constants to a theme configuration file for better maintainability:
- const defaultcolor = theme.palette.mode === 'light' ? 'black' : 'white'; - const selectedPink = '#FF00C3'; + const defaultcolor = theme.palette.text.primary; + const selectedPink = theme.palette.primary.main;Create a theme configuration file (e.g.,
src/theme/index.ts) to define these colors:export const themeConfig = { palette: { primary: { main: '#FF00C3', }, // ... other theme configurations }, };
46-69: Extract tab styles to a separate styled componentThe inline styles for tabs are quite lengthy. Consider extracting them to a styled component for better maintainability:
const StyledTabs = styled(Tabs)(({ theme }) => ({ alignItems: 'flex-start', '& .MuiTab-root': { color: theme.palette.text.primary, textTransform: 'none', fontSize: 'medium', justifyContent: 'flex-start', textAlign: 'left', '&.Mui-selected': { color: theme.palette.primary.main, }, '& .MuiTabs-indicator': { backgroundColor: theme.palette.primary.main, }, }, }));Then use it in your component:
-<Tabs +<StyledTabs value={value} onChange={handleChange} orientation="vertical" TabIndicatorProps={{ style: { - backgroundColor: '#ff00c3', + backgroundColor: theme.palette.primary.main, width: '2px', right: 0, }, }} - sx={{...}} />src/components/molecules/BrowserNavBar.tsx (3)
16-26: Consider using MUI's styling system for consistencyThe component mixes styled-components with Material-UI. Consider using MUI's styling system for consistency:
import { styled } from '@mui/material/styles'; const StyledNavBar = styled('div')<{ browserWidth: number }>( ({ theme, browserWidth }) => ({ display: 'flex', alignItems: 'center', padding: '10px 20px', backgroundColor: theme.palette.background.paper, width: `${browserWidth}px`, borderRadius: '0px 0px 8px 8px', boxShadow: theme.shadows[1], transition: theme.transitions.create(['background-color', 'box-shadow']), marginBottom: '15px', }) );
28-46: Extract IconButton component to a separate fileThe
IconButtoncomponent has significant styling logic. Consider moving it to a separate file:// src/components/atoms/buttons/IconButton.tsx import { styled } from '@mui/material/styles'; import { NavBarButton } from './buttons'; export const IconButton = styled(NavBarButton)(({ theme }) => ({ display: 'flex', alignItems: 'center', justifyContent: 'center', padding: '8px', marginRight: '12px', backgroundColor: theme.palette.mode === 'dark' ? '#40444B' : '#E0E0E0', borderRadius: '50%', transition: theme.transitions.create(['background-color', 'transform']), color: theme.palette.text.primary, cursor: 'pointer', '&:hover': { backgroundColor: theme.palette.mode === 'dark' ? '#586069' : '#D0D0D0', }, '&:active': { transform: 'scale(0.95)', }, }));
106-140: Extract navigation buttons into a separate componentThe repeated button pattern could be extracted into a reusable component:
interface NavButtonProps { onClick: () => void; icon: React.ReactNode; mode: string; } const NavigationButton: React.FC<NavButtonProps> = ({ onClick, icon, mode }) => ( <IconButton type="button" onClick={onClick} disabled={false} mode={mode} > {icon} </IconButton> );Usage:
<NavigationButton onClick={() => socket?.emit('input:back')} icon={<ArrowBackIcon />} mode={isDarkMode ? 'dark' : 'light'} />src/components/molecules/ActionDescriptionBox.tsx (1)
97-104: Simplify checkbox styling using theme tokensThe checkbox styling can be simplified by using MUI's theme tokens instead of conditional colors.
Consider this implementation:
- sx={{ - color: isDarkMode ? 'white' : 'default', - '&.Mui-checked': { - color: isDarkMode ? '#90caf9' : '#1976d2', - }, - }} + sx={(theme) => ({ + color: theme.palette.text.primary, + '&.Mui-checked': { + color: theme.palette.primary.main, + }, + })}src/components/molecules/IntegrationSettings.tsx (1)
Line range hint
307-322: Consider extracting theme-specific styles to a theme configuration.While the dark mode implementation works, the hardcoded colors could be moved to a theme configuration for better maintainability.
Consider creating a theme configuration:
const themeColors = { dark: { alertBg: '#3b002d', alertBorder: '#ff00c3', text: 'white' }, light: { alertBg: '#ffc4f1', alertBorder: '#ff00c3', text: 'black' } };src/components/organisms/ProxyForm.tsx (1)
Accessibility concerns identified in dark mode color contrasts
The code reveals potential WCAG contrast issues:
- The pink accent color (#ff00c3) is used consistently across both light and dark modes without adjustment
- Alert component in dark mode uses a very dark purple background (#3b002d) with white text
- Tab indicators and hover states use the same pink (#FF69B4) regardless of theme
To ensure better accessibility:
- Consider using a lighter shade of pink for dark mode to maintain sufficient contrast
- Validate the contrast ratio between the dark purple background (#3b002d) and white text
- Ensure the pink accent colors meet a minimum contrast ratio of 4.5:1 for normal text and 3:1 for large text
🔗 Analysis chain
Line range hint
300-322: Consider improving accessibility for dark mode.While the color scheme is visually appealing, ensure all interactive elements meet WCAG contrast requirements in both themes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential accessibility issues in theme-related styles ast-grep --pattern 'style={{ color:' # Find inline color styles ast-grep --pattern 'sx={{ color:' # Find MUI sx color propsLength of output: 74
Script:
#!/bin/bash # Search for color-related styles in the ProxyForm component with more context rg -A 2 -B 2 "color:|backgroundColor:" "src/components/organisms/ProxyForm.tsx" # Search for theme-related configurations rg -A 5 "createTheme|ThemeProvider|useTheme|isDarkMode" "src/components/organisms/ProxyForm.tsx"Length of output: 3773
src/components/organisms/RightSidePanel.tsx (2)
Line range hint
444-474: Consolidate repeated theme-specific styles.Multiple components share similar theme-specific styling. Consider extracting these into a shared style configuration.
const commonStyles = { background: isDarkMode ? "#1E2124" : 'white', color: isDarkMode ? "white" : 'black' };
415-415: Standardize button styling across the component.Multiple buttons share the same styling properties. Consider creating a custom Button component with theme-aware styling.
const ThemedButton = styled(Button)(({ theme }) => ({ backgroundColor: "#ff00c3", color: theme.darkMode ? 'white' : 'black' }));Also applies to: 482-482, 491-491
src/index.css (1)
14-14: Remove unnecessary empty linesThese empty lines within the CSS rules don't serve any purpose and should be removed for better code cleanliness.
body { scrollbar-gutter: stable; overflow-y: auto; - } #browser-recorder { overflow: hidden; position: relative; - }Also applies to: 47-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (25)
src/App.tsx(1 hunks)src/components/atoms/Loader.tsx(2 hunks)src/components/atoms/RecorderIcon.tsx(1 hunks)src/components/atoms/buttons/buttons.tsx(1 hunks)src/components/atoms/canvas.tsx(1 hunks)src/components/molecules/ActionDescriptionBox.tsx(6 hunks)src/components/molecules/ActionSettings.tsx(3 hunks)src/components/molecules/BrowserNavBar.tsx(5 hunks)src/components/molecules/BrowserRecordingSave.tsx(1 hunks)src/components/molecules/BrowserTabs.tsx(4 hunks)src/components/molecules/IntegrationSettings.tsx(4 hunks)src/components/molecules/InterpretationLog.tsx(4 hunks)src/components/molecules/NavBar.tsx(2 hunks)src/components/molecules/RunContent.tsx(2 hunks)src/components/molecules/SaveRecording.tsx(1 hunks)src/components/organisms/BrowserContent.tsx(3 hunks)src/components/organisms/BrowserWindow.tsx(1 hunks)src/components/organisms/MainMenu.tsx(3 hunks)src/components/organisms/ProxyForm.tsx(4 hunks)src/components/organisms/RightSidePanel.tsx(11 hunks)src/context/theme-provider.tsx(1 hunks)src/index.css(3 hunks)src/pages/Login.tsx(1 hunks)src/pages/RecordingPage.tsx(4 hunks)src/pages/Register.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/components/atoms/RecorderIcon.tsx
- src/pages/Register.tsx
- src/components/atoms/canvas.tsx
- src/components/organisms/BrowserWindow.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/index.css
[error] 60-62: An empty block isn't allowed.
Consider removing the empty block or adding styles inside it.
(lint/suspicious/noEmptyBlock)
🔇 Additional comments (6)
src/components/molecules/InterpretationLog.tsx (1)
20-20: LGTM: Theme hook integration looks good!
The integration of the useThemeMode hook is clean and follows React best practices.
Also applies to: 117-118
src/components/atoms/Loader.tsx (1)
Line range hint 10-35: LGTM!
The implementation correctly adapts the loader text color based on the theme mode.
src/pages/Login.tsx (1)
41-41: LGTM: Proper CORS configuration
The addition of withCredentials: true is correct for handling cross-origin authentication properly.
src/components/molecules/IntegrationSettings.tsx (1)
30-42: LGTM: Cookie management implementation is secure and well-typed.
The custom cookie management functions provide a good replacement for the js-cookie library. The implementation follows security best practices by using HttpOnly cookies and proper expiration handling.
src/components/organisms/ProxyForm.tsx (1)
26-45: LGTM: Well-structured custom tab components with proper theme integration.
The custom styled components provide consistent styling and good user experience with proper hover states and selection indicators.
src/index.css (1)
60-62:
Add dark theme styles to the empty #browser selector
Given that this PR implements dark theme support, the empty #browser selector should include theme-specific styles. Consider adding background color, text color, and other relevant dark theme properties.
Let's verify the theme implementation in other files:
Example implementation:
#browser {
background-color: var(--background-color);
color: var(--text-color);
transition: background-color 0.3s ease, color 0.3s ease;
}
/* Dark theme */
[data-theme="dark"] #browser {
--background-color: #1a1a1a;
--text-color: #ffffff;
}
/* Light theme */
[data-theme="light"] #browser {
--background-color: #ffffff;
--text-color: #000000;
}🧰 Tools
🪛 Biome (1.9.4)
[error] 60-62: An empty block isn't allowed.
Consider removing the empty block or adding styles inside it.
(lint/suspicious/noEmptyBlock)
| @@ -159,6 +199,7 @@ export const RunContent = ({ row, currentLog, interpretationInProgress, logEndRe | |||
| background: 'rgba(0,0,0,0.06)', | |||
| maxHeight: '300px', | |||
| overflow: 'scroll', | |||
| backgroundColor: '#19171c' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use theme-aware background color
The background color should adapt to the current theme mode instead of using a hardcoded dark color.
- backgroundColor: '#19171c'
+ backgroundColor: theme => theme.palette.mode === 'dark' ? theme.palette.grey[900] : theme.palette.grey[100]📝 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.
| backgroundColor: '#19171c' | |
| backgroundColor: theme => theme.palette.mode === 'dark' ? theme.palette.grey[900] : theme.palette.grey[100] |
| background: `${isDarkMode ? '#1e2124' : 'white'}`, | ||
| color: `${isDarkMode ? 'white' : 'black'}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using MUI's theme system for better integration
Instead of directly using template literals for theme conditions, consider using MUI's theme system for better integration and maintainability:
PaperProps={{
sx: {
- background: `${isDarkMode ? '#1e2124' : 'white'}`,
- color: `${isDarkMode ? 'white' : 'black'}`,
+ bgcolor: 'background.paper',
+ color: 'text.primary',
padding: '10px',
height: 500,
width: width - 10,This approach:
- Uses MUI's theme tokens which automatically handle light/dark modes
- Provides better type safety
- Ensures consistency with MUI's theming system
📝 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.
| background: `${isDarkMode ? '#1e2124' : 'white'}`, | |
| color: `${isDarkMode ? 'white' : 'black'}`, | |
| bgcolor: 'background.paper', | |
| color: 'text.primary', |
| @@ -12,6 +12,7 @@ import { | |||
| } from "../../api/recording"; | |||
| import { Box } from "@mui/material"; | |||
| import { InterpretationLog } from "../molecules/InterpretationLog"; | |||
| import { Height } from "@mui/icons-material"; | |||
There was a problem hiding this comment.
Remove unused import Height
The Height icon from MUI is imported but never used in the component.
-import { Height } from "@mui/icons-material";| backgroundColor: '#f5f5f5', // Synced dark background for tabs | ||
| borderRadius: '8px 8px 0px 0px', | ||
| marginRight: '8px', | ||
| minHeight: '48px', | ||
| textTransform: 'none', | ||
| fontWeight: '500', | ||
| fontSize: '14px', | ||
| color: 'black', // Synced light gray text color | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adjust Tab styles to adapt to theme mode
The backgroundColor and color properties for the Tab component are hardcoded and do not change with the theme. To ensure consistent theming, adjust these properties based on isDarkMode.
Apply this diff to make the Tab styles adapt to the theme mode:
sx={{
- backgroundColor: '#f5f5f5', // Synced dark background for tabs
+ backgroundColor: `${isDarkMode ? '#2a2a2a' : '#f5f5f5'}`, // Adjust background color based on theme
borderRadius: '8px 8px 0px 0px',
marginRight: '8px',
minHeight: '48px',
textTransform: 'none',
fontWeight: '500',
fontSize: '14px',
- color: 'black', // Synced light gray text color
+ color: `${isDarkMode ? '#FFFFFF' : 'black'}`, // Adjust text color based on theme
'&.Mui-selected': {
backgroundColor: `${isDarkMode ? '#2a2a2a' : '#f5f5f5'}`, // Synced selected tab color
color: '#ff00c3', // Slightly lighter text when selected
},
}}Committable suggestion skipped: line range outside the PR's diff.
| export const NavBarButton = styled.button<{ disabled: boolean, mode: 'light' | 'dark' }>` | ||
| margin-left: 10px; | ||
| margin-right: 5px; | ||
| padding: 0; | ||
| border: none; | ||
| background-color: transparent; | ||
| background-color: ${mode => mode ? '#333' : '#ffffff'}; | ||
| cursor: ${({ disabled }) => disabled ? 'default' : 'pointer'}; | ||
| width: 24px; | ||
| height: 24px; | ||
| border-radius: 12px; | ||
| outline: none; | ||
| color: ${({ disabled }) => disabled ? '#999' : '#333'}; | ||
| color: ${mode => mode ? '#ffffff' : '#333333'}; | ||
|
|
||
|
|
There was a problem hiding this comment.
Fix style functions to correctly access the mode prop
In the styled component, the template literals should access the mode prop correctly. Currently, background-color: ${mode => mode ? ...} does not access the mode prop as intended.
Apply this diff to fix the issue:
export const NavBarButton = styled.button<{ disabled: boolean, mode: 'light' | 'dark' }>`
margin-left: 10px;
margin-right: 5px;
padding: 0;
border: none;
- background-color: ${mode => mode ? '#333' : '#ffffff'};
+ background-color: ${({ mode }) => mode === 'dark' ? '#333' : '#ffffff'};
cursor: ${({ disabled }) => disabled ? 'default' : 'pointer'};
width: 24px;
height: 24px;
border-radius: 12px;
outline: none;
- color: ${mode => mode ? '#ffffff' : '#333333'};
+ color: ${({ mode }) => mode === 'dark' ? '#ffffff' : '#333333'};
`;📝 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.
| export const NavBarButton = styled.button<{ disabled: boolean, mode: 'light' | 'dark' }>` | |
| margin-left: 10px; | |
| margin-right: 5px; | |
| padding: 0; | |
| border: none; | |
| background-color: transparent; | |
| background-color: ${mode => mode ? '#333' : '#ffffff'}; | |
| cursor: ${({ disabled }) => disabled ? 'default' : 'pointer'}; | |
| width: 24px; | |
| height: 24px; | |
| border-radius: 12px; | |
| outline: none; | |
| color: ${({ disabled }) => disabled ? '#999' : '#333'}; | |
| color: ${mode => mode ? '#ffffff' : '#333333'}; | |
| export const NavBarButton = styled.button<{ disabled: boolean, mode: 'light' | 'dark' }>` | |
| margin-left: 10px; | |
| margin-right: 5px; | |
| padding: 0; | |
| border: none; | |
| background-color: ${({ mode }) => mode === 'dark' ? '#333' : '#ffffff'}; | |
| cursor: ${({ disabled }) => disabled ? 'default' : 'pointer'}; | |
| width: 24px; | |
| height: 24px; | |
| border-radius: 12px; | |
| outline: none; | |
| color: ${({ mode }) => mode === 'dark' ? '#ffffff' : '#333333'}; | |
| interface CustomBoxContainerProps { | ||
| isDarkMode: boolean; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using MUI's theme system instead of prop drilling
The isDarkMode prop is being passed through multiple components when MUI's theme system could handle this more elegantly.
Consider using MUI's theme system:
interface CustomBoxContainerProps {
theme: Theme; // Use MUI's Theme type
}| background-color: ${({ isDarkMode }) => (isDarkMode ? '#313438' : 'white')}; | ||
| color: ${({ isDarkMode }) => (isDarkMode ? 'white' : 'black')}; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use theme tokens instead of hardcoded colors
Hardcoded colors (#313438, white) should be replaced with theme tokens for better maintainability and consistency.
Consider this implementation:
- background-color: ${({ isDarkMode }) => (isDarkMode ? '#313438' : 'white')};
- color: ${({ isDarkMode }) => (isDarkMode ? 'white' : 'black')};
+ background-color: ${({ theme }) => theme.palette.background.paper};
+ color: ${({ theme }) => theme.palette.text.primary};
- border-bottom: 20px solid ${({ isDarkMode }) => (isDarkMode ? '#313438' : 'white')};
+ border-bottom: 20px solid ${({ theme }) => theme.palette.background.paper};Also applies to: 33-33
src/pages/RecordingPage.tsx
Outdated
| if (darkMode) { | ||
|
|
||
| document.body.style.background = 'rgba(18,18,18,1)'; | ||
|
|
||
| } else { | ||
| document.body.style.background = 'radial-gradient(circle, rgba(255, 255, 255, 1) 0%, rgba(232, 191, 222, 1) 100%, rgba(255, 255, 255, 1) 100%)'; | ||
| } | ||
|
|
||
| return () => { | ||
| // Cleanup the background when leaving the page | ||
| document.body.style.background = ''; | ||
| document.body.style.filter = ''; | ||
| }; | ||
| }, []); | ||
| }, [darkMode]); |
There was a problem hiding this comment.
Avoid direct DOM manipulation for theming
Direct manipulation of document.body.style is not recommended for theming. This approach can lead to conflicts with other styles and doesn't leverage MUI's theme system.
Consider these alternatives:
- Use MUI's theme system with styled components:
const PageWrapper = styled('div')(({ theme }) => ({
background: theme.palette.mode === 'dark'
? theme.palette.background.default
: 'radial-gradient(circle, rgba(255, 255, 255, 1) 0%, rgba(232, 191, 222, 1) 100%, rgba(255, 255, 255, 1) 100%)',
minHeight: '100vh'
}));- Or use CSS variables for dynamic theming:
// In your theme setup
document.documentElement.style.setProperty(
'--page-background',
theme.palette.mode === 'dark'
? 'rgba(18,18,18,1)'
: 'radial-gradient(...)'
);
// In your styled component
const PageWrapper = styled('div')`
background: var(--page-background);
min-height: 100vh;
`;
src/pages/RecordingPage.tsx
Outdated
| @@ -159,4 +164,5 @@ const RecordingPageWrapper = styled.div` | |||
| width: 100vw; | |||
| height: 100vh; | |||
| overflow: hidden; | |||
|
|
|||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add theme-aware background to RecordingPageWrapper
The styled RecordingPageWrapper component should include theme-aware background styling.
const RecordingPageWrapper = styled.div`
position: relative;
width: 100vw;
height: 100vh;
overflow: hidden;
+ background: ${({ theme }) => theme.palette.background.default};
`;Committable suggestion skipped: line range outside the PR's diff.
| <Paper | ||
| sx={{ | ||
| borderRadius: '8px 8px 0px 0px', // Slightly more rounded corners for a smoother look | ||
| height: '520px', | ||
| width: 'auto', | ||
| alignItems: "center", | ||
| padding: '16px', // Add padding for spacing inside the component | ||
| background: isDarkMode | ||
| ? 'linear-gradient(135deg, #1E2124 0%, #292C2F 100%)' // Subtle gradient for dark mode | ||
| : 'linear-gradient(135deg, #f5f5f5 0%, #ffffff 100%)', // Subtle gradient for light mode | ||
| color: isDarkMode ? '#E0E0E0' : '#333333', // Adjusted color for better contrast | ||
| // Smooth transition for shadows and background changes | ||
| }} | ||
| id="browser-actions" | ||
| elevation={0} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using MUI's theme system for consistent styling.
The current implementation uses inline styles for theme-specific colors. Consider leveraging MUI's theme system for better maintainability and consistency.
Example implementation:
const theme = createTheme({
palette: {
mode: isDarkMode ? 'dark' : 'light',
background: {
paper: isDarkMode
? 'linear-gradient(135deg, #1E2124 0%, #292C2F 100%)'
: 'linear-gradient(135deg, #f5f5f5 0%, #ffffff 100%)'
}
}
});There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
src/pages/PageWrappper.tsx (2)
15-15: Remove unused importThe
AppBarcomponent from@mui/materialis imported but never used in the code.-import { AppBar } from '@mui/material';
54-56: Consider simplifying the NavBar conditional renderingThe current conditional rendering can be simplified by moving the condition inside the NavBar component or using the logical AND operator.
-{!browserId && <NavBar recordingName={recordingName} isRecording={!!browserId} />} +<NavBar + recordingName={recordingName} + isRecording={!!browserId} + show={!browserId} +/>src/components/molecules/NavBar.tsx (4)
101-108: Enhance iframe accessibility and performanceThe GitHub button iframe could benefit from some improvements:
- Loading attribute for performance
- More descriptive title for accessibility
<iframe src="https://ghbtns.com/github-btn.html?user=getmaxun&repo=maxun&type=star&count=true&size=large" frameBorder="0" scrolling="0" width="170" height="30" - title="GitHub" + title="GitHub Star Button for Maxun Repository" + loading="lazy" />
190-226: Consider moving styles to a separate fileThe styles object is quite large and could be moved to a separate file for better maintainability.
Consider creating a new file
NavBar.styles.ts:// NavBar.styles.ts export const styles = { socialButton: {...}, userButton: (darkMode: boolean) => ({...}), discardButton: {...} };
231-235: Consider using MUI's theme system for consistent stylingInstead of using styled-components with manual theme checks, consider leveraging MUI's theme system for better integration and consistency.
import { styled as muiStyled } from '@mui/material/styles'; const NavBarWrapper = muiStyled('div')(({ theme }) => ({ gridArea: 'navbar', backgroundColor: theme.palette.background.paper, padding: theme.spacing(0.5), display: 'flex', justifyContent: 'space-between', borderBottom: `1px solid ${theme.palette.divider}` }));
68-82: Consider component composition for better maintainabilityThe current approach of using render methods could be improved by breaking down into smaller components.
Consider creating separate components:
// components/NavBar/BrandSection.tsx const BrandSection = () => {...} // components/NavBar/SocialButtons.tsx const SocialButtons = () => {...} // components/NavBar/UserMenu.tsx const UserMenu = () => {...} // components/NavBar/ThemeToggle.tsx const ThemeToggle = () => {...} // components/NavBar/RecordingControls.tsx const RecordingControls = () => {...}This would improve:
- Code organization and maintainability
- Testing capabilities
- Potential for code reuse
Also applies to: 84-110, 112-139, 141-155, 157-168
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/molecules/NavBar.tsx(2 hunks)src/pages/Login.tsx(3 hunks)src/pages/PageWrappper.tsx(2 hunks)src/pages/Register.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Login.tsx
- src/pages/Register.tsx
| const { state, dispatch } = useContext(AuthContext); | ||
| const { user } = state; | ||
| const navigate = useNavigate(); | ||
|
|
||
| const { darkMode, toggleTheme } = useThemeMode(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding a theme persistence mechanism
The theme preference should persist across page reloads for better user experience.
Consider storing the theme preference in localStorage:
const { darkMode, toggleTheme } = useThemeMode();
useEffect(() => {
localStorage.setItem('theme', darkMode ? 'dark' : 'light');
}, [darkMode]);
src/components/molecules/NavBar.tsx
Outdated
| background: 'red', | ||
| color: 'white', | ||
| marginRight: '10px', | ||
| '&:hover': { | ||
| color: 'white', | ||
| backgroundColor: '#ff0000' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error prevention UI
The discard button's styling suggests a destructive action. Consider adding a confirmation dialog before discarding.
const [showConfirmDialog, setShowConfirmDialog] = useState(false);
const handleDiscard = () => {
setShowConfirmDialog(true);
};
// Add Dialog component
<Dialog
open={showConfirmDialog}
onClose={() => setShowConfirmDialog(false)}
>
<DialogTitle>Confirm Discard</DialogTitle>
<DialogContent>
Are you sure you want to discard the current recording?
</DialogContent>
<DialogActions>
<Button onClick={() => setShowConfirmDialog(false)}>Cancel</Button>
<Button onClick={goToMainMenu} color="error">Discard</Button>
</DialogActions>
</Dialog>There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/molecules/ScheduleSettings.tsx (1)
Refine theme integration for consistent styling
The component already uses MUI's theming system but needs refinement:
- Replace hardcoded
#ff00c3color with appropriate theme color- Replace hardcoded
whitesmokewith theme-aware background color- Add theme typing for better type safety
import React, { useState, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; +import { useTheme } from '@mui/material/styles'; +import type { Theme } from '@mui/material/styles'; import { GenericModal } from "../atoms/GenericModal";Replace hardcoded styles with theme-aware alternatives:
- sx={{ - color: '#ff00c3 !important', - borderColor: '#ff00c3 !important', - '&:hover': { - borderColor: '#ff00c3 !important', - backgroundColor: 'whitesmoke !important', - } - }} + sx={(theme: Theme) => ({ + color: theme.palette.secondary.main, + borderColor: theme.palette.secondary.main, + '&:hover': { + borderColor: theme.palette.secondary.main, + backgroundColor: theme.palette.action.hover, + } + })}🔗 Analysis chain
Line range hint
1-4: Add proper theme integration to the componentThe component needs proper theme integration to fully support dark mode.
Add the following imports and types:
import React, { useState, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; +import { useTheme } from '@mui/material/styles'; import { GenericModal } from "../atoms/GenericModal";Let's verify the theme integration in related components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check theme integration in related components echo "Checking theme integration in related components..." rg -l "useTheme|ThemeProvider" src/components/Length of output: 522
Script:
#!/bin/bash # Check ScheduleSettings implementation and similar components echo "Checking ScheduleSettings implementation..." rg -A 5 "style|color|background|theme" src/components/molecules/ScheduleSettings.tsx echo -e "\nChecking theme usage patterns in similar components..." for file in $(rg -l "useTheme" src/components/molecules/); do echo -e "\n=== $file ===" rg "const.*theme.*=.*useTheme\(\)" -A 3 "$file" doneLength of output: 2495
♻️ Duplicate comments (3)
src/components/molecules/NavBar.tsx (3)
106-120: 🛠️ Refactor suggestionReplace hardcoded colors with theme tokens
The theme toggle button uses hardcoded color values that should be using theme tokens for consistency.
const renderThemeToggle = () => ( <Tooltip title="Toggle light/dark theme"> <IconButton onClick={toggleTheme} sx={{ - color: darkMode ? '#ffffff' : '#333333', + color: darkMode ? 'primary.contrastText' : 'text.primary', '&:hover': { - color: '#ff00c3' + color: 'primary.main' } }} > {darkMode ? <Brightness7 /> : <Brightness4 />} </IconButton> </Tooltip> );
178-184: 🛠️ Refactor suggestionAdd theme persistence across page reloads
The theme mode should persist across page reloads for better user experience.
+ useEffect(() => { + const savedTheme = localStorage.getItem('theme'); + if (savedTheme && savedTheme !== (darkMode ? 'dark' : 'light')) { + toggleTheme(); + } + }, []); + useEffect(() => { + localStorage.setItem('theme', darkMode ? 'dark' : 'light'); + }, [darkMode]); return ( <NavBarWrapper mode={darkMode ? 'dark' : 'light'}>
493-513: 🛠️ Refactor suggestionCentralize theme configuration
Move theme colors to a central configuration for better maintainability and consistency.
+ // src/theme/constants.ts + export const themeColors = { + light: { + background: '#ffffff', + text: '#3f4853', + border: '#e0e0e0' + }, + dark: { + background: '#1e2124', + text: '#ffffff', + border: '#333' + } + }; const NavBarWrapper = styled.div<{ mode: 'light' | 'dark' }>` grid-area: navbar; - background-color: ${({ mode }) => (mode === 'dark' ? '#1e2124' : '#ffffff')}; + background-color: ${({ mode }) => themeColors[mode].background}; padding: 5px; display: flex; justify-content: space-between; - border-bottom: 1px solid ${({ mode }) => (mode === 'dark' ? '#333' : '#e0e0e0')}; + border-bottom: 1px solid ${({ mode }) => themeColors[mode].border}; `; const ProjectName = styled.b<{ mode: 'light' | 'dark' }>` - color: ${({ mode }) => (mode === 'dark' ? '#ffffff' : '#3f4853')}; + color: ${({ mode }) => themeColors[mode].text}; font-size: 1.3em; `;
🧹 Nitpick comments (2)
src/components/molecules/ScheduleSettings.tsx (1)
276-288: Maintain consistency with existing theme implementationThe modal already uses theme-aware styling (e.g.,
backgroundColor: 'background.paper'). The button styling should follow the same pattern for consistency with the rest of the application's theme implementation.Consider these architectural improvements:
- Use the same theme-aware approach as other components mentioned in the changes (e.g.,
ActionSettings,NavBarButton)- If custom colors are needed, define them in the theme's palette instead of hard-coding them
- Consider extracting common button styles into a shared theme component configuration
src/components/molecules/NavBar.tsx (1)
7-8: Consider organizing imports by categoryGroup related imports together for better maintainability:
- React and core dependencies
- Material-UI components
- Material-UI icons
- Local components and utilities
- Assets and types
import { useTranslation } from "react-i18next"; import React, { useState, useContext, useEffect } from 'react'; import axios from 'axios'; import styled from "styled-components"; + import { useNavigate } from 'react-router-dom'; + import { + IconButton, Menu, MenuItem, Typography, + Chip, Button, Modal, Tabs, Tab, Box, + Snackbar, Tooltip + } from "@mui/material"; + import { + AccountCircle, Logout, Clear, YouTube, X, + Update, Close, Language, Brightness7, Brightness4 + } from "@mui/icons-material"; import { stopRecording } from "../../api/recording"; import { useGlobalInfoStore } from "../../context/globalInfo"; - import { IconButton, Menu, MenuItem, Typography, Chip, Button, Modal, Tabs, Tab, Box, Snackbar, Tooltip } from "@mui/material"; - import { AccountCircle, Logout, Clear, YouTube, X, Update, Close, Language, Brightness7, Brightness4 } from "@mui/icons-material"; - import { useNavigate } from 'react-router-dom'; import { AuthContext } from '../../context/auth'; import { SaveRecording } from '../molecules/SaveRecording'; import DiscordIcon from '../atoms/DiscordIcon'; import { apiUrl } from '../../apiConfig'; import { useThemeMode } from '../../context/theme-provider'; import MaxunLogo from "../../assets/maxunlogo.png"; import packageJson from "../../../package.json"Also applies to: 15-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/molecules/BrowserRecordingSave.tsx(2 hunks)src/components/molecules/NavBar.tsx(7 hunks)src/components/molecules/RobotDuplicate.tsx(1 hunks)src/components/molecules/RobotEdit.tsx(1 hunks)src/components/molecules/ScheduleSettings.tsx(1 hunks)src/components/organisms/BrowserWindow.tsx(2 hunks)src/components/organisms/RightSidePanel.tsx(12 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/molecules/RobotEdit.tsx
- src/components/molecules/RobotDuplicate.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/organisms/BrowserWindow.tsx
- src/components/molecules/BrowserRecordingSave.tsx
- src/components/organisms/RightSidePanel.tsx
🔇 Additional comments (1)
src/components/molecules/NavBar.tsx (1)
31-31: Verify theme implementation consistencyLet's verify that the theme implementation is consistent across all components using the theme provider.
✅ Verification successful
Theme implementation verified ✓
The theme implementation is consistent across the application with proper usage of the theme provider, hooks, and styled-components. The dark/light mode transitions are handled systematically while maintaining brand identity colors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using theme provider echo "Components using theme provider:" rg -l "useThemeMode|ThemeModeProvider" "src/" # Search for hardcoded color values that might need theme support echo -e "\nPotential hardcoded colors that might need theme support:" rg -n "'#[0-9a-fA-F]{3,6}'" "src/components/" # Search for styled-components using theme props echo -e "\nComponents using theme props:" ast-grep --pattern 'styled.$_<{ mode: $_}>'Length of output: 9539
| <Button | ||
| onClick={handleClose} | ||
| color="primary" | ||
| variant="outlined" | ||
| style={{ marginLeft: '10px' }} | ||
| sx={{ | ||
| color: '#ff00c3 !important', | ||
| borderColor: '#ff00c3 !important', | ||
| '&:hover': { | ||
| borderColor: '#ff00c3 !important', | ||
| backgroundColor: 'whitesmoke !important', | ||
| } | ||
| }}> |
There was a problem hiding this comment.
Refactor button styling to support dark theme properly
The current implementation has several issues that need to be addressed:
- Hard-coded color values don't support dark theme
- Excessive use of
!importantflags indicates potential styling conflicts - No integration with Material-UI's theme system
Please refactor the button styling to use Material-UI's theme system:
- <Button
- onClick={handleClose}
- color="primary"
- variant="outlined"
- style={{ marginLeft: '10px' }}
- sx={{
- color: '#ff00c3 !important',
- borderColor: '#ff00c3 !important',
- '&:hover': {
- borderColor: '#ff00c3 !important',
- backgroundColor: 'whitesmoke !important',
- }
- }}>
+ <Button
+ onClick={handleClose}
+ color="secondary"
+ variant="outlined"
+ sx={(theme) => ({
+ marginLeft: '10px',
+ borderColor: theme.palette.secondary.main,
+ color: theme.palette.secondary.main,
+ '&:hover': {
+ borderColor: theme.palette.secondary.main,
+ backgroundColor: theme.palette.action.hover,
+ }
+ })}>Also, ensure that the theme's palette includes appropriate secondary colors for both light and dark modes in your theme configuration file.
📝 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.
| <Button | |
| onClick={handleClose} | |
| color="primary" | |
| variant="outlined" | |
| style={{ marginLeft: '10px' }} | |
| sx={{ | |
| color: '#ff00c3 !important', | |
| borderColor: '#ff00c3 !important', | |
| '&:hover': { | |
| borderColor: '#ff00c3 !important', | |
| backgroundColor: 'whitesmoke !important', | |
| } | |
| }}> | |
| <Button | |
| onClick={handleClose} | |
| color="secondary" | |
| variant="outlined" | |
| sx={(theme) => ({ | |
| marginLeft: '10px', | |
| borderColor: theme.palette.secondary.main, | |
| color: theme.palette.secondary.main, | |
| '&:hover': { | |
| borderColor: theme.palette.secondary.main, | |
| backgroundColor: theme.palette.action.hover, | |
| } | |
| })}> |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/organisms/RightSidePanel.tsx (3)
619-623: Centralize TextField styles using MUI's theme system.TextField components have duplicate theme-dependent styles. Consider using MUI's theme system for consistent styling.
// src/theme/components.ts export const getMuiComponents = (isDarkMode: boolean) => ({ MuiTextField: { styleOverrides: { root: { backgroundColor: isDarkMode ? "#1E2124" : 'white', color: isDarkMode ? "white" : 'black', } } } });Also applies to: 704-704
683-683: Centralize Box styles using MUI's theme system.Box components have duplicate theme-dependent styles. Consider using MUI's theme system for consistent styling.
// src/theme/components.ts export const getMuiComponents = (isDarkMode: boolean) => ({ MuiBox: { styleOverrides: { root: { backgroundColor: isDarkMode ? "#1E2124" : 'white', color: isDarkMode ? "white" : 'black', } } } });Also applies to: 753-753
635-659: Remove unnecessary Fragment.The Fragment wrapping the Box component is redundant as it contains only one child.
- {getText && - <> <Box display="flex" justifyContent="space-between" gap={2} style={{ margin: '15px' }}> {/* ... */} </Box> - </> - } + {getText && + <Box display="flex" justifyContent="space-between" gap={2} style={{ margin: '15px' }}> + {/* ... */} + </Box> + }🧰 Tools
🪛 Biome (1.9.4)
[error] 635-659: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/organisms/RightSidePanel.tsx(10 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/organisms/RightSidePanel.tsx
[error] 635-659: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (1)
src/components/organisms/RightSidePanel.tsx (1)
465-467: Consider using MUI's theme system for consistent styling.The current implementation directly uses the theme context. Consider leveraging MUI's theme system for better maintainability and consistency.
Also applies to: 473-474
| sx={{ | ||
| color: '#ff00c3 !important', | ||
| borderColor: '#ff00c3 !important', | ||
| backgroundColor: 'whitesmoke !important', | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Centralize button styles and remove !important declarations.
The current implementation has several issues:
- Duplicate button styles across multiple buttons
- Overuse of
!importantdeclarations - Hardcoded colors instead of theme variables
Consider creating a centralized button style configuration:
// src/theme/components.ts
export const getButtonStyles = (variant: 'primary' | 'error', isDarkMode: boolean) => ({
primary: {
color: variant === 'contained' ? 'whitesmoke' : '#ff00c3',
borderColor: '#ff00c3',
backgroundColor: variant === 'contained' ? '#ff00c3' : 'whitesmoke',
},
error: {
color: 'red',
borderColor: 'red',
backgroundColor: 'whitesmoke',
}
});Then use it in your components:
<Button
variant="outlined"
sx={getButtonStyles('primary', isDarkMode)}
onClick={handleBackCaptureList}
>
{t('right_panel.buttons.back')}
</Button>Also applies to: 497-501, 508-516, 525-534, 535-544, 545-554, 555-564, 565-573, 637-646, 647-657, 667-677
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/organisms/MainMenu.tsx (3)
29-31: Follow JavaScript naming conventionsThe variable name should be camelCase for consistency with JavaScript conventions.
-const defaultcolor = theme.palette.mode === 'light' ? 'black' : 'white'; +const defaultColor = theme.palette.mode === 'light' ? 'black' : 'white';
32-43: Use theme palette colors instead of hardcoded valuesReplace the hardcoded color
#6C6C6Cwith an appropriate color from the theme palette for better theme consistency.- color: theme.palette.mode === 'light' ? '#6C6C6C' : 'inherit', + color: theme.palette.mode === 'light' ? theme.palette.text.secondary : 'inherit',
Line range hint
58-108: Consolidate repeated Tab stylesThe same styles are repeated for each Tab component. Consider extracting these into a common style object.
+ const tabStyles = { + justifyContent: 'flex-start', + textAlign: 'left', + fontSize: 'medium', + }; // Then in each Tab component: <Tab - sx={{ - justifyContent: 'flex-start', - textAlign: 'left', - fontSize: 'medium', - }} + sx={tabStyles} // ... other props />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/atoms/Loader.tsx(2 hunks)src/components/molecules/BrowserNavBar.tsx(5 hunks)src/components/molecules/BrowserTabs.tsx(4 hunks)src/components/molecules/NavBar.tsx(7 hunks)src/components/molecules/SaveRecording.tsx(1 hunks)src/components/organisms/MainMenu.tsx(1 hunks)src/components/organisms/ProxyForm.tsx(4 hunks)src/pages/RecordingPage.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/molecules/BrowserTabs.tsx
- src/components/organisms/ProxyForm.tsx
- src/components/molecules/SaveRecording.tsx
- src/components/molecules/BrowserNavBar.tsx
- src/pages/RecordingPage.tsx
🔇 Additional comments (9)
src/components/atoms/Loader.tsx (2)
3-3: Theme integration looks good!The theme context is properly integrated with the component, and the darkMode state is correctly passed to the styled component.
Also applies to: 10-11, 20-20
29-33: Consider using theme tokens for colorsThe hardcoded colors should be replaced with theme tokens for better maintainability and consistency.
- color: ${({ darkMode }) => (darkMode ? 'white' : '#333')}; + color: ${({ darkMode }) => (darkMode ? themeColors.dark.text : themeColors.light.text)};src/components/molecules/NavBar.tsx (5)
106-120: Theme toggle implementation looks good!The theme toggle is well-implemented with proper tooltip and icon changes based on the current theme.
111-114: Replace hardcoded colors with theme tokensThe IconButton styles use hardcoded color values that should be replaced with theme tokens.
- color: darkMode ? '#ffffff' : '#333333', - '&:hover': { - color: '#ff00c3' - } + color: darkMode ? themeColors.dark.text : themeColors.light.text, + '&:hover': { + color: themeColors.accent.primary + }
492-499: NavBarWrapper theme implementation looks good!The styled component properly handles theme-based styling with appropriate color changes for background and border.
507-512: NavBarRight implementation looks good!The styled component follows best practices for flex layout and positioning.
31-31: Consider persisting theme preferenceThe theme preference should be saved to localStorage for a better user experience.
useEffect(() => { localStorage.setItem('theme', darkMode ? 'dark' : 'light'); }, [darkMode]);src/components/organisms/MainMenu.tsx (2)
1-19: LGTM! Clean imports and well-defined TypeScript interface.
Line range hint
1-124: Well-implemented dark theme support!The implementation properly integrates with Material-UI's theming system and handles both light and dark modes effectively. The use of theme-aware styling ensures consistent appearance across theme changes.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/components/atoms/canvas.tsx (5)
29-29: Consider re-addingReact.memofor performance optimization.The
Canvascomponent was previously wrapped withReact.memo, which prevents unnecessary re-renders when the props remain unchanged. RemovingReact.memomight lead to performance degradation if the component re-renders frequently without prop changes.Apply this diff to re-add
React.memo:-const Canvas = ({ width, height, onCreateRef }: CanvasProps) => { +const Canvas = React.memo(({ width, height, onCreateRef }: CanvasProps) => {
163-177: Ensure consistent dependency arrays inonKeyboardEventcallback.The
onKeyboardEventcallback depends onsocketbut might also rely on other variables likelastMousePosition. Check if all necessary dependencies are included to prevent stale closures.Consider including
lastMousePositionin the dependency array:const onKeyboardEvent = useCallback((event: KeyboardEvent) => { if (socket) { // Existing logic... } -}, [socket]); +}, [socket, lastMousePosition]);
77-111: Ensure proper cleanup of socket event listeners.The
useEffecthook adds event listeners tosocketbut only removes them when the component unmounts orsocketchanges. Ifsocketisnullor changes, the listeners might not be cleaned up properly.Modify the cleanup function to handle cases where
socketisnull:useEffect(() => { if (socket) { // Existing logic... return () => { socket.off('showDatePicker'); socket.off('showDropdown'); socket.off('showTimePicker'); socket.off('showDateTimePicker'); }; } + return undefined; }, [socket]);Additionally, consider handling the scenario where
socketchanges to a new instance.
6-9: Optimize imports by using dynamic imports for better performance.Previously, the components like
DatePicker,Dropdown, etc., might have been loaded lazily. Consider reintroducing lazy loading to improve initial load performance.Example:
-import DatePicker from './DatePicker'; -import Dropdown from './Dropdown'; -import TimePicker from './TimePicker'; -import DateTimeLocalPicker from './DateTimeLocalPicker'; +const DatePicker = React.lazy(() => import('./DatePicker')); +const Dropdown = React.lazy(() => import('./Dropdown')); +const TimePicker = React.lazy(() => import('./TimePicker')); +const DateTimeLocalPicker = React.lazy(() => import('./DateTimeLocalPicker'));Ensure to wrap the usage of these components with
<Suspense>to handle the loading state.
214-243: Addkeyprops to lists of React elements.When rendering lists or dynamic components, React expects a
keyprop to help identify which items have changed. Ensure that if any of the picker components are rendered in a list or dynamic context, they include a uniquekeyprop.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/atoms/canvas.tsx(2 hunks)src/components/molecules/BrowserRecordingSave.tsx(2 hunks)src/components/molecules/RobotDuplicate.tsx(1 hunks)src/components/molecules/RobotEdit.tsx(1 hunks)src/components/molecules/ScheduleSettings.tsx(1 hunks)src/components/organisms/BrowserContent.tsx(1 hunks)src/pages/Login.tsx(5 hunks)src/pages/Register.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/organisms/BrowserContent.tsx
- src/components/molecules/RobotEdit.tsx
- src/pages/Login.tsx
- src/components/molecules/ScheduleSettings.tsx
- src/components/molecules/RobotDuplicate.tsx
- src/components/molecules/BrowserRecordingSave.tsx
- src/pages/Register.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/atoms/canvas.tsx
[error] 148-148: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 149-152: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (1)
src/components/atoms/canvas.tsx (1)
214-243: Verify the conditional rendering of picker components.Multiple picker components are conditionally rendered based on their respective state variables. Ensure that only one picker is displayed at a time and that there are no conflicts in their visibility.
Would you like me to help test the rendering logic or open a GitHub issue to track any potential conflicts?
| if (canvasRef.current) { | ||
| onCreateRef(canvasRef); | ||
| canvasRef.current.addEventListener('mousedown', onMouseEvent); | ||
| canvasRef.current.addEventListener('mousemove', onMouseEvent); | ||
| canvasRef.current.addEventListener('wheel', onMouseEvent, { passive: true }); | ||
| canvasRef.current.addEventListener('keydown', onKeyboardEvent); | ||
| canvasRef.current.addEventListener('keyup', onKeyboardEvent); | ||
|
|
||
| return () => { | ||
| if (canvasRef.current) { | ||
| canvasRef.current.removeEventListener('mousedown', onMouseEvent); | ||
| canvasRef.current.removeEventListener('mousemove', onMouseEvent); | ||
| canvasRef.current.removeEventListener('wheel', onMouseEvent); | ||
| canvasRef.current.removeEventListener('keydown', onKeyboardEvent); | ||
| canvasRef.current.removeEventListener('keyup', onKeyboardEvent); | ||
| } | ||
|
|
||
| Object.entries(handlers).forEach(([event, handler]) => socket.on(event, handler)); | ||
| return () => { | ||
| Object.keys(handlers).forEach(event => socket.off(event)); | ||
| }; | ||
| }, [socket]); | ||
| }; | ||
| } else { | ||
| console.log('Canvas not initialized'); | ||
| } | ||
|
|
||
| const memoizedDimensions = useMemo(() => ({ | ||
| width: width || 900, | ||
| height: height || 400 | ||
| }), [width, height]); | ||
| }, [onMouseEvent]); |
There was a problem hiding this comment.
Add onKeyboardEvent to the dependency array of useEffect.
The useEffect hook adds event listeners that depend on onMouseEvent and onKeyboardEvent, but only onMouseEvent is included in the dependency array. Omitting onKeyboardEvent may cause the effect to not run when onKeyboardEvent changes.
Apply this diff to include onKeyboardEvent:
useEffect(() => {
if (canvasRef.current) {
// Existing logic...
}
- }, [onMouseEvent]);
+ }, [onMouseEvent, onKeyboardEvent]);📝 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.
| if (canvasRef.current) { | |
| onCreateRef(canvasRef); | |
| canvasRef.current.addEventListener('mousedown', onMouseEvent); | |
| canvasRef.current.addEventListener('mousemove', onMouseEvent); | |
| canvasRef.current.addEventListener('wheel', onMouseEvent, { passive: true }); | |
| canvasRef.current.addEventListener('keydown', onKeyboardEvent); | |
| canvasRef.current.addEventListener('keyup', onKeyboardEvent); | |
| return () => { | |
| if (canvasRef.current) { | |
| canvasRef.current.removeEventListener('mousedown', onMouseEvent); | |
| canvasRef.current.removeEventListener('mousemove', onMouseEvent); | |
| canvasRef.current.removeEventListener('wheel', onMouseEvent); | |
| canvasRef.current.removeEventListener('keydown', onKeyboardEvent); | |
| canvasRef.current.removeEventListener('keyup', onKeyboardEvent); | |
| } | |
| Object.entries(handlers).forEach(([event, handler]) => socket.on(event, handler)); | |
| return () => { | |
| Object.keys(handlers).forEach(event => socket.off(event)); | |
| }; | |
| }, [socket]); | |
| }; | |
| } else { | |
| console.log('Canvas not initialized'); | |
| } | |
| const memoizedDimensions = useMemo(() => ({ | |
| width: width || 900, | |
| height: height || 400 | |
| }), [width, height]); | |
| }, [onMouseEvent]); | |
| if (canvasRef.current) { | |
| onCreateRef(canvasRef); | |
| canvasRef.current.addEventListener('mousedown', onMouseEvent); | |
| canvasRef.current.addEventListener('mousemove', onMouseEvent); | |
| canvasRef.current.addEventListener('wheel', onMouseEvent, { passive: true }); | |
| canvasRef.current.addEventListener('keydown', onKeyboardEvent); | |
| canvasRef.current.addEventListener('keyup', onKeyboardEvent); | |
| return () => { | |
| if (canvasRef.current) { | |
| canvasRef.current.removeEventListener('mousedown', onMouseEvent); | |
| canvasRef.current.removeEventListener('mousemove', onMouseEvent); | |
| canvasRef.current.removeEventListener('wheel', onMouseEvent); | |
| canvasRef.current.removeEventListener('keydown', onKeyboardEvent); | |
| canvasRef.current.removeEventListener('keyup', onKeyboardEvent); | |
| } | |
| }; | |
| } else { | |
| console.log('Canvas not initialized'); | |
| } | |
| }, [onMouseEvent, onKeyboardEvent]); |
| const wheelEvent = event as WheelEvent; | ||
| const deltas = { | ||
| deltaX: Math.round(wheelEvent.deltaX), | ||
| deltaY: Math.round(wheelEvent.deltaY) | ||
| }); | ||
| setLastAction('scroll'); | ||
| }); | ||
| break; | ||
| deltaY: Math.round(wheelEvent.deltaY), | ||
| }; |
There was a problem hiding this comment.
Wrap declarations in switch cases to prevent hoisting issues.
Variables declared within a switch case are hoisted to the function scope, which can lead to unexpected behavior. According to the static analysis hints, the wheelEvent and deltas declarations should be wrapped in a block.
Apply this diff to wrap the declarations:
case 'wheel':
+ {
const wheelEvent = event as WheelEvent;
const deltas = {
deltaX: Math.round(wheelEvent.deltaX),
deltaY: Math.round(wheelEvent.deltaY),
};
socket.emit('input:wheel', deltas);
notifyLastAction('scroll');
+ }
break;This ensures the variables are scoped correctly within the case.
📝 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.
| const wheelEvent = event as WheelEvent; | |
| const deltas = { | |
| deltaX: Math.round(wheelEvent.deltaX), | |
| deltaY: Math.round(wheelEvent.deltaY) | |
| }); | |
| setLastAction('scroll'); | |
| }); | |
| break; | |
| deltaY: Math.round(wheelEvent.deltaY), | |
| }; | |
| { | |
| const wheelEvent = event as WheelEvent; | |
| const deltas = { | |
| deltaX: Math.round(wheelEvent.deltaX), | |
| deltaY: Math.round(wheelEvent.deltaY), | |
| }; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 148-148: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 149-152: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
| <div style={{ borderRadius: '0px 0px 5px 5px', overflow: 'hidden', backgroundColor: 'white' }}> | ||
| <canvas | ||
| tabIndex={0} | ||
| ref={canvasRef} | ||
| height={memoizedDimensions.height} | ||
| width={memoizedDimensions.width} | ||
| className="block" | ||
| height={400} | ||
| width={900} | ||
| style={{ display: 'block' }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make height and width props dynamic.
Currently, the height and width are hardcoded to 400 and 900, respectively, despite being received as props. To make the component more flexible and responsive, use the height and width props.
Apply this diff to use dynamic dimensions:
<canvas
tabIndex={0}
ref={canvasRef}
- height={400}
- width={900}
+ height={height}
+ width={width}
style={{ display: 'block' }}
/>This change allows the Canvas component to adjust its size based on the props passed.
Committable suggestion skipped: line range outside the PR's diff.
| if (socket && canvasRef.current) { | ||
| // Get the canvas bounding rectangle | ||
| const rect = canvasRef.current.getBoundingClientRect(); | ||
| const clickCoordinates = { | ||
| x: event.clientX - rect.left, // Use relative x coordinate | ||
| y: event.clientY - rect.top, // Use relative y coordinate | ||
| }; | ||
|
|
||
| switch (event.type) { | ||
| case 'mousedown': | ||
| debouncer.current.add(() => { | ||
| if (refs.current.getText) { | ||
| switch (event.type) { | ||
| case 'mousedown': | ||
| if (getTextRef.current === true) { | ||
| console.log('Capturing Text...'); | ||
| } else if (refs.current.getList) { | ||
| } else if (getListRef.current === true) { | ||
| console.log('Capturing List...'); | ||
| } else { | ||
| socket.emit('input:mousedown', coordinates); | ||
| socket.emit('input:mousedown', clickCoordinates); | ||
| } | ||
| setLastAction('click'); | ||
| }, true); // High priority | ||
| break; | ||
|
|
||
| case 'mousemove': | ||
| if (refs.current.lastMousePosition.x !== coordinates.x || | ||
| refs.current.lastMousePosition.y !== coordinates.y) { | ||
| debouncer.current.add(() => { | ||
| refs.current.lastMousePosition = coordinates; | ||
| socket.emit('input:mousemove', coordinates); | ||
| setLastAction('move'); | ||
| }); | ||
| } | ||
| break; | ||
|
|
||
| case 'wheel': | ||
| const wheelEvent = event as WheelEvent; | ||
| debouncer.current.add(() => { | ||
| socket.emit('input:wheel', { | ||
| notifyLastAction('click'); | ||
| break; | ||
| case 'mousemove': | ||
| if (lastMousePosition.current.x !== clickCoordinates.x || | ||
| lastMousePosition.current.y !== clickCoordinates.y) { | ||
| lastMousePosition.current = { | ||
| x: clickCoordinates.x, | ||
| y: clickCoordinates.y, | ||
| }; | ||
| socket.emit('input:mousemove', { | ||
| x: clickCoordinates.x, | ||
| y: clickCoordinates.y, | ||
| }); | ||
| notifyLastAction('move'); | ||
| } | ||
| break; | ||
| case 'wheel': | ||
| const wheelEvent = event as WheelEvent; | ||
| const deltas = { | ||
| deltaX: Math.round(wheelEvent.deltaX), | ||
| deltaY: Math.round(wheelEvent.deltaY) | ||
| }); | ||
| setLastAction('scroll'); | ||
| }); | ||
| break; | ||
| deltaY: Math.round(wheelEvent.deltaY), | ||
| }; | ||
| socket.emit('input:wheel', deltas); | ||
| notifyLastAction('scroll'); | ||
| break; | ||
| default: | ||
| console.log('Default mouseEvent registered'); | ||
| return; | ||
| } | ||
| } | ||
| }, [socket, getEventCoordinates]); | ||
|
|
||
| const handleKeyboardEvent = useCallback((event: KeyboardEvent) => { | ||
| if (!socket) return; | ||
| }, [socket]); |
There was a problem hiding this comment.
Address potential stale closures in event handlers.
The onMouseEvent callback depends on socket, but it does not include getText, getList, or lastMousePosition in its dependency array. This may lead to stale values being used inside the callback. Additionally, consider including all dependencies to ensure the callback has the latest values.
Apply this diff to include all necessary dependencies:
const onMouseEvent = useCallback((event: MouseEvent) => {
if (socket && canvasRef.current) {
// Existing logic...
}
-}, [socket]);
+}, [socket, getTextRef, getListRef, lastMousePosition]);Alternatively, if certain values are refs or constants that do not change, ensure this is documented to avoid confusion.
📝 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.
| if (socket && canvasRef.current) { | |
| // Get the canvas bounding rectangle | |
| const rect = canvasRef.current.getBoundingClientRect(); | |
| const clickCoordinates = { | |
| x: event.clientX - rect.left, // Use relative x coordinate | |
| y: event.clientY - rect.top, // Use relative y coordinate | |
| }; | |
| switch (event.type) { | |
| case 'mousedown': | |
| debouncer.current.add(() => { | |
| if (refs.current.getText) { | |
| switch (event.type) { | |
| case 'mousedown': | |
| if (getTextRef.current === true) { | |
| console.log('Capturing Text...'); | |
| } else if (refs.current.getList) { | |
| } else if (getListRef.current === true) { | |
| console.log('Capturing List...'); | |
| } else { | |
| socket.emit('input:mousedown', coordinates); | |
| socket.emit('input:mousedown', clickCoordinates); | |
| } | |
| setLastAction('click'); | |
| }, true); // High priority | |
| break; | |
| case 'mousemove': | |
| if (refs.current.lastMousePosition.x !== coordinates.x || | |
| refs.current.lastMousePosition.y !== coordinates.y) { | |
| debouncer.current.add(() => { | |
| refs.current.lastMousePosition = coordinates; | |
| socket.emit('input:mousemove', coordinates); | |
| setLastAction('move'); | |
| }); | |
| } | |
| break; | |
| case 'wheel': | |
| const wheelEvent = event as WheelEvent; | |
| debouncer.current.add(() => { | |
| socket.emit('input:wheel', { | |
| notifyLastAction('click'); | |
| break; | |
| case 'mousemove': | |
| if (lastMousePosition.current.x !== clickCoordinates.x || | |
| lastMousePosition.current.y !== clickCoordinates.y) { | |
| lastMousePosition.current = { | |
| x: clickCoordinates.x, | |
| y: clickCoordinates.y, | |
| }; | |
| socket.emit('input:mousemove', { | |
| x: clickCoordinates.x, | |
| y: clickCoordinates.y, | |
| }); | |
| notifyLastAction('move'); | |
| } | |
| break; | |
| case 'wheel': | |
| const wheelEvent = event as WheelEvent; | |
| const deltas = { | |
| deltaX: Math.round(wheelEvent.deltaX), | |
| deltaY: Math.round(wheelEvent.deltaY) | |
| }); | |
| setLastAction('scroll'); | |
| }); | |
| break; | |
| deltaY: Math.round(wheelEvent.deltaY), | |
| }; | |
| socket.emit('input:wheel', deltas); | |
| notifyLastAction('scroll'); | |
| break; | |
| default: | |
| console.log('Default mouseEvent registered'); | |
| return; | |
| } | |
| } | |
| }, [socket, getEventCoordinates]); | |
| const handleKeyboardEvent = useCallback((event: KeyboardEvent) => { | |
| if (!socket) return; | |
| }, [socket]); | |
| if (socket && canvasRef.current) { | |
| // Get the canvas bounding rectangle | |
| const rect = canvasRef.current.getBoundingClientRect(); | |
| const clickCoordinates = { | |
| x: event.clientX - rect.left, // Use relative x coordinate | |
| y: event.clientY - rect.top, // Use relative y coordinate | |
| }; | |
| switch (event.type) { | |
| case 'mousedown': | |
| if (getTextRef.current === true) { | |
| console.log('Capturing Text...'); | |
| } else if (getListRef.current === true) { | |
| console.log('Capturing List...'); | |
| } else { | |
| socket.emit('input:mousedown', clickCoordinates); | |
| } | |
| notifyLastAction('click'); | |
| break; | |
| case 'mousemove': | |
| if (lastMousePosition.current.x !== clickCoordinates.x || | |
| lastMousePosition.current.y !== clickCoordinates.y) { | |
| lastMousePosition.current = { | |
| x: clickCoordinates.x, | |
| y: clickCoordinates.y, | |
| }; | |
| socket.emit('input:mousemove', { | |
| x: clickCoordinates.x, | |
| y: clickCoordinates.y, | |
| }); | |
| notifyLastAction('move'); | |
| } | |
| break; | |
| case 'wheel': | |
| const wheelEvent = event as WheelEvent; | |
| const deltas = { | |
| deltaX: Math.round(wheelEvent.deltaX), | |
| deltaY: Math.round(wheelEvent.deltaY), | |
| }; | |
| socket.emit('input:wheel', deltas); | |
| notifyLastAction('scroll'); | |
| break; | |
| default: | |
| console.log('Default mouseEvent registered'); | |
| return; | |
| } | |
| } | |
| }, [socket, getTextRef, getListRef, lastMousePosition]); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 148-148: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 149-152: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
20241106-1902-08.3805076.mp4
Hey, I have added the dark theme.
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the updated release notes:
Dark Mode
UI/UX Improvements
Performance and Maintenance
js-cookiewith custom cookie handling functions.New Features
These changes primarily focus on enhancing the application's visual design and user experience through a comprehensive dark mode implementation.