-
Notifications
You must be signed in to change notification settings - Fork 3k
Add new welcome screen #5531
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
Add new welcome screen #5531
Changes from 2 commits
1b715c0
ff3955d
f084227
66dbaf2
0e8481f
d5040cf
dd9ff72
3d63d9c
29b02df
0701ced
7b3ad03
91339a2
5d2f26e
e101e11
22d09fe
e0e6d03
db04215
b7942ae
e678523
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import ChatView, { ChatViewRef } from "./components/chat/ChatView" | |
| import HistoryView from "./components/history/HistoryView" | ||
| import SettingsView, { SettingsViewRef } from "./components/settings/SettingsView" | ||
| import WelcomeView from "./components/kilocode/welcome/WelcomeView" // kilocode_change | ||
| import OnboardingView from "./components/kilocode/welcome/OnboardingView" // kilocode_change | ||
| import ProfileView from "./components/kilocode/profile/ProfileView" // kilocode_change | ||
| import McpView from "./components/mcp/McpView" // kilocode_change | ||
| import AuthView from "./components/kilocode/auth/AuthView" // kilocode_change | ||
|
|
@@ -94,6 +95,8 @@ const App = () => { | |
| renderContext, | ||
| mdmCompliant, | ||
| apiConfiguration, // kilocode_change | ||
| hasCompletedOnboarding, // kilocode_change: Track onboarding state | ||
| taskHistoryFullLength, // kilocode_change: Used to detect existing users | ||
| } = useExtensionState() | ||
|
|
||
| // Create a persistent state manager | ||
|
|
@@ -314,16 +317,59 @@ const App = () => { | |
| } | ||
| }, [tab]) | ||
|
|
||
| // kilocode_change start: Onboarding handlers | ||
| const handleSelectFreeModels = useCallback(() => { | ||
| // Mark onboarding as complete | ||
| vscode.postMessage({ type: "hasCompletedOnboarding", bool: true }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: "Free models" selection only sets onboarding complete; it may still show the Welcome screen
|
||
| // The default profile is already set up with a free model, so just close welcome | ||
| // This will trigger a state update that sets showWelcome to false | ||
This comment was marked as outdated.
Sorry, something went wrong. |
||
| }, []) | ||
|
|
||
| const handleSelectPremiumModels = useCallback(() => { | ||
| // Mark onboarding as complete | ||
| vscode.postMessage({ type: "hasCompletedOnboarding", bool: true }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Onboarding is marked complete before premium sign-in succeeds
Consider setting completion only after successful authentication (or using a separate "started onboarding" flag). |
||
| // Navigate to auth view which will show the device code and handle the OAuth flow | ||
| // The AuthView auto-starts device auth on mount | ||
| switchTab("auth") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING:
Consider hiding onboarding immediately (e.g., local state) or making the render conditional prioritize |
||
| setAuthReturnTo("chat") | ||
| }, [switchTab]) | ||
|
|
||
| const handleSelectBYOK = useCallback(() => { | ||
| // Mark onboarding as complete | ||
| vscode.postMessage({ type: "hasCompletedOnboarding", bool: true }) | ||
| // Navigate to settings with providers section | ||
| switchTab("settings") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Navigation won’t be visible until onboarding is dismissed Because |
||
| setCurrentSection("providers") | ||
| }, [switchTab]) | ||
| // kilocode_change end | ||
|
|
||
| if (!didHydrateState) { | ||
| return null | ||
| } | ||
|
|
||
| // kilocode_change start: Show OnboardingView for new users who haven't completed onboarding | ||
| // Show onboarding only if: | ||
| // 1. hasCompletedOnboarding is not true (undefined or false) | ||
| // 2. AND user has no task history (meaning they're truly new, not an existing user upgrading) | ||
| // | ||
| // This ensures existing users who upgrade don't see the onboarding screen, | ||
| // while new users who have never used the extension will see it. | ||
| const isExistingUser = (taskHistoryFullLength ?? 0) > 0 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we can also set 'hasCompletedOnboarding' if this is true so we can choose this logic at some point in the future, that way this choice doesn't tie in to specific architecture choices
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @markijbema - can you review the new code? Hopefully this is better integrated now. |
||
| const showOnboarding = hasCompletedOnboarding !== true && !isExistingUser | ||
|
|
||
| // Do not conditionally load ChatView, it's expensive and there's state we | ||
| // don't want to lose (user input, disableInput, askResponse promise, etc.) | ||
| // kilocode_change: no WelcomeViewProvider toggle | ||
| return showWelcome ? ( | ||
| return showOnboarding ? ( | ||
| <OnboardingView | ||
| onSelectFreeModels={handleSelectFreeModels} | ||
| onSelectPremiumModels={handleSelectPremiumModels} | ||
| onSelectBYOK={handleSelectBYOK} | ||
| /> | ||
| ) : showWelcome ? ( | ||
| <WelcomeView /> | ||
| ) : ( | ||
| // kilocode_change end | ||
| <> | ||
| {/* kilocode_change start */} | ||
| <MemoryWarningBanner /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||
| // kilocode_change - new file | ||||||
| import React from "react" | ||||||
| import Logo from "../common/Logo" | ||||||
| import { useAppTranslation } from "@/i18n/TranslationContext" | ||||||
|
|
||||||
| interface OnboardingOptionProps { | ||||||
| title: string | ||||||
| description: string | ||||||
| onClick: () => void | ||||||
| } | ||||||
|
|
||||||
| const OnboardingOption: React.FC<OnboardingOptionProps> = ({ title, description, onClick }) => { | ||||||
| return ( | ||||||
| <button | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: Add an explicit Buttons default to
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem to be a risk, since this is not in a form and unlikely to be. |
||||||
| className="w-full p-5 rounded-lg border border-vscode-panel-border bg-vscode-editor-background hover:bg-vscode-list-hoverBackground cursor-pointer text-left transition-colors" | ||||||
| onClick={onClick}> | ||||||
| <h3 className="text-lg font-semibold text-vscode-foreground m-0 mb-2">{title}</h3> | ||||||
| <p className="text-sm text-vscode-descriptionForeground m-0">{description}</p> | ||||||
| </button> | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| interface OnboardingViewProps { | ||||||
| onSelectFreeModels: () => void | ||||||
| onSelectPremiumModels: () => void | ||||||
| onSelectBYOK: () => void | ||||||
| } | ||||||
|
|
||||||
| const OnboardingView: React.FC<OnboardingViewProps> = ({ onSelectFreeModels, onSelectPremiumModels, onSelectBYOK }) => { | ||||||
| const { t } = useAppTranslation() | ||||||
|
|
||||||
| return ( | ||||||
| <div className="flex flex-col items-center justify-center min-h-screen p-6 bg-vscode-sideBar-background"> | ||||||
| <Logo width={80} height={80} /> | ||||||
|
|
||||||
| <h1 className="text-2xl font-bold text-vscode-foreground text-center mt-4 mb-10"> | ||||||
| {t("kilocode:onboarding.title")} | ||||||
| </h1> | ||||||
|
|
||||||
| <div className="w-full max-w-md flex flex-col gap-4"> | ||||||
| <OnboardingOption | ||||||
| title={t("kilocode:onboarding.freeModels.title")} | ||||||
| description={t("kilocode:onboarding.freeModels.description")} | ||||||
| onClick={onSelectFreeModels} | ||||||
| /> | ||||||
|
|
||||||
| <OnboardingOption | ||||||
| title={t("kilocode:onboarding.premiumModels.title")} | ||||||
| description={t("kilocode:onboarding.premiumModels.description")} | ||||||
| onClick={onSelectPremiumModels} | ||||||
| /> | ||||||
|
|
||||||
| <OnboardingOption | ||||||
| title={t("kilocode:onboarding.byok.title")} | ||||||
| description={t("kilocode:onboarding.byok.description")} | ||||||
| onClick={onSelectBYOK} | ||||||
| /> | ||||||
| </div> | ||||||
| </div> | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| export default OnboardingView | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| // kilocode_change - new file | ||
| // npx vitest src/components/kilocode/welcome/__tests__/OnboardingView.spec.tsx | ||
|
|
||
| import { render, screen, fireEvent } from "@/utils/test-utils" | ||
| import OnboardingView from "../OnboardingView" | ||
|
|
||
| // Mock Logo component | ||
| vi.mock("../../common/Logo", () => ({ | ||
| default: () => <div data-testid="kilo-logo">Kilo Logo</div>, | ||
| })) | ||
|
|
||
| describe("OnboardingView", () => { | ||
| const mockOnSelectFreeModels = vi.fn() | ||
| const mockOnSelectPremiumModels = vi.fn() | ||
| const mockOnSelectBYOK = vi.fn() | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| it("renders the Kilo logo", () => { | ||
| render( | ||
| <OnboardingView | ||
| onSelectFreeModels={mockOnSelectFreeModels} | ||
| onSelectPremiumModels={mockOnSelectPremiumModels} | ||
| onSelectBYOK={mockOnSelectBYOK} | ||
| />, | ||
| ) | ||
|
|
||
| expect(screen.getByTestId("kilo-logo")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("renders the title", () => { | ||
| render( | ||
| <OnboardingView | ||
| onSelectFreeModels={mockOnSelectFreeModels} | ||
| onSelectPremiumModels={mockOnSelectPremiumModels} | ||
| onSelectBYOK={mockOnSelectBYOK} | ||
| />, | ||
| ) | ||
|
|
||
| // The translation key is returned as-is by the test-utils mock | ||
| expect(screen.getByText("kilocode:onboarding.title")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("renders all three options", () => { | ||
| render( | ||
| <OnboardingView | ||
| onSelectFreeModels={mockOnSelectFreeModels} | ||
| onSelectPremiumModels={mockOnSelectPremiumModels} | ||
| onSelectBYOK={mockOnSelectBYOK} | ||
| />, | ||
| ) | ||
|
|
||
| expect(screen.getByText("kilocode:onboarding.freeModels.title")).toBeInTheDocument() | ||
| expect(screen.getByText("kilocode:onboarding.freeModels.description")).toBeInTheDocument() | ||
|
|
||
| expect(screen.getByText("kilocode:onboarding.premiumModels.title")).toBeInTheDocument() | ||
| expect(screen.getByText("kilocode:onboarding.premiumModels.description")).toBeInTheDocument() | ||
|
|
||
| expect(screen.getByText("kilocode:onboarding.byok.title")).toBeInTheDocument() | ||
| expect(screen.getByText("kilocode:onboarding.byok.description")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("calls onSelectFreeModels when Free models option is clicked", () => { | ||
| render( | ||
| <OnboardingView | ||
| onSelectFreeModels={mockOnSelectFreeModels} | ||
| onSelectPremiumModels={mockOnSelectPremiumModels} | ||
| onSelectBYOK={mockOnSelectBYOK} | ||
| />, | ||
| ) | ||
|
|
||
| const freeModelsButton = screen.getByText("kilocode:onboarding.freeModels.title").closest("button") | ||
| expect(freeModelsButton).toBeInTheDocument() | ||
| fireEvent.click(freeModelsButton!) | ||
|
|
||
| expect(mockOnSelectFreeModels).toHaveBeenCalledTimes(1) | ||
| expect(mockOnSelectPremiumModels).not.toHaveBeenCalled() | ||
| expect(mockOnSelectBYOK).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it("calls onSelectPremiumModels when Premium models option is clicked", () => { | ||
| render( | ||
| <OnboardingView | ||
| onSelectFreeModels={mockOnSelectFreeModels} | ||
| onSelectPremiumModels={mockOnSelectPremiumModels} | ||
| onSelectBYOK={mockOnSelectBYOK} | ||
| />, | ||
| ) | ||
|
|
||
| const premiumModelsButton = screen.getByText("kilocode:onboarding.premiumModels.title").closest("button") | ||
| expect(premiumModelsButton).toBeInTheDocument() | ||
| fireEvent.click(premiumModelsButton!) | ||
|
|
||
| expect(mockOnSelectPremiumModels).toHaveBeenCalledTimes(1) | ||
| expect(mockOnSelectFreeModels).not.toHaveBeenCalled() | ||
| expect(mockOnSelectBYOK).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it("calls onSelectBYOK when BYOK option is clicked", () => { | ||
| render( | ||
| <OnboardingView | ||
| onSelectFreeModels={mockOnSelectFreeModels} | ||
| onSelectPremiumModels={mockOnSelectPremiumModels} | ||
| onSelectBYOK={mockOnSelectBYOK} | ||
| />, | ||
| ) | ||
|
|
||
| const byokButton = screen.getByText("kilocode:onboarding.byok.title").closest("button") | ||
| expect(byokButton).toBeInTheDocument() | ||
| fireEvent.click(byokButton!) | ||
|
|
||
| expect(mockOnSelectBYOK).toHaveBeenCalledTimes(1) | ||
| expect(mockOnSelectFreeModels).not.toHaveBeenCalled() | ||
| expect(mockOnSelectPremiumModels).not.toHaveBeenCalled() | ||
| }) | ||
| }) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
we should now never show this right? so why have two? Any reason not to ditch the welcomeview?
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.
@markijbema - can you take another look? I removed this in 0e8481f