-
-
Notifications
You must be signed in to change notification settings - Fork 19
feat(calendar): show Google logo for events synced from Google Calendar #2658
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
Conversation
It displays the Google logo before the event title for Google-synced events. It can be changed to google calendar icon soon because currently, I am not sure how to add an image to be usable.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughConditional UI enhancements were added to multiple calendar event components. When an event includes a non-empty Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsxOops! Something went wrong! :( ESLint: 9.26.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs packages/ui/src/components/ui/legacy/calendar/EventCard.tsxOops! Something went wrong! :( ESLint: 9.26.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsxOops! Something went wrong! :( ESLint: 9.26.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🪛 GitHub Check: codecov/patchpackages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsx[warning] 98-106: packages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsx#L98-L106 packages/ui/src/components/ui/legacy/calendar/EventCard.tsx[warning] 1017-1025: packages/ui/src/components/ui/legacy/calendar/EventCard.tsx#L1017-L1025 packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx[warning] 855-869: packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx#L855-L869 ⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2658 +/- ##
==========================================
- Coverage 0.82% 0.82% -0.01%
==========================================
Files 2146 2146
Lines 252097 252128 +31
Branches 2586 2586
==========================================
Hits 2078 2078
- Misses 248174 248205 +31
Partials 1845 1845 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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
🧹 Nitpick comments (1)
packages/ui/src/components/ui/legacy/calendar/EventCard.tsx (1)
1017-1024
: Good implementation for Google Calendar integration.The conditional rendering of the Google logo for events synced from Google Calendar is well-implemented. The code correctly checks for the
google_event_id
property and displays an appropriate visual indicator with helpful alt text and tooltip.Consider these minor improvements:
- Add an error handling fallback in case the image fails to load
- Eventually move from using a static image path to an imported SVG icon or a component for better maintainability
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 1017-1023: packages/ui/src/components/ui/legacy/calendar/EventCard.tsx#L1017-L1023
Added lines #L1017 - L1023 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
[warning] 1017-1023: packages/ui/src/components/ui/legacy/calendar/EventCard.tsx#L1017-L1023
Added lines #L1017 - L1023 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- Add Google Calendar icon to event cards and all-day events - Add Google Calendar indicator badge in event edit modal - Implement robust conditional rendering with proper type checking - Add responsive light/dark mode styling for better visibility
1653fa2
to
52e981a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better and improved version.
… feat/add-google-event-icon
There was a problem hiding this 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
♻️ Duplicate comments (1)
packages/ui/src/components/ui/legacy/calendar/EventCard.tsx (1)
1017-1025
: Add missing tests for Google Calendar logo in EventCard.The Google Calendar icon implementation looks good, with proper conditional rendering and accessibility attributes. However, test coverage is needed to ensure this feature works correctly.
As noted in the previous review, please add tests that verify:
- When
google_event_id
is provided, the Google logo is rendered with correct attributes- When
google_event_id
is absent, the Google logo is not renderedExample test assertions:
it('renders Google logo when google_event_id is set', () => { const { queryByAltText } = render(<EventCard {...props} google_event_id="123" />); const logo = queryByAltText('Google Calendar'); expect(logo).toBeInTheDocument(); expect(logo).toHaveAttribute('src', '/media/google-calendar-icon.png'); expect(logo).toHaveAttribute('title', 'Synced from Google Calendar'); }); it('does not render Google logo when google_event_id is undefined', () => { const { queryByAltText } = render(<EventCard {...props} />); expect(queryByAltText('Google Calendar')).toBeNull(); });🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 1017-1024: packages/ui/src/components/ui/legacy/calendar/EventCard.tsx#L1017-L1024
Added lines #L1017 - L1024 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/public/media/google-calendar-icon.png
is excluded by!**/*.png
📒 Files selected for processing (3)
packages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsx
(1 hunks)packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
(1 hunks)packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx
[warning] 855-867: packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx#L855-L867
Added lines #L855 - L867 were not covered by tests
packages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsx
[warning] 98-105: packages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsx#L98-L105
Added lines #L98 - L105 were not covered by tests
packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
[warning] 1017-1024: packages/ui/src/components/ui/legacy/calendar/EventCard.tsx#L1017-L1024
Added lines #L1017 - L1024 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
packages/ui/src/components/ui/legacy/calendar/AllDayEventBar.tsx
Outdated
Show resolved
Hide resolved
packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @Adinorio.
It displays the Google logo before the event title for Google-synced events. It can be changed to google calendar icon soon because currently, I am not sure how to add an image to be usable.
Summary by CodeRabbit