[#384] added visio join button #403
Conversation
WalkthroughAdds a video-conference "Join" button to search results and refactors the event item layout from a grid to horizontal flex, stabilizing column widths and element flex behavior. Adds a short "Join" localization key in English, French, Russian, and Vietnamese. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. 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: 1
🧹 Nitpick comments (1)
src/features/Search/SearchResultsPage.tsx (1)
188-289: Flex row layout and truncation look reasonable; consider small-screen behaviorSwitching the result row to a horizontal flex layout with min-widths and ellipsis for organizer, location, and description should improve readability for dense lists. However, with several fixed
minWidthvalues (date, time, organizer, location) there’s a risk of horizontal squeeze or overflow on narrow viewports.If you see layout issues on smaller screens, consider:
- Making some widths responsive (e.g., via breakpoints) instead of hard-coded pixel
minWidths.- Allowing the title and/or description to wrap on small screens instead of forcing everything into one line.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/features/Search/SearchResultsPage.tsx(4 hunks)src/locales/en.json(1 hunks)src/locales/fr.json(1 hunks)src/locales/ru.json(1 hunks)src/locales/vi.json(1 hunks)
🔇 Additional comments (5)
src/locales/ru.json (1)
244-244: Short join label looks correct and consistent
eventPreview.joinVideoShortmatches the existing longerjoinVideotext and fits the intended “Join” action in Russian.src/locales/fr.json (1)
244-244: French short join label is appropriate
eventPreview.joinVideoShort= "Rejoindre" is consistent with the longerjoinVideolabel and works well as a compact button text.src/locales/vi.json (1)
244-244: Vietnamese short join label is consistent
eventPreview.joinVideoShort= "Tham gia" correctly reflects the “Join” action and matches the longer video-join text.src/locales/en.json (1)
244-244: English short join label wired correctlyAdding
eventPreview.joinVideoShort= "Join" aligns with the existingjoinVideostring and is suitable for the new compact join button.src/features/Search/SearchResultsPage.tsx (1)
4-5: New imports align with added video join buttonImporting
VideocamIconandButtonis consistent with the new video-conference join control further down; no issues spotted here.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/features/Search/SearchResultsPage.tsx(4 hunks)
🔇 Additional comments (5)
src/features/Search/SearchResultsPage.tsx (5)
4-5: LGTM! Clean imports for the video join feature.The VideocamIcon and Button imports are properly structured and used in the video join button implementation.
188-199: LGTM! Layout refactor to flex row enables horizontal element arrangement.The change from grid to flex row layout appropriately supports the addition of inline action buttons and provides consistent spacing through the gap property.
201-232: LGTM! Date and time display with stabilized widths.The minWidth constraints on date (90px) and time (120px) elements prevent layout shifts, and the conditional rendering of time for non-all-day events is correct.
234-275: LGTM! Icon, organizer, and location display with proper flex constraints.The flexShrink: 0 on the calendar icon prevents unwanted shrinking, and the width constraints with ellipsis overflow on organizer and location fields provide stable, predictable layout behavior.
291-306: LGTM! Video join button with proper security hardening.The join button implementation correctly applies security best practices—
window.openincludes"_blank", "noopener,noreferrer"to prevent tabnabbing vulnerabilities. Thee.stopPropagation()call appropriately prevents the row click handler from firing, and theeventPreview.joinVideoShorttranslation key exists in all locale files.Note: Similar
window.opencalls insrc/features/Events/EventDisplayPreview.tsxandsrc/components/Calendar/handlers/eventHandlers.tslack these security parameters and should be addressed in separate PRs.
chibenwa
left a comment
chibenwa
left a comment
There was a problem hiding this comment.
Functionnally tet on preprod: we are fine!
(Please review rabbit comments, if relevant)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/features/Search/SearchResultsPage.tsx (1)
184-289: Flex row layout looks solid; consider truncation for very long titlesThe refactored horizontal layout with fixed-width date/time, constrained organizer/location, and a flexible, ellipsized description reads well and should behave predictably across widths.
One optional UX tweak: the event title at Line 243 currently has no overflow handling. Very long titles could visually overlap adjacent columns or force them to shrink more than desired.
You could add ellipsis handling and make the title the flexible part within its own box:
- <Box display="flex" flexDirection="row" gap={1} sx={{ minWidth: 0 }}> - <Typography sx={styles.M3BodyLarge}> + <Box display="flex" flexDirection="row" gap={1} sx={{ minWidth: 0 }}> + <Typography + sx={{ + ...styles.M3BodyLarge, + flex: 1, + minWidth: 0, + whiteSpace: "nowrap", + overflow: "hidden", + textOverflow: "ellipsis", + }} + > {eventData.data.summary || t("event.untitled")} </Typography> {eventData.data.isRecurrentMaster && <RepeatIcon />} </Box>This keeps the title readable but prevents it from crowding organizer/location/description in edge cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/features/Search/SearchResultsPage.tsx(4 hunks)
🔇 Additional comments (2)
src/features/Search/SearchResultsPage.tsx (2)
4-5: New imports are consistent and fully used
VideocamIconandButtonare correctly imported and used only where needed; no dead imports or type issues visible.
290-305: Join button behavior and hardening look correctThe join button is conditionally rendered only when a video URL is present, stops row-click propagation, and uses
window.openwith"_blank"and"noopener,noreferrer", which addresses the earlier tabnabbing concern. The short localized label also fits the compact layout.No changes requested here.
related to #384
docker image on eriikaah/twake-calendar-front:issue-384-videourl-addition-in-event-search-results
Summary by CodeRabbit
New Features
Improvements
Localization
✏️ Tip: You can customize this high-level summary in your review settings.