-
Notifications
You must be signed in to change notification settings - Fork 128
feat(ask_sb): Add onboarding tutorial #408
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
Warning Rate limit exceeded@brendan-kellam has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughA multi-step tutorial dialog for the "Agentic Search" feature was introduced, including a new React component, cookie-based dismissal tracking, and related UI enhancements. Supporting constants, actions, and prop interfaces were added or updated across several files to manage tutorial state and dialog presentation. Minor UI and import path adjustments were also made. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Homepage
participant AgenticSearch
participant AgenticSearchTutorialDialog
participant CookieStore
User->>Homepage: Visit page
Homepage->>CookieStore: Read AGENTIC_SEARCH_TUTORIAL_DISMISSED_COOKIE_NAME
CookieStore-->>Homepage: Return dismissal state
Homepage->>AgenticSearch: Pass isTutorialDismissed prop
AgenticSearch->>AgenticSearchTutorialDialog: Show dialog if not dismissed
User->>AgenticSearchTutorialDialog: Dismiss tutorial
AgenticSearchTutorialDialog->>AgenticSearch: Call onTutorialDismissed
AgenticSearch->>CookieStore: setAgenticSearchTutorialDismissedCookie(true)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
This comment has been minimized.
This comment has been minimized.
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: 0
🔭 Outside diff range comments (1)
packages/web/src/actions.ts (1)
2011-2016
: Avoid unnecessaryawait cookies()
and return a value for parity with the other dismiss action
cookies()
is synchronous; awaiting it is a no-op that TypeScript won’t flag but adds cognitive noise.
dismissMobileUnsupportedSplashScreen
returnstrue
, while this new helper returnsvoid
, which is inconsistent when both mutate a cookie for UI state.Diff proposal (only touches newly-added lines):
-export async function setAgenticSearchTutorialDismissedCookie(dismissed: boolean) { - const cookieStore = await cookies(); - cookieStore.set(AGENTIC_SEARCH_TUTORIAL_DISMISSED_COOKIE_NAME, dismissed ? "true" : "false", { - httpOnly: false, // Allow client-side access - }); +export async function setAgenticSearchTutorialDismissedCookie(dismissed: boolean): Promise<boolean> { + const cookieStore = cookies(); + cookieStore.set( + AGENTIC_SEARCH_TUTORIAL_DISMISSED_COOKIE_NAME, + dismissed ? "true" : "false", + { + httpOnly: false, // Client-side access needed + // sameSite, secure, path, etc. could be set later if required + }, + ); + return true; }Also applies to: 2018-2023
🧹 Nitpick comments (1)
packages/web/src/actions.ts (1)
28-28
: Import block is getting large — consider grouping feature-specific constantsYou now import three “UI dismissal” cookie constants in one very long list. Pulling those three into their own import line keeps the diff smaller next time a cookie is added and improves scan-ability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/web/public/ask_sb_tutorial_at_mentions.png
is excluded by!**/*.png
packages/web/public/ask_sb_tutorial_citations.png
is excluded by!**/*.png
packages/web/public/ask_sb_tutorial_search_scope.png
is excluded by!**/*.png
📒 Files selected for processing (11)
packages/web/src/actions.ts
(2 hunks)packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx
(4 hunks)packages/web/src/app/[domain]/components/homepage/agenticSearchTutorialDialog.tsx
(1 hunks)packages/web/src/app/[domain]/components/homepage/askSourcebotDemoCards.tsx
(1 hunks)packages/web/src/app/[domain]/components/homepage/index.tsx
(3 hunks)packages/web/src/app/[domain]/page.tsx
(3 hunks)packages/web/src/components/ui/dialog.tsx
(2 hunks)packages/web/src/features/chat/components/chatBox/chatBox.tsx
(1 hunks)packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
(1 hunks)packages/web/src/features/chat/components/chatBox/searchScopeInfoCard.tsx
(2 hunks)packages/web/src/lib/constants.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
packages/web/src/app/[domain]/components/homepage/askSourcebotDemoCards.tsx
packages/web/src/app/[domain]/page.tsx
packages/web/src/features/chat/components/chatBox/chatBox.tsx
packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx
packages/web/src/lib/constants.ts
packages/web/src/features/chat/components/chatBox/searchScopeInfoCard.tsx
packages/web/src/app/[domain]/components/homepage/index.tsx
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
packages/web/src/components/ui/dialog.tsx
packages/web/src/app/[domain]/components/homepage/agenticSearchTutorialDialog.tsx
packages/web/src/actions.ts
🔇 Additional comments (24)
packages/web/src/lib/constants.ts (1)
27-27
: Constant addition looks goodClearly-named key for the tutorial dismissal cookie – matches existing naming style and avoids collisions.
packages/web/src/app/[domain]/components/homepage/askSourcebotDemoCards.tsx (1)
12-13
: Import path update is sensiblePath now reflects the new feature-scoped location; avoids future circular-import headaches.
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx (1)
14-16
: Consistent relocation of shared cardsBoth info-card imports now point to the colocated chatBox directory, keeping feature boundaries tidy.
packages/web/src/features/chat/components/chatBox/chatBox.tsx (1)
284-287
: Placeholder copy change approvedThe new text better describes the dual refinement mechanisms (scopes & @mentions).
packages/web/src/app/[domain]/page.tsx (3)
13-13
: LGTM! Clean import addition.The import follows the existing pattern and groups logically with other constants.
54-54
: LGTM! Proper cookie handling.The cookie value comparison with
"true"
is correct since cookie values are always strings. The boolean conversion is appropriate for the component prop.
79-79
: LGTM! Consistent prop passing.The new prop is properly integrated into the existing Homepage component call structure.
packages/web/src/app/[domain]/components/homepage/index.tsx (2)
26-26
: LGTM! Proper interface extension.The new boolean prop is correctly typed and follows the existing interface pattern.
37-37
: LGTM! Clean prop drilling with appropriate renaming.The prop is properly destructured and forwarded to the AgenticSearch component. The renaming from
isAgenticSearchTutorialDismissed
toisTutorialDismissed
is good as it's more concise within the AgenticSearch context.Also applies to: 95-95
packages/web/src/features/chat/components/chatBox/searchScopeInfoCard.tsx (3)
1-1
: LGTM! Clean import optimization.Removing unused imports and keeping only the necessary icons improves bundle size and code clarity.
11-11
: LGTM! Better user-friendly terminology.Changing "constrain" to "focus" is more positive and user-friendly while conveying the same meaning.
15-15
: LGTM! Simplified icon implementation.Using a static
BookMarkedIcon
is simpler than dynamic repository icon rendering and maintains consistency with the design.packages/web/src/components/ui/dialog.tsx (2)
34-36
: LGTM! Proper prop type extension.The optional
closeButtonClassName
prop is correctly typed and extends the existing props interface cleanly.
37-37
: LGTM! Clean implementation of customizable close button styling.The prop is properly destructured and applied using the
cn
utility for class combination. This maintains the base styling while allowing customization.Also applies to: 50-50
packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx (4)
9-9
: LGTM! Clean imports for tutorial functionality.The new imports are properly organized and necessary for the tutorial feature implementation.
Also applies to: 14-15
28-28
: LGTM! Proper prop integration.The new
isTutorialDismissed
prop is correctly typed and integrated into the component interface and parameters.Also applies to: 37-37
43-47
: LGTM! Well-implemented tutorial state management.The state initialization using
!isTutorialDismissed
correctly shows the tutorial when it hasn't been dismissed. The callback properly handles both closing the dialog and persisting the dismissal state via cookie.
89-93
: LGTM! Clean conditional rendering.The tutorial dialog is properly conditionally rendered and receives the appropriate close handler. The component integration follows React best practices.
packages/web/src/app/[domain]/components/homepage/agenticSearchTutorialDialog.tsx (6)
1-30
: LGTM: Well-organized imports.The imports are properly structured with good separation between UI components, utilities, assets, and external dependencies. All imports appear to be used within the component.
31-33
: LGTM: Clean interface definition.The interface is well-defined with appropriate typing for the callback prop.
36-76
: Good implementation with minor suggestions.The GitHubStarButton component is well-implemented with proper error handling, caching, and retry logic. A few observations:
- External API dependency: The component relies on GitHub's API which could fail or be rate-limited
- Security: Using
window.open
is acceptable here since it's opening a known safe URL- User experience: Consider showing a loading indicator or skeleton while fetching
Consider adding a loading indicator for better UX:
<span className="font-medium"> { + isLoading ? 'Loading...' : !isLoading && !isError && starCount ? `Star (${formatStarCount(starCount)})` : 'Star' } </span>
251-268
: LGTM: Clean state management and navigation logic.The component properly manages the current step state with appropriate bounds checking in the navigation functions.
270-337
: Excellent dialog implementation with great UX.The dialog component is well-implemented with:
- Proper open/close state management
- Intuitive two-column layout
- Clear navigation with progress indicators
- Appropriate button states for edge cases
- Good use of conditional styling
The fixed height (
h-[525px]
) works well for a modal, but consider making it responsive for smaller screens if needed.
79-249
: Well-structured tutorial content (external URLs verified)The tutorial steps are well-organized with good use of React components, proper accessibility attributes, and clear, informative content. All external links and the hero video asset have been verified and return HTTP 200.
• All external URLs in the tutorial (docs.sourcebot.dev, github.com/sourcebot-dev, sourcebot.dev, and the Google Storage asset) are accessible.
• Consider extracting repeated hardcoded URLs (and the video asset path) into constants or a small configuration module to improve maintainability.
Adds a tutorial modal/useful information about Ask Sourcebot. It will only appear when the user first switches the search mode to
Ask
&& the modal has previously been dismissed.Summary by CodeRabbit
New Features
Enhancements
UI Improvements