feat: add sync to google sheet button for mobile#8876
Merged
Ur-imazing merged 12 commits intomainfrom Mar 23, 2026
Merged
Conversation
Ur-imazing
added
effort: 2
type: fix
…ing-on-mobile-view
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughVisitors report page now handles Google integration creation and opens a Google Sheets sync dialog; FilterDrawer no longer manages sync state and exposes an onSyncDialogOpen callback. A new GoogleSheetsSyncButton component and tests were added; one localization entry was moved. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VisitorsPage as "Visitors Page"
participant IntegrationAPI as "Integration API"
participant SyncDialog as "GoogleSheetsSyncDialog"
participant Snackbar
User->>VisitorsPage: Click "Sync to Google Sheets"
VisitorsPage->>IntegrationAPI: Start Google integration (OAuth)
IntegrationAPI-->>VisitorsPage: success / error
alt success
VisitorsPage->>Snackbar: show success message
VisitorsPage->>SyncDialog: open
else error
VisitorsPage->>Snackbar: show error message
end
User->>SyncDialog: perform sync actions
SyncDialog-->>VisitorsPage: close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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 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 |
|
View your CI Pipeline Execution ↗ for commit fb7bf34
☁️ Nx Cloud last updated this comment at |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/FilterDrawer.tsx (2)
82-84: Pre-existing:onClickshould be onIconButton, notX2Icon.The click handler is placed on the icon rather than the button. While this works, it's inconsistent with MUI patterns and may have accessibility implications. Consider moving it to the
IconButton.♻️ Suggested fix
- <IconButton sx={{ ml: 'auto' }}> - <X2Icon onClick={handleClose} /> + <IconButton sx={{ ml: 'auto' }} onClick={handleClose} aria-label={t('Close')}> + <X2Icon /> </IconButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/FilterDrawer.tsx` around lines 82 - 84, The onClick handler is currently assigned to X2Icon instead of the parent IconButton; move the click handler to the IconButton (i.e., add onClick={handleClose} to IconButton and remove onClick from X2Icon) and ensure IconButton includes an appropriate aria-label for accessibility; update the JSX using IconButton, X2Icon, and handleClose accordingly.
181-189: Consider adding a guard foronSyncDialogOpenwhen enabled.When
disableExportButtonisfalseandonSyncDialogOpenisundefined, clicking the button does nothing silently. Consider making the prop required or adding defensive handling.Additionally, the Sync button lacks the Tooltip treatment that the Export button has when disabled (lines 190-221), creating an inconsistent UX.
♻️ Consider consistent disabled state handling
If you want consistent tooltip behavior for both buttons when disabled:
+ {disableExportButton ? ( + <Tooltip + title={t( + 'Only team members and journey owners can export data.' + )} + placement="top" + > + <span> + <Button + variant="contained" + color="secondary" + sx={{ width: '100%', mb: 2 }} + disabled + > + {t('Sync to Google Sheets')} + </Button> + </span> + </Tooltip> + ) : ( <Button variant="contained" color="secondary" sx={{ width: '100%', mb: 2 }} onClick={onSyncDialogOpen} - disabled={disableExportButton} > {t('Sync to Google Sheets')} </Button> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/FilterDrawer.tsx` around lines 181 - 189, The Sync button currently calls onSyncDialogOpen directly and shows no tooltip when disabled, so add defensive handling: ensure onSyncDialogOpen is either required or provide a safe no-op fallback (e.g., check if onSyncDialogOpen is defined before invoking inside the Button's onClick or default the prop to () => {}), and wrap the Sync Button with the same Tooltip behavior used by the Export button when disableExportButton is true so disabled state UX is consistent; update references to the Button rendering in FilterDrawer (the Sync Button), the onSyncDialogOpen prop, and the disableExportButton usage accordingly.apps/journeys-admin/src/components/JourneyVisitorsList/GoogleSheetsSyncButton/GoogleSheetsSyncButton.tsx (1)
30-36: RemoveonClickhandler from the disabled button.The
onClick={onSyncClick}is assigned to the disabledIconButton. While MUI'sdisabledprop prevents clicks from firing, having the handler attached is unnecessary and slightly misleading. Removing it clarifies intent.♻️ Suggested fix
<IconButton aria-label={`${t('Sync to Google Sheets')} - ${disabledTooltip}`} - onClick={onSyncClick} disabled > <ArrowRefresh6 /> </IconButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/JourneyVisitorsList/GoogleSheetsSyncButton/GoogleSheetsSyncButton.tsx` around lines 30 - 36, In GoogleSheetsSyncButton remove the unnecessary onClick handler from the disabled IconButton; locate the IconButton component (the one rendering ArrowRefresh6) and only pass onClick={onSyncClick} when the button is enabled (e.g., conditionally set onClick to undefined or render a different prop when disabled) so the disabled state does not include an attached handler and intent is clear; reference IconButton, onSyncClick, and disabled/disabledTooltip when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/FilterDrawer.tsx`:
- Around line 82-84: The onClick handler is currently assigned to X2Icon instead
of the parent IconButton; move the click handler to the IconButton (i.e., add
onClick={handleClose} to IconButton and remove onClick from X2Icon) and ensure
IconButton includes an appropriate aria-label for accessibility; update the JSX
using IconButton, X2Icon, and handleClose accordingly.
- Around line 181-189: The Sync button currently calls onSyncDialogOpen directly
and shows no tooltip when disabled, so add defensive handling: ensure
onSyncDialogOpen is either required or provide a safe no-op fallback (e.g.,
check if onSyncDialogOpen is defined before invoking inside the Button's onClick
or default the prop to () => {}), and wrap the Sync Button with the same Tooltip
behavior used by the Export button when disableExportButton is true so disabled
state UX is consistent; update references to the Button rendering in
FilterDrawer (the Sync Button), the onSyncDialogOpen prop, and the
disableExportButton usage accordingly.
In
`@apps/journeys-admin/src/components/JourneyVisitorsList/GoogleSheetsSyncButton/GoogleSheetsSyncButton.tsx`:
- Around line 30-36: In GoogleSheetsSyncButton remove the unnecessary onClick
handler from the disabled IconButton; locate the IconButton component (the one
rendering ArrowRefresh6) and only pass onClick={onSyncClick} when the button is
enabled (e.g., conditionally set onClick to undefined or render a different prop
when disabled) so the disabled state does not include an attached handler and
intent is clear; reference IconButton, onSyncClick, and disabled/disabledTooltip
when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f210e166-ac72-4c30-a476-3660432a51d5
📒 Files selected for processing (7)
apps/journeys-admin/pages/journeys/[journeyId]/reports/visitors.tsxapps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/FilterDrawer.spec.tsxapps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/FilterDrawer.tsxapps/journeys-admin/src/components/JourneyVisitorsList/GoogleSheetsSyncButton/GoogleSheetsSyncButton.spec.tsxapps/journeys-admin/src/components/JourneyVisitorsList/GoogleSheetsSyncButton/GoogleSheetsSyncButton.tsxapps/journeys-admin/src/components/JourneyVisitorsList/GoogleSheetsSyncButton/index.tslibs/locales/en/apps-journeys-admin.json
edmonday
approved these changes
Mar 19, 2026
…ing-on-mobile-view
…ing-on-mobile-view
This comment has been minimized.
This comment has been minimized.
…ing-on-mobile-view
…ing-on-mobile-view
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Refactor
Breaking Changes