Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new WeekCalendar component that displays the current week with Day.js integration and Material-UI styling. The component supports Hebrew locale, enables day selection, and includes theme-based styling. Dependencies for Material-UI date pickers and Day.js were added, and the component is integrated into the App. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/UI/Calendar/SGLCalendar.tsx (1)
8-8: Avoid locale global mutation inside a UI component module.
dayjs.locale('he')at import time changes global Day.js behavior as a side effect. Prefer setting locale once in app bootstrap/config (e.g.,src/main.tsx) to keep component modules pure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UI/Calendar/SGLCalendar.tsx` at line 8, The module-level call dayjs.locale('he') in SGLCalendar.tsx mutates global Day.js state; remove this side-effect from the component file and instead set the locale once during application bootstrap (e.g., in your app entry like main.tsx) or provide a locale prop to the SGLCalendar component so the component stays pure; update references to dayjs in SGLCalendar (and any tests) to assume locale is configured externally and remove the dayjs.locale('he') invocation from SGLCalendar.tsx.src/components/UI/Calendar/styles.ts (1)
9-14: Use theme background token instead of hardcoded white.Hardcoding
backgroundColor: 'white'makes this container insensitive to theme mode/palette changes. Prefertheme.palette.background.paper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UI/Calendar/styles.ts` around lines 9 - 14, The paperStyles object currently hardcodes backgroundColor: 'white', so change paperStyles into a theme-aware style (e.g., a function or Sx factory) that uses theme.palette.background.paper instead; update the export of paperStyles to accept a Theme (or receive theme in the calling component) and return the same keys (p, borderRadius, maxWidth) but set backgroundColor: theme.palette.background.paper so the component respects light/dark theme changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 21: Remove the unused dependency entry "@mui/x-date-pickers" from
package.json; open package.json, locate the dependency key
"@mui/x-date-pickers": "^8.27.2" and delete that line, then run the project's
dependency manager (e.g., npm install or yarn install) to update lockfile and
node_modules and verify the build/tests to ensure nothing else references
"@mui/x-date-pickers".
In `@src/components/UI/Calendar/SGLCalendar.tsx`:
- Around line 38-46: The day cell in SGLCalendar.tsx uses a non-semantic Box
with only onClick which prevents keyboard users from selecting dates; replace
the Box for each day (keyed by day.format('YYYY-MM-DD')) with a semantic
interactive element (preferably a <button>) or ensure it has role="button",
tabIndex={0}, and keyboard handlers that call setSelectedDate(day) on
Enter/Space, apply the same styling from
getDayStyles({isSelected,isToday,theme}), and add appropriate ARIA like
aria-pressed or aria-current to reflect selection so keyboard and assistive tech
can operate the calendar.
---
Nitpick comments:
In `@src/components/UI/Calendar/SGLCalendar.tsx`:
- Line 8: The module-level call dayjs.locale('he') in SGLCalendar.tsx mutates
global Day.js state; remove this side-effect from the component file and instead
set the locale once during application bootstrap (e.g., in your app entry like
main.tsx) or provide a locale prop to the SGLCalendar component so the component
stays pure; update references to dayjs in SGLCalendar (and any tests) to assume
locale is configured externally and remove the dayjs.locale('he') invocation
from SGLCalendar.tsx.
In `@src/components/UI/Calendar/styles.ts`:
- Around line 9-14: The paperStyles object currently hardcodes backgroundColor:
'white', so change paperStyles into a theme-aware style (e.g., a function or Sx
factory) that uses theme.palette.background.paper instead; update the export of
paperStyles to accept a Theme (or receive theme in the calling component) and
return the same keys (p, borderRadius, maxWidth) but set backgroundColor:
theme.palette.background.paper so the component respects light/dark theme
changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 838e6548-c591-46ea-8952-df97528d441d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.jsonsrc/App.tsxsrc/components/UI/Calendar/SGLCalendar.tsxsrc/components/UI/Calendar/styles.ts
| "@emotion/styled": "^11.14.1", | ||
| "@mui/icons-material": "7.3.9", | ||
| "@mui/material": "^7.3.9", | ||
| "@mui/x-date-pickers": "^8.27.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `@mui/x-date-pickers` is actually referenced in the repo
rg -n --type=ts --type=tsx -C2 "@mui/x-date-pickers|LocalizationProvider|DatePicker|StaticDatePicker|AdapterDayjs"Repository: ColmanDevClubORG/Sagol360Management
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Fix: search for `@mui/x-date-pickers` references without the tsx file type
rg -n "@mui/x-date-pickers|LocalizationProvider|DatePicker|StaticDatePicker|AdapterDayjs" --type=tsRepository: ColmanDevClubORG/Sagol360Management
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Also check if there are any .jsx or .js files that might use this package
rg -n "@mui/x-date-pickers|LocalizationProvider|DatePicker|StaticDatePicker|AdapterDayjs" --type=js --type=jsxRepository: ColmanDevClubORG/Sagol360Management
Length of output: 106
🏁 Script executed:
#!/bin/bash
# And search broadly across all files without type restrictions
rg -n "@mui/x-date-pickers" -C2Repository: ColmanDevClubORG/Sagol360Management
Length of output: 2849
Remove @mui/x-date-pickers from dependencies—it is not referenced anywhere in the codebase.
This unused package adds unnecessary bundle size and expands the dependency surface. The calendar implementation uses dayjs and MUI core primitives without any picker APIs from this package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 21, Remove the unused dependency entry
"@mui/x-date-pickers" from package.json; open package.json, locate the
dependency key "@mui/x-date-pickers": "^8.27.2" and delete that line, then run
the project's dependency manager (e.g., npm install or yarn install) to update
lockfile and node_modules and verify the build/tests to ensure nothing else
references "@mui/x-date-pickers".
| <Box | ||
| key={day.format('YYYY-MM-DD')} | ||
| onClick={() => setSelectedDate(day)} | ||
| sx={getDayStyles({ | ||
| isSelected, | ||
| isToday, | ||
| theme, | ||
| })} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/UI/Calendar/SGLCalendar.tsxRepository: ColmanDevClubORG/Sagol360Management
Length of output: 2492
Make day cells keyboard-accessible and semantic.
The selectable day element is clickable but not keyboard-focusable/operable as implemented. This blocks keyboard-only users from changing dates.
The day cell uses a generic <Box> component with only an onClick handler and no keyboard event listeners, ARIA attributes, or semantic button markup. Convert it to a proper button element with keyboard support and accessibility attributes.
♿ Suggested fix
- <Box
+ <Box
+ component="button"
+ type="button"
key={day.format('YYYY-MM-DD')}
onClick={() => setSelectedDate(day)}
- sx={getDayStyles({
- isSelected,
- isToday,
- theme,
- })}
+ aria-pressed={isSelected}
+ aria-label={day.format('dddd D')}
+ sx={{
+ ...getDayStyles({
+ isSelected,
+ isToday,
+ theme,
+ }),
+ border: 0,
+ width: '100%',
+ }}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/UI/Calendar/SGLCalendar.tsx` around lines 38 - 46, The day
cell in SGLCalendar.tsx uses a non-semantic Box with only onClick which prevents
keyboard users from selecting dates; replace the Box for each day (keyed by
day.format('YYYY-MM-DD')) with a semantic interactive element (preferably a
<button>) or ensure it has role="button", tabIndex={0}, and keyboard handlers
that call setSelectedDate(day) on Enter/Space, apply the same styling from
getDayStyles({isSelected,isToday,theme}), and add appropriate ARIA like
aria-pressed or aria-current to reflect selection so keyboard and assistive tech
can operate the calendar.
Tamir198
left a comment
Tamir198
left a comment
There was a problem hiding this comment.
Lets name this component `SGLCalendar
Also can you make this component responsive to all mobile sized? under 400 px thing
You can do max width to make sure that its not taking too much space
| @@ -0,0 +1,25 @@ | |||
| import type { Theme } from '@mui/material/styles' | |||
|
|
|||
| type DayStylesProps = { | |||
|
|
||
| dayjs.locale('he') | ||
|
|
||
| const getStartOfWeek = (date: Dayjs) => { |
There was a problem hiding this comment.
This function could be extracted to utils file, we dont have one yet to create one and inside utils/datesUtils.ts file
There was a problem hiding this comment.
Also try to give this more generic type and not specific to library
| dayjs.locale('he') | ||
|
|
||
| const getStartOfWeek = (date: Dayjs) => { | ||
| const day = date.day() |
There was a problem hiding this comment.
Same here - getCurrectDay function from utils
| return date.subtract(day, 'day') | ||
| } | ||
|
|
||
| export default function WeekCalendar() { |
There was a problem hiding this comment.
export const WeekCalander = ... instead
| } | ||
|
|
||
| return ( | ||
| <Paper elevation={2} sx={paperStyles}> |
There was a problem hiding this comment.
Can you use the card component instead?
|
|
||
| return ( | ||
| <Paper elevation={2} sx={paperStyles}> | ||
| <Box display="grid" gridTemplateColumns="repeat(7, 1fr)" gap={1} textAlign="center"> |
There was a problem hiding this comment.
NO inline styles, styles should come from styles file
| const currentWeekStart = getStartOfWeek(today) | ||
|
|
||
| const weekDays = useMemo( | ||
| () => Array.from({ length: 7 }, (_, index) => currentWeekStart.add(index, 'day')), |
| p: 1.2, | ||
| transition: '0.2s', | ||
| backgroundColor: isSelected | ||
| ? theme.palette.purple.main |
There was a problem hiding this comment.
We have a bit cleaner way of doing things, for example:
export const getCardStyles = (theme: Theme, variant: CardVariant = 'purple') => {
const color = theme.palette[variant].main
return {
borderRadius: '10px',
background: `linear-gradient(135deg, ${alpha(color, 0.5)} 0%, ${color} 60%)`,
}
}This way the color is the color that we are passing to the variant
2 participants
Description
Added a calender component using mui.
Related Issue(s)
Fixes # 86ex0ccn4
Checklist:
Screenshots (if appropriate):
Summary by CodeRabbit