Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed Event and Holiday Calendar View #2529

Conversation

gurramkarthiknetha
Copy link
Contributor

@gurramkarthiknetha gurramkarthiknetha commented Nov 30, 2024

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.
image

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

  • New Features

    • Updated environment variables for enhanced functionality, including reCAPTCHA support.
    • Revamped the Event Calendar component to display holidays and events with improved visual layout and color-coded indicators.
  • Bug Fixes

    • Removed outdated hour-based event rendering logic for a more streamlined event display.
  • Chores

    • Modified pre-commit hook for improved development workflow.
    • Added a placeholder for future CSS styles in the index.html file.
    • Introduced new CSS classes for enhanced styling in the Event Calendar component.
    • Updated project dependencies for improved performance and functionality.

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
Copy link
Contributor

coderabbitai bot commented Nov 30, 2024

Walkthrough

This pull request includes modifications to several files. The .env.example file has updated environment variable assignments for API URLs and reCAPTCHA settings. The pre-commit hook in .husky/pre-commit has been simplified to execute specific npm commands directly, while the index.html file now contains an empty <style> tag for future use. Significant changes were made to the EventCalendar.tsx component, where the previous hour-based rendering logic was removed and replaced with a new layout for displaying holidays and events, including color-coded indicators.

Changes

File Change Summary
.env.example Updated REACT_APP_TALAWA_URL, REACT_APP_USE_RECAPTCHA, and REACT_APP_RECAPTCHA_SITE_KEY.
.husky/pre-commit Removed shebang and sourcing; added npm commands; commented out npm run lint:fix.
index.html Introduced an empty <style> tag in the <head> section.
src/components/EventCalendar/EventCalendar.tsx Removed CurrentHourIndicator; added layout for holidays and events with color-coded indicators.
src/components/EventCalendar/EventCalendar.module.css Added new CSS classes for styling holidays and events.
package.json Updated dependencies: added chart.js, modified @types/react-chartjs-2, and downgraded jest-preview.

Assessment against linked issues

Objective Addressed Explanation
Add a list of holidays and events to the calendar view (1887)
Use color coding for events in the calendar view (1887)

Possibly related PRs

Suggested labels

test, ignore-sensitive-files-pr

Suggested reviewers

  • varshith257
  • pranshugupta54
  • palisadoes

Poem

In the meadow where events bloom,
A calendar shines, dispelling gloom.
With colors bright, and holidays near,
We gather 'round, spreading cheer!
So let us hop, and dance with delight,
For every event is a joyful sight! 🐇✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Our Pull Request Approval Process

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

  1. We don't merge PRs with poor code quality.
    1. Follow coding best practices such that CodeRabbit.ai approves your PR.
  2. We don't merge PRs with failed tests.
    1. When tests fail, click on the Details link to learn more.
    2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
    3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
  3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (2)
src/components/EventCalendar/EventCalendar.tsx (2)

77-77: Remove console.log statement

Debug statements should not be committed to production code.

-  console.log(hours);

Line range hint 652-658: Remove duplicate YearlyEventCalender rendering

The YearlyEventCalender component is being rendered twice, which could cause performance issues and duplicate events.

      </div>
-      <div>
-        {viewType == ViewType.YEAR ? (
-          <YearlyEventCalender eventData={eventData} />
-        ) : (
-          <div className={styles.calendar__hours}>{renderHours()}</div>
-        )}
-      </div>
    </div>
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3d1f1f0 and 6103bdd.

📒 Files selected for processing (4)
  • .env.example (1 hunks)
  • .husky/pre-commit (0 hunks)
  • index.html (1 hunks)
  • src/components/EventCalendar/EventCalendar.tsx (3 hunks)
💤 Files with no reviewable changes (1)
  • .husky/pre-commit
✅ Files skipped from review due to trivial changes (1)
  • index.html
🧰 Additional context used
🪛 Gitleaks (8.21.2)
.env.example

24-24: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

.env.example Outdated
@@ -8,20 +8,20 @@ PORT=4321

# Run Talawa-api locally in your system, and put its url into the same.

REACT_APP_TALAWA_URL=
REACT_APP_TALAWA_URL="http://API-IP-ADRESS:4000/graphql/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix the API URL placeholder format

The placeholder "API-IP-ADRESS" should follow URL formatting conventions to avoid confusion. Consider using a more standard placeholder format.

-REACT_APP_TALAWA_URL="http://API-IP-ADRESS:4000/graphql/"
+REACT_APP_TALAWA_URL="http://localhost:4000/graphql/"
📝 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.

Suggested change
REACT_APP_TALAWA_URL="http://API-IP-ADRESS:4000/graphql/"
REACT_APP_TALAWA_URL="http://localhost:4000/graphql/"

.env.example Outdated
Comment on lines 17 to 24
REACT_APP_USE_RECAPTCHA=yes

# If you are using Google Recaptcha, i.e., REACT_APP_USE_RECAPTCHA=yes, read the following steps
# Get the google recaptcha site key from google recaptcha admin or https://www.google.com/recaptcha/admin/create
# from here for reCAPTCHA v2 and "I'm not a robot" Checkbox, and paste the key here.
# Note: In domains, fill localhost

REACT_APP_RECAPTCHA_SITE_KEY=
REACT_APP_RECAPTCHA_SITE_KEY=6Lfe3ogqAAAAAC4x1p5OTKq3VUakk9_IAUBxo99n
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Revise reCAPTCHA configuration in example file

While enabling reCAPTCHA by default is good for security, including a real site key in an example file is not recommended. Consider using a placeholder value instead.

REACT_APP_USE_RECAPTCHA=yes

-REACT_APP_RECAPTCHA_SITE_KEY=6Lfe3ogqAAAAAC4x1p5OTKq3VUakk9_IAUBxo99n
+REACT_APP_RECAPTCHA_SITE_KEY=your-recaptcha-site-key-here
📝 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.

Suggested change
REACT_APP_USE_RECAPTCHA=yes
# If you are using Google Recaptcha, i.e., REACT_APP_USE_RECAPTCHA=yes, read the following steps
# Get the google recaptcha site key from google recaptcha admin or https://www.google.com/recaptcha/admin/create
# from here for reCAPTCHA v2 and "I'm not a robot" Checkbox, and paste the key here.
# Note: In domains, fill localhost
REACT_APP_RECAPTCHA_SITE_KEY=
REACT_APP_RECAPTCHA_SITE_KEY=6Lfe3ogqAAAAAC4x1p5OTKq3VUakk9_IAUBxo99n
REACT_APP_USE_RECAPTCHA=yes
# If you are using Google Recaptcha, i.e., REACT_APP_USE_RECAPTCHA=yes, read the following steps
# Get the google recaptcha site key from google recaptcha admin or https://www.google.com/recaptcha/admin/create
# from here for reCAPTCHA v2 and "I'm not a robot" Checkbox, and paste the key here.
# Note: In domains, fill localhost
REACT_APP_RECAPTCHA_SITE_KEY=your-recaptcha-site-key-here
🧰 Tools
🪛 Gitleaks (8.21.2)

24-24: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Comment on lines 324 to 485
<CurrentHourIndicator />
)}
<div
className={
expanded === index
? styles.expand_list_container
: styles.list_container
}
style={{ width: 'fit-content' }}
style={{
display: 'flex',
alignItems: 'center',
gap: '8px',
}}
>
<div
className={
expanded === index
? styles.expand_event_list
: styles.event_list
}
>
{/*istanbul ignore next*/}
{expanded === index
? timeEventsList
: timeEventsList?.slice(0, 1)}
</div>
{(timeEventsList?.length > 1 ||
(windowWidth <= 700 && timeEventsList?.length > 0)) && (
<button
className={styles.btn__more}
onClick={() => {
toggleExpand(index);
}}
>
{expanded === index ? 'View less' : 'View all'}
</button>
)}
<span
style={{
backgroundColor: 'gray',
borderRadius: '5px',
width: '20px',
height: '12px',
display: 'inline-block',
}}
></span>
<span style={{ fontSize: '14px', color: '#555555' }}>
Holidays
</span>
</div>

{/* Events Created by Organization */}
<div
style={{
display: 'flex',
alignItems: 'center',
gap: '8px',
}}
>
<span
style={{
backgroundColor: '#42a5f5',
borderRadius: '5px',
width: '20px',
height: '12px',
display: 'inline-block',
}}
></span>
<span style={{ fontSize: '14px', color: '#555555' }}>
Events Created by Organization
</span>
</div>

{/* Events Created by User */}
<div
style={{
display: 'flex',
alignItems: 'center',
gap: '8px',
}}
>
<span
style={{
backgroundColor: '#8bc34a',
borderRadius: '5px',
width: '20px',
height: '12px',
display: 'inline-block',
}}
></span>
<span style={{ fontSize: '14px', color: '#555555' }}>
Events Created by User
</span>
</div>
</div>
</div>
);
})}
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Move inline styles to CSS modules

The extensive use of inline styles makes the code harder to maintain and violates separation of concerns. Consider moving these styles to the existing CSS module file.

Example refactor:

- style={{
-   display: 'flex',
-   flexDirection: 'row',
-   justifyContent: 'space-between',
-   alignItems: 'flex-start',
-   padding: '20px',
-   gap: '20px',
-   backgroundColor: '#f9f9f9',
- }}
+ className={styles.calendarEventsContainer}

Then in EventCalendar.module.css:

.calendarEventsContainer {
  display: flex;
  flex-direction: row;
  justify-content: space-between;
  align-items: flex-start;
  padding: 20px;
  gap: 20px;
  background-color: #f9f9f9;
}

Comment on lines 363 to 368
{holidays
?.filter(
(holiday) =>
parseInt(holiday.date.slice(0, 2), 10) - 1 ===
currentMonth,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve holiday date parsing logic

The current date parsing using string slicing is fragile and could break with different date formats. Consider using dayjs for consistent date handling.

- parseInt(holiday.date.slice(0, 2), 10) - 1 === currentMonth
+ dayjs(holiday.date, 'MM-DD').month() === currentMonth

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
src/components/EventCalendar/EventCalendar.module.css (1)

343-379: Consider adding responsive design for new elements.

The new styles for indicators and legends might need adjustments for mobile views. Consider adding media queries to handle:

  • Indicator sizes on smaller screens
  • Legend layout and spacing in mobile view
  • Text size adjustments for better readability

Example implementation:

@media only screen and (max-width: 700px) {
  .listContainer,
  .eventsLegend {
    gap: 4px;
  }
  
  .holidayIndicator,
  .organizationIndicator {
    width: 16px;
    height: 10px;
  }
  
  .holidayText,
  .legendText {
    font-size: 12px;
  }
}
src/components/EventCalendar/EventCalendar.tsx (2)

77-77: Remove console.log statement

Debug logging should be removed before committing code.

-  console.log(hours);

Line range hint 652-656: Remove duplicate YearlyEventCalender rendering

The YearlyEventCalender component is being rendered twice, which could cause performance issues and unexpected behavior.

      </div>
-      <div>
-        {viewType == ViewType.YEAR ? (
-          <YearlyEventCalender eventData={eventData} />
-        ) : (
-          <div className={styles.calendar__hours}>{renderHours()}</div>
-        )}
-      </div>
    </div>
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 6103bdd and e65bf72.

📒 Files selected for processing (3)
  • .env.example (1 hunks)
  • src/components/EventCalendar/EventCalendar.module.css (1 hunks)
  • src/components/EventCalendar/EventCalendar.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .env.example
🔇 Additional comments (6)
src/components/EventCalendar/EventCalendar.module.css (3)

343-348: LGTM! Clean flexbox implementation for list container.

The implementation follows modern CSS best practices using flexbox for alignment and spacing.


362-366: LGTM! Consistent legend styling.

The events legend implementation maintains consistency with the list container styling pattern.


376-379: LGTM! Consistent and accessible text styling.

The text styling provides good readability and maintains consistency with holiday text styling.

src/components/EventCalendar/EventCalendar.tsx (3)

382-383: Use consistent date parsing approach

The date parsing using string slicing is fragile and could break with different date formats. Consider using dayjs consistently throughout the component, as already done in the filter logic above.


324-454: Move inline styles to CSS modules

The extensive use of inline styles makes the code harder to maintain and violates separation of concerns. The component already uses CSS modules for other styles.


322-455: Implementation aligns with PR objectives

The new calendar view successfully implements the required features:

  1. Comprehensive list of holidays
  2. List of user and organization-created events
  3. Color coding for different types of events

The functionality looks good, though the code structure could be improved by addressing the inline styles issue.

Comment on lines 368 to 374
.organizationIndicator {
background-color: #42a5f5;
border-radius: 5px;
width: 20px;
height: 12px;
display: inline-block;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Align organization indicator color with existing theme.

There's an inconsistency between the new .organizationIndicator using #42a5f5 and the existing .orgEvent__color using rgba(82, 172, 255, 0.5). Consider using the same color scheme for consistency.

.organizationIndicator {
-  background-color: #42a5f5;
+  background-color: rgba(82, 172, 255, 0.5);
  border-radius: 5px;
  width: 20px;
  height: 12px;
  display: inline-block;
}
📝 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.

Suggested change
.organizationIndicator {
background-color: #42a5f5;
border-radius: 5px;
width: 20px;
height: 12px;
display: inline-block;
}
.organizationIndicator {
background-color: rgba(82, 172, 255, 0.5);
border-radius: 5px;
width: 20px;
height: 12px;
display: inline-block;
}

Comment on lines 350 to 361
.holidayIndicator {
background-color: gray;
border-radius: 5px;
width: 20px;
height: 12px;
display: inline-block;
}

.holidayText {
font-size: 14px;
color: #555555;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Align holiday indicator color with existing theme.

There's an inconsistency between the new .holidayIndicator using gray and the existing .holidays__color using rgba(0, 0, 0, 0.15). Consider using the same color scheme for consistency.

.holidayIndicator {
-  background-color: gray;
+  background-color: rgba(0, 0, 0, 0.15);
  border-radius: 5px;
  width: 20px;
  height: 12px;
  display: inline-block;
}
📝 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.

Suggested change
.holidayIndicator {
background-color: gray;
border-radius: 5px;
width: 20px;
height: 12px;
display: inline-block;
}
.holidayText {
font-size: 14px;
color: #555555;
}
.holidayIndicator {
background-color: rgba(0, 0, 0, 0.15);
border-radius: 5px;
width: 20px;
height: 12px;
display: inline-block;
}
.holidayText {
font-size: 14px;
color: #555555;
}

changed some changes according to the requirement
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
src/components/EventCalendar/EventCalendar.tsx (1)

Line range hint 667-675: Remove duplicate rendering of calendar components

The YearlyEventCalender and calendar hours are being rendered twice. This creates unnecessary DOM elements and may cause performance issues.

- <div>
-   {viewType == ViewType.YEAR ? (
-     <YearlyEventCalender eventData={eventData} />
-   ) : (
-     <div className={styles.calendar__hours}>{renderHours()}</div>
-   )}
- </div>
src/components/EventCalendar/EventCalendar.module.css (2)

343-348: Maintain consistent naming convention for CSS classes

There's an inconsistency in class naming: .listContainer uses camelCase while .list_container (line 225) uses snake_case. Consider using snake_case consistently across the codebase.

-.listContainer {
+.list_container_new {
  width: fit-content;
  display: flex;
  align-items: center;
  gap: 8px;
}

358-361: Consider consolidating duplicate text styles

.holidayText and .legendText share identical styles. Consider creating a common class to reduce duplication.

+.commonText {
+  font-size: 14px;
+  color: #555555;
+}
+
-.holidayText {
-  font-size: 14px;
-  color: #555555;
-}
+.holidayText {
+  composes: commonText;
+}
-
-.legendText {
-  font-size: 14px;
-  color: #555555;
-}
+.legendText {
+  composes: commonText;
+}

Also applies to: 376-379

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e65bf72 and 9442bb2.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • package.json (3 hunks)
  • src/components/EventCalendar/EventCalendar.module.css (1 hunks)
  • src/components/EventCalendar/EventCalendar.tsx (4 hunks)
🔇 Additional comments (11)
src/components/EventCalendar/EventCalendar.tsx (4)

324-332: Move inline styles to CSS modules

The extensive use of inline styles makes the code harder to maintain and violates separation of concerns.

Also applies to: 336-342, 391-398, 432-436


381-382: Improve holiday date parsing logic

The current date parsing using string slicing is fragile and could break with different date formats.


Line range hint 651-661: LGTM! Well-structured calendar rendering logic

The calendar rendering logic is well-organized, using proper separation of concerns and CSS modules.


362-366: Verify holiday filtering implementation across the codebase

Let's ensure the holiday filtering logic is consistently implemented across the application.

✅ Verification successful

Holiday date filtering implementation is consistent

The holiday filtering implementation in EventCalendar.tsx is the only instance in the codebase and is consistently implemented. The component uses two approaches for holiday handling:

  • Filtering holidays by month using dayjs(holiday.date, 'MM-DD').month()
  • Matching specific dates using dayjs(date).format('MM-DD')

Both implementations correctly handle the 'MM-DD' date format used throughout the component.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other implementations of holiday filtering
rg -l "holiday.*filter" --type ts --type tsx

# Search for date parsing patterns
ast-grep --pattern 'holiday.date.slice'

Length of output: 346


Script:

#!/bin/bash
# Search for holiday filtering with correct file type
rg -l "holiday.*filter" --type typescript --type-add 'typescript:*.{ts,tsx}'

# Search for holiday date parsing patterns with dayjs
rg "dayjs.*holiday.date" --type typescript --type-add 'typescript:*.{ts,tsx}'

# Search for holiday-related code
rg "holiday\." -A 3 --type typescript --type-add 'typescript:*.{ts,tsx}'

Length of output: 2270

src/components/EventCalendar/EventCalendar.module.css (4)

350-356: Holiday indicator implementation looks good!

The color scheme matches the existing .holidays__color class, maintaining consistency throughout the calendar view.


362-366: Events legend implementation looks good!

The flexbox implementation provides proper alignment and spacing for the legend items, matching the design requirements.


368-374: Organization indicator implementation looks good!

The color scheme matches the existing .orgEvent__color class, maintaining consistency throughout the calendar view.


380-396: Responsive design implementation looks good!

The media query properly adjusts sizes and spacing for mobile views while maintaining consistency with existing breakpoints.

package.json (3)

125-125: LGTM: Type definitions for chart.js.

The addition of @types/react-chartjs-2 with version ^2.5.7 provides proper TypeScript type definitions, ensuring type safety when using chart.js components.


146-146: Review impact of jest-preview downgrade.

The downgrade of jest-preview from v0.3.1 to v0.2.3 could potentially affect the new unit tests mentioned in the PR objectives. Please verify that all calendar view tests still function correctly with this version.

✅ Verification successful

No impact from jest-preview downgrade

The verification shows that jest-preview is primarily used for debugging failing tests in the browser, as documented in INSTALLATION.md and CONTRIBUTING.md. The actual usage in test files is minimal, with only two test files importing the debug function:

  • src/components/UserPortal/EventCard/EventCard.test.tsx
  • src/screens/LoginPage/LoginPage.test.tsx

The downgrade from v0.3.1 to v0.2.3 is safe because:

  1. The package is only used as a development tool for test debugging
  2. The core functionality (CSS and file transformations) remains the same in jest.config.js
  3. No git diff changes were detected in test files related to jest-preview
  4. No TODO/FIXME comments or known issues were found
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any test failures or warnings related to jest-preview

# Search for jest-preview usage in test files
echo "Checking jest-preview usage in tests:"
rg "jest-preview" "src/**/*.test.*"

# Look for any TODO/FIXME comments related to jest-preview
echo "Checking for known issues:"
rg "TODO.*jest-preview|FIXME.*jest-preview" "src/**/*"

Length of output: 345


Script:

#!/bin/bash
# Let's first check if there are any test files using fd
echo "Searching for test files:"
fd -e test.js -e test.jsx -e test.ts -e test.tsx

# Then search for jest-preview usage in all files
echo -e "\nChecking jest-preview usage:"
rg "jest-preview"

# Check git diff for changes related to jest-preview
echo -e "\nChecking git diff for jest-preview changes:"
git diff origin/main -- '**/jest-preview*' '**/*.test.*'

# Look for jest configuration files
echo -e "\nSearching for jest config files:"
fd "jest.config" -e js -e ts -e json

Length of output: 13653


29-29: Verify chart.js integration with calendar view.

The addition of chart.js aligns with the PR objectives for implementing an enhanced calendar view with color-coded markers. However, let's verify its integration.

✅ Verification successful

Chart.js is appropriately integrated for event statistics visualization

The addition of chart.js is justified and properly implemented:

  • Used in EventStatistics component for visualizing event attendance data
  • Implements both Bar and Line charts from react-chartjs-2
  • Properly tested with mocked chart components
  • Used for displaying gender demographics and age-based attendance trends
  • Includes data export functionality
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify chart.js usage in calendar components

# Search for chart.js imports and usage in calendar-related files
echo "Checking chart.js imports and usage:"
rg -l "chart.js|react-chartjs-2" "src/**/Event*"
rg -l "chart.js|react-chartjs-2" "src/**/Calendar*"

# Check test coverage for chart.js components
echo "Checking test coverage:"
rg -l "chart.js|react-chartjs-2" "src/**/*.test.*"

Length of output: 464


Script:

#!/bin/bash
# Let's first find the actual calendar and event-related files
echo "Finding calendar and event-related files:"
fd -t f -e js -e jsx -e ts -e tsx . | grep -i "calendar\|event"

echo -e "\nFinding all test files:"
fd -t f "test" -e js -e jsx -e ts -e tsx

echo -e "\nSearching for Chart.js usage across all files:"
rg "chart.js|react-chartjs-2|Chart from" -l

Length of output: 17193


Script:

#!/bin/bash
# Let's examine how Chart.js is being used in the identified files
echo "Checking Chart.js implementation in EventStatistics.tsx:"
rg -A 10 "chart.js|react-chartjs-2|Chart from" "src/components/EventManagement/EventAttendance/EventStatistics.tsx"

echo -e "\nChecking Chart.js test coverage:"
cat "src/components/EventManagement/EventAttendance/EventStatistics.test.tsx"

Length of output: 10622

Comment on lines +334 to +388
{/* Holidays Card */}
<div
style={{
flex: 1,
padding: '20px',
backgroundColor: '#fff8e1',
borderRadius: '10px',
boxShadow: '0px 2px 4px rgba(0, 0, 0, 0.1)',
}}
>
<h3
style={{
fontSize: '16px',
fontWeight: '600',
color: '#333333',
marginBottom: '15px',
}}
>
Holidays
</h3>
<ul
style={{
listStyle: 'none',
padding: '0',
margin: '0',
}}
>
{/* Filter holidays to show only those in the current month */}
{holidays
?.filter(
(holiday) =>
dayjs(holiday.date, 'MM-DD').month() === currentMonth,
)
.map((holiday, index) => (
<li
key={index}
style={{
display: 'flex',
justifyContent: 'space-between',
alignItems: 'center',
marginBottom: '10px',
fontSize: '14px',
color: '#555555',
}}
>
{/* Show month name and holiday date */}
<span style={{ fontWeight: '500', color: '#ff9800' }}>
{months[parseInt(holiday.date.slice(0, 2), 10) - 1]}{' '}
{holiday.date.slice(3)}
</span>
<span>{holiday.name}</span>
</li>
))}
</ul>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Componentize the Holiday and Events cards

The holiday and events sections contain repeated patterns and complex nested structures. Consider extracting these into separate components for better maintainability and reusability.

+ // HolidayCard.tsx
+ interface HolidayCardProps {
+   holidays: Array<{ date: string; name: string }>;
+   currentMonth: number;
+ }
+ 
+ const HolidayCard: React.FC<HolidayCardProps> = ({ holidays, currentMonth }) => {
+   return (
+     <div className={styles.holidayCard}>
+       <h3 className={styles.cardTitle}>Holidays</h3>
+       <ul className={styles.holidayList}>
+         {holidays
+           ?.filter((holiday) => dayjs(holiday.date, 'MM-DD').month() === currentMonth)
+           .map((holiday, index) => (
+             <li key={index} className={styles.holidayItem}>
+               <span className={styles.holidayDate}>
+                 {dayjs(holiday.date, 'MM-DD').format('MMM DD')}
+               </span>
+               <span className={styles.holidayName}>{holiday.name}</span>
+             </li>
+           ))}
+       </ul>
+     </div>
+   );
+ };

Also applies to: 390-452

Copy link
Contributor

@palisadoes palisadoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR will need to be closed.

@@ -8,20 +8,21 @@ PORT=4321

# Run Talawa-api locally in your system, and put its url into the same.

REACT_APP_TALAWA_URL=
REACT_APP_TALAWA_URL="http://localhost:4000/graphql/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"@reduxjs/toolkit": "^2.3.0",
"@vitejs/plugin-react": "^4.3.2",
"babel-plugin-transform-import-meta": "^2.2.1",
"bootstrap": "^5.3.3",
"chart.js": "^4.4.6",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

import { ViewType } from 'screens/OrganizationEvents/OrganizationEvents';
import HolidayCard from '../HolidayCards/HolidayCard';
import { holidays, hours, months, weekdays } from './constants';
import { holidays, months, weekdays } from './constants';
import type { InterfaceRecurrenceRule } from 'utils/recurrenceUtils';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screens designs have changed significantly recently. We have an issue to refactor events. You should ask to be assigned the issue as part of completing this PR.

  1. The file you edited will be affected src/components/EventCalendar/EventCalendar.tsx
  2. Refactor CSS files in src/screens/OrganizationEvents #2514

@palisadoes
Copy link
Contributor

  1. Please submit your PRs against our develop-postgres branch. The default is main, so you will have to modify this before submitting your PR for review. PRs made against main will be closed. Closing.
  2. Please submit your PRs against our develop-postgres branch. To facilitate our reviews, the source and target branches in the PR must be different. The failing test indicates this. Closing.
  3. Screens designs have changed significantly recently. We have an issue to refactor events. You should ask to be assigned the issue as part of completing this PR.
    1. The file you edited will be affected src/components/EventCalendar/EventCalendar.tsx
    2. Refactor CSS files in src/screens/OrganizationEvents #2514

@palisadoes palisadoes closed this Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ADMIN & USER REDESIGN - List of events and holidays in calender view of events.
2 participants