feat: add onboarding tour for first-time users#58
Conversation
|
@prachishelke1312 is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thanks for your contribution, @prachishelke1312! Please make sure CI passes and the checklist in the PR template is complete. A maintainer will review this soon. — The AgroNavis team |
|
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:
📝 WalkthroughWalkthroughAdds a first-time onboarding tour using ChangesOnboarding Tour
Build Script Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
Add the issue no. The CI lint job is failing due to three critical issues in your frontend code: 1. Missing
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/CropScanTab.tsx (1)
97-104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing dependency in
useEffecthook.The hook calls
loadHistory()(line 103), butloadHistoryis not in the dependency array. SinceloadHistoryis defined withuseCallback, it should be included.♻️ Proposed fix to add missing dependency
useEffect(() => { farmApi.getFarms().then((data: Farm[]) => { setFarms(data || []) }).catch(console.error) // Load all scan history loadHistory() - }, []) + }, [loadHistory])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/CropScanTab.tsx` around lines 97 - 104, The useEffect hook in CropScanTab.tsx calls the loadHistory function but does not include it in the dependency array. Since loadHistory is defined with useCallback, add loadHistory to the dependency array of the useEffect hook (currently an empty array) to ensure the effect properly responds to changes in the loadHistory function and follows React's rules of hooks.frontend/src/components/Dashboard.tsx (1)
184-193:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing dependencies in
useEffecthook.The hook depends on
router(used on line 191) andsetActiveTab(used on line 190), but they are not in the dependency array. This can cause stale closures.♻️ Proposed fix to add missing dependencies
useEffect(() => { const mode = router.query.mode as string; const farmId = router.query.farmId as string; if (mode === 'draw' && farmId && farms.length > 0) { setSelectedFarmId(farmId); setActiveTab('map'); router.replace('/dashboard', undefined, { shallow: true }); } - }, [router.query, farms]); + }, [router.query, farms, router, setActiveTab]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Dashboard.tsx` around lines 184 - 193, The useEffect hook in the Dashboard component uses router and setActiveTab but they are missing from the dependency array. Add router, setActiveTab, and setSelectedFarmId to the dependency array of the useEffect hook so that the effect properly re-runs when these dependencies change and avoids stale closure issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Dashboard.tsx`:
- Line 277: The OnboardingTour component is used in the file but lacks an import
statement, causing a build failure. Add an import statement for OnboardingTour
at the top of the file alongside the other component imports to resolve the
missing dependency.
In `@frontend/src/components/OnboardingTour.tsx`:
- Around line 7-13: The component reads the "agronavis-tour-seen" localStorage
key in the useEffect hook to determine if the tour should run, but it never
writes to this key after the tour completes. Add a callback prop to the Joyride
component that fires when the tour finishes, and in that callback function, set
localStorage.setItem("agronavis-tour-seen", "true") to persist the completion
state so the tour only runs once.
---
Outside diff comments:
In `@frontend/src/components/CropScanTab.tsx`:
- Around line 97-104: The useEffect hook in CropScanTab.tsx calls the
loadHistory function but does not include it in the dependency array. Since
loadHistory is defined with useCallback, add loadHistory to the dependency array
of the useEffect hook (currently an empty array) to ensure the effect properly
responds to changes in the loadHistory function and follows React's rules of
hooks.
In `@frontend/src/components/Dashboard.tsx`:
- Around line 184-193: The useEffect hook in the Dashboard component uses router
and setActiveTab but they are missing from the dependency array. Add router,
setActiveTab, and setSelectedFarmId to the dependency array of the useEffect
hook so that the effect properly re-runs when these dependencies change and
avoids stale closure issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c8626804-b0e5-40d2-858c-4fe4126d5d76
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/package.jsonfrontend/src/components/CropScanTab.tsxfrontend/src/components/Dashboard.tsxfrontend/src/components/OnboardingTour.tsxfrontend/src/components/map/PolygonMapper.tsx
|
Still failing and some |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 12: The package.json manifest has been modified (including the prepare
script), but the corresponding lockfile(s) have not been updated, causing npm ci
to fail in CI. Regenerate the lockfile by running npm install locally, which
will update package-lock.json (and any other lockfiles like npm-shrinkwrap.json
if present) to reflect the changes in package.json, then commit the updated
lockfile(s) along with the package.json changes to make the PR installable in
CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f07d325-82db-49c7-b264-b0100194acb8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
|
Resolved the requested CI issues. Changes made:
Current status:
Could you please re-review the PR when convenient? |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thanks for working on this! The onboarding tour is a fantastic idea and installing react-joyride was the perfect choice. The design of the steps is exactly what we need. I was testing this out locally and noticed two issues that cause the tour to crash when logging in:
When react-joyride tries to find #polygon-map, it throws an error and breaks the tour because the user hasn't clicked on the "Field Map" tab yet. To fix this, we need to programmatically switch tabs using Joyride's callback prop. Could you modify OnboardingTour.tsx to accept the setActiveTab function from Dashboard.tsx? Then, inside OnboardingTour, you can listen to step changes: Before step 1 (Map steps), call setActiveTab('map') Looking forward to merging this once those DOM issues are ironed out! |
|
Thanks for the review! I've addressed the reported issues:
I've pushed the latest changes to this PR. Could you please take another look and let me know if there's anything else that needs adjustment? Thank you! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/map/PolygonMapper.tsx (1)
252-256:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
#confirm-boundary-btntarget is conditionally mounted, so the tour step can miss its target.At Line 252/Line 255, the button exists only when
isCompleteis true. The onboarding step that targets#confirm-boundary-btncan fail for first-time users before polygon completion. Keep the target mounted consistently (e.g., render a disabled button with the same id until complete), or retarget the step to an always-present container.Suggested minimal fix
- {isComplete && ( - <button - onClick={handleConfirm} - id="confirm-boundary-btn" + <button + onClick={handleConfirm} + id="confirm-boundary-btn" + disabled={!isComplete} style={{ padding: '6px 16px', border: 'none', borderRadius: '6px', background: '`#16a34a`', fontSize: '13px', - cursor: 'pointer', + cursor: isComplete ? 'pointer' : 'not-allowed', color: 'white', fontWeight: 500, + opacity: isComplete ? 1 : 0.6, }} > Add field → - </button> - )} + </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/map/PolygonMapper.tsx` around lines 252 - 256, The button with id "confirm-boundary-btn" in the handleConfirm block is conditionally rendered only when isComplete is true, which causes the onboarding tour step targeting it to fail for first-time users before polygon completion. Fix this by keeping the button mounted at all times but rendering it in a disabled state until isComplete becomes true, or alternatively by retargeting the tour step to target a parent container or other always-present element instead of the conditionally mounted button.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Dashboard.tsx`:
- Line 278: The OnboardingTour component receives setActiveTab as a prop but is
not using Joyride callbacks to preemptively switch tabs before steps that target
elements in the map or cropscan tabs are displayed. Modify the OnboardingTour
component to implement Joyride's callback mechanism (such as the callback prop
with event types like before or after) to call setActiveTab to switch to the
appropriate tab (map or cropscan) before advancing to steps that target
selectors within those tabs. This ensures the target elements are mounted and
accessible when Joyride tries to display them, preventing the tour from breaking
when activeTab is overview.
---
Outside diff comments:
In `@frontend/src/components/map/PolygonMapper.tsx`:
- Around line 252-256: The button with id "confirm-boundary-btn" in the
handleConfirm block is conditionally rendered only when isComplete is true,
which causes the onboarding tour step targeting it to fail for first-time users
before polygon completion. Fix this by keeping the button mounted at all times
but rendering it in a disabled state until isComplete becomes true, or
alternatively by retargeting the tour step to target a parent container or other
always-present element instead of the conditionally mounted button.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 21e2b771-1a3e-4b07-8288-921370198cbc
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/package.jsonfrontend/src/components/Dashboard.tsxfrontend/src/components/OnboardingTour.tsxfrontend/src/components/map/PolygonMapper.tsx
💤 Files with no reviewable changes (1)
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/OnboardingTour.tsx
|
Thanks for the review. I've updated the onboarding tour to switch tabs before steps targeting the Field Map and Crop Scan sections. The tour now uses Joyride event handling together with I also verified the project builds successfully after the change and pushed the update to the branch. Please take another look when you have a chance. |
2 participants
Summary
Closes #16
Adds a first-time onboarding tour using react-joyride to help users understand the main workflows in AgroNavis.
Changes
Added
OnboardingTourcomponentAdded onboarding steps for:
Added tour target IDs in
PolygonMapperAdded upload target ID in
CropScanTabIntegrated onboarding tour into
DashboardAdded
react-joyridedependencyTesting
npm run lintnpm run buildBuild completed successfully.
Summary by CodeRabbit
New Features
Implementation Updates
Chores
preparescript to run Husky without fallback.react-joyridefor the tour and removed the prior local dependency entry.