fixed:#1887 Event and Holiday Calendar View #2534
Conversation
WalkthroughThe pull request introduces modifications to several files, primarily focusing on the event calendar feature. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
👮 Files not reviewed due to content moderation or server errors (2)
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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
src/components/EventCalendar/EventCalendar.module.css (4)
424-464: Consider using CSS variables for colors and spacingThe implementation looks good, but consider extracting the repeated values into CSS variables for better maintainability:
- Color values (rgba(0, 0, 0, 0.15), rgba(82, 172, 255, 0.5))
- Spacing values (8px)
:root { + --holiday-indicator-color: rgba(0, 0, 0, 0.15); + --organization-indicator-color: rgba(82, 172, 255, 0.5); + --indicator-spacing: 8px; } .list_container_new { - gap: 8px; + gap: var(--indicator-spacing); } .holidayIndicator { - background-color: rgba(0, 0, 0, 0.15); + background-color: var(--holiday-indicator-color); } .organizationIndicator { - background-color: rgba(82, 172, 255, 0.5); + background-color: var(--organization-indicator-color); }
482-490: Maintain consistency in color formats and spacing valuesConsider:
- Using rgba format for background-color to match other color definitions
- Extracting padding and gap values to CSS variables for consistency with other spacing values
:root { + --container-padding: 20px; + --container-gap: 20px; + --container-bg: rgba(249, 249, 249, 1); } .container { - padding: 20px; - gap: 20px; - background-color: #f9f9f9; + padding: var(--container-padding); + gap: var(--container-gap); + background-color: var(--container-bg); }
526-558: Maintain consistency in color value formatThe implementation looks good, but there's an inconsistency in color formats:
- Most colors use rgba format
- User indicator uses hex (#8bc34a)
Consider converting all colors to rgba format for consistency.
.user_indicator { - background-color: #8bc34a; + background-color: rgba(139, 195, 74, 1); }
424-558: Well-structured implementation of calendar view stylesThe CSS implementation effectively supports the calendar view requirements with:
- Clear visual differentiation between holidays and events
- Responsive design considerations
- Consistent layout patterns
- Good use of CSS Modules features
The structure aligns well with the Figma design mentioned in the PR objectives.
Consider creating a CSS variables file to centralize all color and spacing values, making it easier to maintain consistent theming across the application.
src/components/EventCalendar/EventCalendar.tsx (2)
478-482: Refactor Nested Ternary Operator for Improved ReadabilityThe nested ternary operator in this code segment reduces readability and may make future maintenance difficult. Consider refactoring it using conditional statements or extracting the logic into a helper function to enhance clarity.
Here's a possible refactor using an immediately invoked function expression (IIFE):
{(() => { if (expanded === index) { return allEventsList; } else if (holidayList?.length > 0) { return allEventsList?.slice(0, 1); } else { return allEventsList?.slice(0, 2); } })()}
Line range hint
564-569: Eliminate Redundant Rendering to Optimize PerformanceThe components
<YearlyEventCalender>and<div className={styles.calendar__hours}>{renderHours()}</div>are being rendered twice—once in the conditional rendering starting at line 548 and again in this block. This redundancy can lead to unnecessary rendering cycles and affect performance. Please refactor the code to remove the duplicate rendering.You can remove the redundant block:
- <div> - {viewType == ViewType.YEAR ? ( - <YearlyEventCalender eventData={eventData} /> - ) : ( - <div className={styles.calendar__hours}>{renderHours()}</div> - )} - </div>Since the component rendering is already handled within the conditional rendering starting at line 548, this additional block is unnecessary.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.husky/pre-commit(0 hunks)package.json(2 hunks)src/components/EventCalendar/EventCalendar.module.css(1 hunks)src/components/EventCalendar/EventCalendar.tsx(6 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- .husky/pre-commit
🔇 Additional comments (4)
src/components/EventCalendar/EventCalendar.module.css (2)
465-481: LGTM! Responsive design implementation looks good
The media query appropriately adjusts sizes and spacing for mobile screens while maintaining visual hierarchy and proportions.
491-525: LGTM! Holiday card styling aligns with design requirements
The implementation successfully:
- Uses warm colors to distinguish holiday entries
- Maintains good contrast for readability
- Provides clear visual hierarchy
tsconfig.json (1)
3-3: Addition of Vitest Types Confirmed
Including "vitest" in the types array ensures that type definitions for Vitest are available, enhancing the development and testing experience.
package.json (1)
153-153: Confirm Downgrade of jest-preview Dependency
The jest-preview package has been downgraded from ^0.3.1 to ^0.2.3. Please verify if this change is intentional. Downgrading may reintroduce fixed bugs or remove features present in newer versions.
Ensure that this downgrade does not adversely affect the testing environment or introduce compatibility issues.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (7)
src/components/EventCalendar/EventCalendar.tsx (2)
321-341: Consider memoizing the filtered holidays listThe holiday filtering logic is correct but could benefit from memoization since it's recalculated on every render.
Consider using
useMemo:+ const filteredHolidays = useMemo( + () => holidays?.filter( + (holiday) => dayjs(holiday.date, 'MM-DD').month() === currentMonth + ), + [holidays, currentMonth] + ); <div className={styles.holidays_card}> <h3 className={styles.card_title}>Holidays</h3> <ul className={styles.holiday_list}> - {holidays - ?.filter( - (holiday) => - dayjs(holiday.date, 'MM-DD').month() === currentMonth, - ) + {filteredHolidays .map((holiday, index) => (
Line range hint
568-576: Remove duplicate calendar hours renderingThe
renderHours()component is being rendered twice, which could cause performance issues.Remove the duplicate rendering:
- <div> - {viewType == ViewType.YEAR ? ( - <YearlyEventCalender eventData={eventData} /> - ) : ( - <div className={styles.calendar__hours}>{renderHours()}</div> - )} - </div>src/components/EventCalendar/EventCalendar.module.css (5)
424-429: Consider consolidating container classesThe
.list_container_newclass appears to be a new version of the existing.list_container. Consider consolidating these classes to maintain consistency and reduce duplication.-.list_container_new { - width: fit-content; - display: flex; - align-items: center; - gap: var(--indicator-spacing); -} .list_container { padding: 5px; width: fit-content; display: flex; - flex-direction: row; + align-items: center; + gap: var(--indicator-spacing); }Also applies to: 492-500
439-446: Maintain consistent naming convention for text classesThe
.user_legend_textclass should follow the same pattern as.holidayTextand.legendTextby composing from.commonText..user_legend_text { - font-size: 14px; - color: #555555; + composes: commonText; }Also applies to: 462-464, 565-568
465-481: Define breakpoints using CSS variablesConsider defining the breakpoint value as a CSS variable for better maintainability.
:root { + --mobile-breakpoint: 700px; } -@media only screen and (max-width: 700px) { +@media only screen and (max-width: var(--mobile-breakpoint)) { /* ... */ }
483-490: Move hardcoded colors to CSS variablesConsider moving all color values to CSS variables for better maintainability and theming support.
:root { /* ... existing variables ... */ + --user-indicator-color: rgba(139, 195, 74, 1); + --holiday-card-bg: #fff8e1; + --holiday-date-color: #ff9800; + --text-color-primary: #333333; + --text-color-secondary: #555555; }
502-543: Standardize box shadow valuesConsider defining box shadow values as CSS variables for consistency across components.
:root { + --card-shadow: 0px 2px 4px rgba(0, 0, 0, 0.1); } .holidays_card { - box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.1); + box-shadow: var(--card-shadow); } .events_card { - box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.1); + box-shadow: var(--card-shadow); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/EventCalendar/EventCalendar.module.css(1 hunks)src/components/EventCalendar/EventCalendar.tsx(6 hunks)
🔇 Additional comments (3)
src/components/EventCalendar/EventCalendar.tsx (2)
342-363: LGTM! Well-structured legend implementation
The color-coded legend implementation effectively differentiates between holidays and different types of events, improving visual clarity as per the PR objectives.
Line range hint 552-561: LGTM! Clean calendar layout implementation
The calendar grid layout is well-structured and semantically correct.
src/components/EventCalendar/EventCalendar.module.css (1)
545-555: LGTM!
The legend styles are well-structured and consistent with the design requirements.
| {(() => { | ||
| if (expanded === index) { | ||
| return allEventsList; | ||
| } else if (holidayList?.length > 0) { | ||
| return allEventsList?.slice(0, 1); | ||
| } else { | ||
| return allEventsList?.slice(0, 2); | ||
| } | ||
| })()} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the event display logic
The current implementation using an IIFE makes the code harder to read and maintain.
Consider simplifying the logic:
- {(() => {
- if (expanded === index) {
- return allEventsList;
- } else if (holidayList?.length > 0) {
- return allEventsList?.slice(0, 1);
- } else {
- return allEventsList?.slice(0, 2);
- }
- })()}
+ {expanded === index
+ ? allEventsList
+ : allEventsList?.slice(0, holidayList?.length > 0 ? 1 : 2)
+ }📝 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 (expanded === index) { | |
| return allEventsList; | |
| } else if (holidayList?.length > 0) { | |
| return allEventsList?.slice(0, 1); | |
| } else { | |
| return allEventsList?.slice(0, 2); | |
| } | |
| })()} | |
| {expanded === index | |
| ? allEventsList | |
| : allEventsList?.slice(0, holidayList?.length > 0 ? 1 : 2) | |
| } |
| .holidayIndicator { | ||
| background-color: var(--holiday-indicator-color); | ||
| border-radius: 5px; | ||
| width: 20px; | ||
| height: 12px; | ||
| display: inline-block; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply DRY principle to indicator styles
The indicator classes (.holidayIndicator, .organizationIndicator, .user_indicator) share identical properties. Consider creating a base indicator class to reduce code duplication.
+.baseIndicator {
+ border-radius: 5px;
+ width: 20px;
+ height: 12px;
+ display: inline-block;
+}
.holidayIndicator {
+ composes: baseIndicator;
- border-radius: 5px;
- width: 20px;
- height: 12px;
- display: inline-block;
background-color: var(--holiday-indicator-color);
}
.organizationIndicator {
+ composes: baseIndicator;
- border-radius: 5px;
- width: 20px;
- height: 12px;
- display: inline-block;
background-color: var(--organization-indicator-color);
}
.user_indicator {
+ composes: baseIndicator;
- border-radius: 5px;
- width: 20px;
- height: 12px;
- display: inline-block;
background-color: rgba(139, 195, 74, 1);
}Also applies to: 454-460, 557-563
changed acc to the bot 2
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (9)
src/components/EventCalendar/EventCalendar.tsx (3)
327-358: Enhance accessibility of the calendar cardsWhile the implementation aligns well with the design requirements, consider these accessibility improvements:
- Add ARIA labels to the indicator spans
- Use semantic list elements for the legend items
<div className={styles.legend}> <div className={styles.list_container_new}> - <span className={styles.holidayIndicator}></span> + <span className={styles.holidayIndicator} role="img" aria-label="Holiday indicator"></span> <span className={styles.holidayText}>Holidays</span> </div> <div className={styles.eventsLegend}> - <span className={styles.organizationIndicator}></span> + <span className={styles.organizationIndicator} role="img" aria-label="Organization event indicator"></span> <span className={styles.legendText}>Events Created by Organization</span> </div> <div className={styles.user_events}> - <span className={styles.user_indicator}></span> + <span className={styles.user_indicator} role="img" aria-label="User event indicator"></span> <span className={styles.user_legend_text}>Events Created by User</span> </div> </div>
473-477: Simplify the event list rendering logic furtherWhile the implementation works, we can make it more readable by extracting the logic into a helper function.
+ const getVisibleEvents = (expanded: boolean, holidayList: JSX.Element[], allEventsList: JSX.Element[]) => { + if (expanded) return allEventsList; + const maxEvents = holidayList?.length > 0 ? 1 : 2; + return allEventsList?.slice(0, maxEvents); + }; - {expanded === index - ? allEventsList - : holidayList?.length > 0 - ? allEventsList?.slice(0, 1) - : allEventsList?.slice(0, 2)} + {getVisibleEvents(expanded === index, holidayList, allEventsList)}Also applies to: 488-488
Line range hint
559-567: Remove duplicate calendar hours renderingThere appears to be duplicate rendering of the calendar hours view. The same logic is already handled in the previous conditional block.
- <div> - {viewType == ViewType.YEAR ? ( - <YearlyEventCalender eventData={eventData} /> - ) : ( - <div className={styles.calendar__hours}>{renderHours()}</div> - )} - </div>src/components/EventCalendar/EventCalendar.module.css (6)
559-561: Use CSS variable for user indicator colorThe
.user_indicatorclass hardcodes the color valuergba(139, 195, 74, 1). To maintain consistency and improve maintainability, consider using the predefined CSS variable--user-indicator-color.Apply this diff to update the code:
.user_indicator { composes: baseIndicator; - background-color: rgba(139, 195, 74, 1); + background-color: var(--user-indicator-color); }
506-506: Use CSS variable for holiday card background colorThe
.holidays_cardclass uses a hardcoded background color#fff8e1. Using the predefined CSS variable--holiday-card-bgenhances consistency and makes future style updates easier.Apply this diff to update the code:
.holidays_card { flex: 1; padding: 20px; - background-color: #fff8e1; + background-color: var(--holiday-card-bg); border-radius: 10px; box-shadow: var(--card-shadow); }
535-535: Use CSS variable for holiday date text colorThe
.holiday_dateclass hardcodes the color#ff9800. Consider using the predefined CSS variable--holiday-date-colorfor consistency across the codebase.Apply this diff to update the code:
.holiday_date { font-weight: 500; - color: #ff9800; + color: var(--holiday-date-color); }
514-514: Use CSS variable for card title text colorThe
.card_titleclass specifies a hardcoded color#333333. Utilizing the CSS variable--text-color-primarywill enhance consistency and ease future maintenance.Apply this diff to update the code:
.card_title { font-size: 16px; font-weight: 600; - color: #333333; + color: var(--text-color-primary); margin-bottom: 15px; }
439-439: Use CSS variable for common text colorThe
.commonTextclass hardcodes the color#555555. To maintain consistency, consider using the CSS variable--text-color-secondary.Apply this diff to update the code:
.commonText { font-size: 14px; - color: #555555; + color: var(--text-color-secondary); }
460-476: Ensure consistent use of CSS variables in media queriesIn the media query starting at line 460, you've correctly used the CSS variable
--mobile-breakpoint. However, elsewhere in the code, there is a media query with a hardcoded500px. For consistency and easier maintenance, consider defining a CSS variable for this breakpoint as well.Add a new variable in the
:rootselector::root { --container-padding: 20px; --container-gap: 20px; --container-bg: rgba(249, 249, 249, 1); --holiday-indicator-color: rgba(0, 0, 0, 0.15); --organization-indicator-color: rgba(82, 172, 255, 0.5); --indicator-spacing: 8px; --mobile-breakpoint: 700px; + --small-mobile-breakpoint: 500px; --user-indicator-color: rgba(139, 195, 74, 1); --holiday-card-bg: #fff8e1; --holiday-date-color: #ff9800; --text-color-primary: #333333; --text-color-secondary: #555555; --card-shadow: 0px 2px 4px rgba(0, 0, 0, 0.1); }Then update the media query at line 321:
@media only screen and (max-width: 500px) { +@media only screen and (max-width: var(--small-mobile-breakpoint)) {This ensures all breakpoints are centralized, making future adjustments simpler.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/EventCalendar/EventCalendar.module.css(3 hunks)src/components/EventCalendar/EventCalendar.tsx(7 hunks)
🔇 Additional comments (2)
src/components/EventCalendar/EventCalendar.tsx (2)
224-230: LGTM! Efficient holiday filtering implementation
The useMemo hook is appropriately used to optimize performance by memoizing the filtered holidays list. The filtering logic correctly matches holidays with the current month.
390-393: LGTM! Clear and correct styling logic
The conditional class assignments for today's date and current month are well-implemented and maintain clear visual distinction between different day types.
There was a problem hiding this comment.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to the files indicated.
.husky/pre-commit
Outdated
| @@ -1,5 +1,3 @@ | |||
| #!/usr/bin/env sh | |||
| . "$(dirname -- "$0")/_/husky.sh" | |||
|
|
|||
There was a problem hiding this comment.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
package.json
Outdated
| @@ -150,13 +150,13 @@ | |||
| "jest": "^27.4.5", | |||
| "jest-localstorage-mock": "^2.4.19", | |||
| "jest-location-mock": "^2.0.0", | |||
| "jest-preview": "^0.3.1", | |||
| "jest-preview": "^0.2.3", | |||
There was a problem hiding this comment.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
tsconfig.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "types": ["vite/client", "vite-plugin-svgr/client", "node"], | |||
| "types": ["vite/client", "vite-plugin-svgr/client", "node", "vitest"], | |||
There was a problem hiding this comment.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
|
Make sure CodeRabbit.ai approves your work. |
gurramkarthiknetha
force-pushed
the
issues/fix-issue-1887
branch
from
ba21ae5 to
b717e05
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (8)
src/components/EventCalendar/EventCalendar.module.css (3)
Line range hint
320-335: Consider adding comments to document breakpoint purposesWhile the media queries are well-structured using CSS variables, adding comments to explain the purpose of each breakpoint would improve maintainability.
Add documentation like this:
+/* Tablet and smaller devices */ @media only screen and (max-width: var(--mobile-breakpoint)) { ... } +/* Small mobile devices */ @media only screen and (max-width: var(--small-mobile-breakpoint)) { ... }
478-493: Standardize color format usageWhile the CSS variables provide excellent theming support, consider standardizing the color format. Currently, there's a mix of rgba and hex colors.
Convert all colors to the same format for consistency:
:root { --holiday-indicator-color: rgba(0, 0, 0, 0.15); - --holiday-date-color: #ff9800; + --holiday-date-color: rgba(255, 152, 0, 1); - --text-color-primary: #333333; + --text-color-primary: rgba(51, 51, 51, 1); - --text-color-secondary: #555555; + --text-color-secondary: rgba(85, 85, 85, 1); }
494-566: Consider adding flex-basis for better layout controlThe card layout system is well-implemented, but could benefit from more precise flex control for consistent card sizes.
Add flex-basis to the cards:
.holidays_card { flex: 1; + flex-basis: 0; padding: 20px; background-color: var(--holiday-card-bg); border-radius: 10px; box-shadow: var(--card-shadow); } .events_card { flex: 1; + flex-basis: 0; padding: 20px; background-color: #ffffff; border-radius: 10px; box-shadow: var(--card-shadow); }This ensures both cards grow from the same base size when using
flex: 1.src/components/EventCalendar/EventCalendar.tsx (5)
224-230: Optimize holiday filtering performanceThe
useMemoimplementation could be more efficient. Since theholidaysarray is likely static, it shouldn't be in the dependency array.Consider this optimization:
const filteredHolidays = useMemo( () => holidays?.filter( (holiday) => dayjs(holiday.date, 'MM-DD').month() === currentMonth, ), - [holidays, currentMonth], + [currentMonth], // holidays array is static );
327-370: Consider extracting legend component for better modularityThe legend implementation with color indicators is well-structured and accessible. However, to improve maintainability, consider extracting it into a separate component.
Example structure:
interface LegendProps { items: Array<{ indicatorClass: string; label: string; ariaLabel: string; }>; } const Legend: React.FC<LegendProps> = ({ items }) => ( <div className={styles.legend}> {items.map(({ indicatorClass, label, ariaLabel }) => ( <div key={label} className={styles.list_container_new}> <span className={indicatorClass} role="img" aria-label={ariaLabel} ></span> <span className={styles.legendText}>{label}</span> </div> ))} </div> );
402-405: Improve readability of className conditionsThe className concatenation logic could be more readable.
Consider using a more declarative approach:
- date.toLocaleDateString() === today.toLocaleDateString() - ? styles.day__today - : '', + isToday(date) ? styles.day__today : '',Add this helper function at the top of the component:
const isToday = (date: Date): boolean => date.toLocaleDateString() === today.toLocaleDateString();
495-499: Simplify event rendering callThe function call could be more concise.
- {getVisibleEvents( - expanded === index, - holidayList, - allEventsList, - )} + {getVisibleEvents(expanded === index, holidayList, allEventsList)}
Line range hint
565-581: Fix duplicate rendering of calendar viewsThere's a critical bug where both the yearly calendar and hours view are being rendered twice. The second rendering block is redundant and should be removed.
Remove the duplicate rendering:
<div className={styles.calendar__scroll} customScroll> {viewType == ViewType.MONTH ? ( <> <div className={styles.calendar__weekdays}> {weekdays.map((weekday, index) => ( <div key={index} className={styles.weekday}> {weekday} </div> ))} </div> <div className={styles.calendar__days}>{renderDays()}</div> </> ) : viewType == ViewType.YEAR ? ( <YearlyEventCalender eventData={eventData} /> ) : ( <div className={styles.calendar__hours}>{renderHours()}</div> )} </div> - <div> - {viewType == ViewType.YEAR ? ( - <YearlyEventCalender eventData={eventData} /> - ) : ( - <div className={styles.calendar__hours}>{renderHours()}</div> - )} - </div>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.husky/pre-commit(0 hunks)package.json(2 hunks)src/components/EventCalendar/EventCalendar.module.css(4 hunks)src/components/EventCalendar/EventCalendar.tsx(8 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- .husky/pre-commit
🚧 Files skipped from review as they are similar to previous changes (2)
- tsconfig.json
- package.json
🔇 Additional comments (2)
src/components/EventCalendar/EventCalendar.module.css (2)
279-280: LGTM! Good use of CSS variables for consistent spacing
The alignment and gap properties using CSS variables improve maintainability and consistency.
426-476: Well implemented indicator system following previous suggestions!
The implementation effectively uses CSS composition with the baseIndicator class, addressing the previous review comment about DRY principle application.
| const getVisibleEvents = ( | ||
| expanded: boolean, | ||
| holidayList: JSX.Element[], | ||
| allEventsList: JSX.Element[], | ||
| // eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
| ) => { | ||
| if (expanded) return allEventsList; | ||
| const maxEvents = holidayList?.length > 0 ? 1 : 2; | ||
| return allEventsList?.slice(0, maxEvents); | ||
| }; |
There was a problem hiding this comment.
Add proper TypeScript types and remove eslint-disable
The getVisibleEvents function needs proper TypeScript types and the eslint-disable comment should be removed.
Apply these changes:
const getVisibleEvents = (
expanded: boolean,
holidayList: JSX.Element[],
allEventsList: JSX.Element[],
- // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
- ) => {
+ ): JSX.Element[] | undefined => {
if (expanded) return allEventsList;
const maxEvents = holidayList?.length > 0 ? 1 : 2;
return allEventsList?.slice(0, maxEvents);
};📝 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 getVisibleEvents = ( | |
| expanded: boolean, | |
| holidayList: JSX.Element[], | |
| allEventsList: JSX.Element[], | |
| // eslint-disable-next-line @typescript-eslint/explicit-function-return-type | |
| ) => { | |
| if (expanded) return allEventsList; | |
| const maxEvents = holidayList?.length > 0 ? 1 : 2; | |
| return allEventsList?.slice(0, maxEvents); | |
| }; | |
| const getVisibleEvents = ( | |
| expanded: boolean, | |
| holidayList: JSX.Element[], | |
| allEventsList: JSX.Element[], | |
| ): JSX.Element[] | undefined => { | |
| if (expanded) return allEventsList; | |
| const maxEvents = holidayList?.length > 0 ? 1 : 2; | |
| return allEventsList?.slice(0, maxEvents); | |
| }; |
ok |
palisadoes
added
the
ignore-sensitive-files-pr
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2534 +/- ##
=====================================================
- Coverage 95.82% 84.08% -11.74%
=====================================================
Files 295 312 +17
Lines 7037 8101 +1064
Branches 1520 1823 +303
=====================================================
+ Hits 6743 6812 +69
- Misses 98 1150 +1052
+ Partials 196 139 -57 ☔ View full report in Codecov by Sentry. |
This feature displays an interactive calendar view for tracking events and holidays. The calendar highlights holidays and user/organization-created events for the selected month, with color-coded markers for easy identification
What kind of change does this PR introduce?
This PR introduces a new feature for displaying an interactive event calendar with a list of holidays and user/organization-created events.
Issue Number:
Fixes #1887
Did you add tests for your changes?
Yes, unit tests have been added to ensure the functionality works as expected.
Snapshots/Videos:
Snapshots and a short demonstration video showcasing the feature have been attached for better understanding.
If relevant, did you update the documentation?
Yes, the relevant sections in the Talawa-Docs have been updated to include details about the new feature.
Summary
This feature addresses the need for a more user-friendly and comprehensive event calendar.
Problem: The existing calendar lacked the ability to display a list of holidays and events in an organized manner.
Solution:
A list of holidays and events is now displayed below the calendar.
Color-coding has been implemented to distinguish between holidays and events for improved usability.
The UI aligns with the Figma design shared by the team.
Does this PR introduce a breaking change?
No, this PR does not introduce any breaking changes.
Other information
The Figma design used for this implementation can be found here.
This PR follows the project’s contribution guidelines and is submitted against the develop branch, as recommended.
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores