feat: add settings link to dashboard and vendor sidebar; enhance onbo… - #14
Merged
Conversation
…arding tour with navigation prompts
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe change adds Settings links to dashboard sidebars and extends the onboarding tour with route-aware prompts, mobile header-menu handling, navigation click capture, target polling, and cross-route backward navigation. ChangesDashboard navigation and onboarding
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingTour
participant NavigationMenu
participant DestinationRoute
User->>OnboardingTour: Start tour
OnboardingTour->>NavigationMenu: Show route navigation prompt
User->>NavigationMenu: Select destination menu item
NavigationMenu->>DestinationRoute: Navigate to route
DestinationRoute->>OnboardingTour: Expose next tour target
OnboardingTour->>User: Advance to destination content step
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🤖 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 `@src/components/tour/onboarding-tour.tsx`:
- Around line 326-336: Update the ACTIONS.PREV branch to select the destination
route’s content step immediately before the inserted header-menu prompt steps,
rather than the first matching route returned by stepsRef.current.findIndex.
Skip both navigation-prompt steps when calculating targetIndex, then pass that
adjacent content-step index to setPending while preserving the existing forward
fallback.
In `@src/components/tour/tour-steps.ts`:
- Line 4: Update the FloatingOptions type to remove the explicit any index
signature and replace it with an appropriate type-safe value type compatible
with the project’s strict TypeScript configuration, while preserving the
strategy property and support for additional keys.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42ad17fc-6e8f-4af2-a902-128572a7a499
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
src/components/dashboard/app-sidebar.tsxsrc/components/dashboard/mobile-header-menu.tsxsrc/components/tour/onboarding-tour.tsxsrc/components/tour/tour-steps.tssrc/components/vendors/vendor-sidebar.tsx
Comment on lines
+326
to
+336
| if (action === ACTIONS.PREV) { | ||
| // Going back across pages: land on that page's content step, | ||
| // skipping the navigation-prompt steps on either side. | ||
| const targetIndex = stepsRef.current.findIndex( | ||
| (s) => s.route === nextStep.route, | ||
| ); | ||
| setPending(nextStep.route, targetIndex); | ||
| } else { | ||
| // Forward fallback for a route without a clickable menu item. | ||
| setPending(nextStep.route, nextIndex); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the adjacent source-page step on Back.
findIndex selects the first step for the destination route. For example, Back from Materials selects the Dashboard welcome step instead of the Dashboard navigation step immediately before the prompt. For header-menu navigation, skip both inserted menu prompt steps before selecting the preceding content step.
Proposed fix
- const targetIndex = stepsRef.current.findIndex(
- (s) => s.route === nextStep.route,
- );
+ let targetIndex = nextIndex - 1;
+ while (
+ targetIndex >= 0 &&
+ (stepsRef.current[targetIndex]?.menuOpener ||
+ stepsRef.current[targetIndex]?.menuItemRoute)
+ ) {
+ targetIndex -= 1;
+ }
+ if (targetIndex < 0) return;
setPending(nextStep.route, targetIndex);📝 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
| if (action === ACTIONS.PREV) { | |
| // Going back across pages: land on that page's content step, | |
| // skipping the navigation-prompt steps on either side. | |
| const targetIndex = stepsRef.current.findIndex( | |
| (s) => s.route === nextStep.route, | |
| ); | |
| setPending(nextStep.route, targetIndex); | |
| } else { | |
| // Forward fallback for a route without a clickable menu item. | |
| setPending(nextStep.route, nextIndex); | |
| } | |
| if (action === ACTIONS.PREV) { | |
| // Going back across pages: land on that page's content step, | |
| // skipping the navigation-prompt steps on either side. | |
| let targetIndex = nextIndex - 1; | |
| while ( | |
| targetIndex >= 0 && | |
| (stepsRef.current[targetIndex]?.menuOpener || | |
| stepsRef.current[targetIndex]?.menuItemRoute) | |
| ) { | |
| targetIndex -= 1; | |
| } | |
| if (targetIndex < 0) return; | |
| setPending(nextStep.route, targetIndex); | |
| } else { | |
| // Forward fallback for a route without a clickable menu item. | |
| setPending(nextStep.route, nextIndex); | |
| } |
🤖 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 `@src/components/tour/onboarding-tour.tsx` around lines 326 - 336, Update the
ACTIONS.PREV branch to select the destination route’s content step immediately
before the inserted header-menu prompt steps, rather than the first matching
route returned by stepsRef.current.findIndex. Skip both navigation-prompt steps
when calculating targetIndex, then pass that adjacent content-step index to
setPending while preserving the existing forward fallback.
|
|
||
| export type TourKind = 'student' | 'vendor'; | ||
| export type TourStep = Step & { route: string }; | ||
| type FloatingOptions = { strategy:string; [key: string]: any }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 '\bany\b|no-explicit-any' \
src/components/tour/tour-steps.ts \
eslint.config.* .eslintrc* 2>/dev/null || trueRepository: royal334/my-school-and-I
Length of output: 950
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tour-steps.ts ---'
cat -n src/components/tour/tour-steps.ts | sed -n '1,35p;60,95p;150,230p'
printf '%s\n' '--- FloatingOptions references ---'
rg -n -C 3 'FloatingOptions|floatingOptions|strategy' src/components/tour src 2>/dev/null | head -n 240
printf '%s\n' '--- TypeScript and lint configuration ---'
fd -HI -t f '^(tsconfig.*|eslint\.config\..*|\.eslintrc.*)$' . -x sh -c 'echo "--- $1"; cat -n "$1"' sh {} \;Repository: royal334/my-school-and-I
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- project lint rule ---'
rg -n -C 2 'no-explicit-any' \
node_modules/eslint-config-next \
node_modules/@next/eslint-plugin-next \
node_modules/@typescript-eslint 2>/dev/null | head -n 80 || true
printf '%s\n' '--- react-joyride type ---'
rg -n -C 5 'interface Step|type Step|floatingOptions|FloatingOptions' \
node_modules/react-joyride 2>/dev/null | head -n 120 || trueRepository: royal334/my-school-and-I
Length of output: 9394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tour step consumers ---'
rg -n -C 5 'insertNavPromptSteps|TourStep|Joyride|steps=' src/components/tour src 2>/dev/null | head -n 220
printf '%s\n' '--- Joyride package metadata and declarations ---'
fd -HI -t f . node_modules/react-joyride node_modules/@types/react-joyride 2>/dev/null | head -n 80
rg -n -C 5 'floatingOptions|Floater|interface Step|type Step' \
node_modules/react-joyride node_modules/@types/react-joyride \
package.json package-lock.json 2>/dev/null | head -n 180Repository: royal334/my-school-and-I
Length of output: 15485
Replace any in FloatingOptions.
@typescript-eslint/no-explicit-any rejects this type, and the project uses strict TypeScript.
Proposed fix
-type FloatingOptions = { strategy:string; [key: string]: any };
+type FloatingOptions = { strategy: string; [key: string]: unknown };📝 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
| type FloatingOptions = { strategy:string; [key: string]: any }; | |
| type FloatingOptions = { strategy: string; [key: string]: unknown }; |
🧰 Tools
🪛 ESLint
[error] 4-4: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 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 `@src/components/tour/tour-steps.ts` at line 4, Update the FloatingOptions type
to remove the explicit any index signature and replace it with an appropriate
type-safe value type compatible with the project’s strict TypeScript
configuration, while preserving the strategy property and support for additional
keys.
Sources: Coding guidelines, Linters/SAST tools
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
1 participant
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.
…arding tour with navigation prompts
Summary by CodeRabbit