-
Notifications
You must be signed in to change notification settings - Fork 474
add an onboarding upgrade modal #2433
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
Changes from all commits
a735691
fdd51fd
01e65e0
d0558d6
e4e2020
01a8be6
030cb5e
d647b90
92262a7
9282711
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 |
---|---|---|
@@ -1,5 +1,5 @@ | ||
--- | ||
sidebar_position: 3.4 | ||
sidebar_position: 1.5 | ||
id: "waveai" | ||
title: "Wave AI" | ||
--- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,20 @@ | ||
// Copyright 2025, Command Line Inc. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import { OnboardingModal } from "@/app/onboarding/onboarding"; | ||
import { atoms, globalStore } from "@/store/global"; | ||
import { NewInstallOnboardingModal } from "@/app/onboarding/onboarding"; | ||
import { CurrentOnboardingVersion } from "@/app/onboarding/onboarding-features"; | ||
import { UpgradeOnboardingModal } from "@/app/onboarding/onboarding-upgrade"; | ||
import { atoms, globalPrimaryTabStartup, globalStore } from "@/store/global"; | ||
import { modalsModel } from "@/store/modalmodel"; | ||
import * as jotai from "jotai"; | ||
import { useEffect } from "react"; | ||
import semver from "semver"; | ||
import { getModalComponent } from "./modalregistry"; | ||
|
||
const ModalsRenderer = () => { | ||
const clientData = jotai.useAtomValue(atoms.client); | ||
const [tosOpen, setTosOpen] = jotai.useAtom(modalsModel.tosOpen); | ||
const [newInstallOnboardingOpen, setNewInstallOnboardingOpen] = jotai.useAtom(modalsModel.newInstallOnboardingOpen); | ||
const [upgradeOnboardingOpen, setUpgradeOnboardingOpen] = jotai.useAtom(modalsModel.upgradeOnboardingOpen); | ||
const [modals] = jotai.useAtom(modalsModel.modalsAtom); | ||
const rtn: React.ReactElement[] = []; | ||
for (const modal of modals) { | ||
|
@@ -19,14 +23,30 @@ const ModalsRenderer = () => { | |
rtn.push(<ModalComponent key={modal.displayName} {...modal.props} />); | ||
} | ||
} | ||
if (tosOpen) { | ||
rtn.push(<OnboardingModal key={OnboardingModal.displayName} />); | ||
if (newInstallOnboardingOpen) { | ||
rtn.push(<NewInstallOnboardingModal key={NewInstallOnboardingModal.displayName} />); | ||
} | ||
if (upgradeOnboardingOpen) { | ||
rtn.push(<UpgradeOnboardingModal key={UpgradeOnboardingModal.displayName} />); | ||
} | ||
useEffect(() => { | ||
if (!clientData.tosagreed) { | ||
setTosOpen(true); | ||
setNewInstallOnboardingOpen(true); | ||
} | ||
}, [clientData]); | ||
|
||
useEffect(() => { | ||
if (!globalPrimaryTabStartup) { | ||
return; | ||
} | ||
if (!clientData.tosagreed) { | ||
return; | ||
} | ||
const lastVersion = clientData.meta?.["onboarding:lastversion"] ?? "v0.0.0"; | ||
if (semver.lt(lastVersion, CurrentOnboardingVersion)) { | ||
setUpgradeOnboardingOpen(true); | ||
} | ||
}, []); | ||
Comment on lines
+38
to
+49
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. Upgrade modal never opens with current effect dependencies The upgrade gating effect runs only once on mount (empty deps), so it observes the initial 🤖 Prompt for AI Agents
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. globalPrimaryTabStartup is set by the waveInit function which runs before the first react render (in wave.ts). 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. 🧩 Analysis chain🏁 Script executed:
Length of output: 7756 🏁 Script executed:
Length of output: 2966 @sawka Thank you for the clarification! You're absolutely right. Looking at the initialization flow in
So by the time the I apologize for the incorrect analysis. The initialization sequence ensures the values are ready, and the empty deps array prevents unnecessary re-execution. The implementation is sound.
🧬 Code Graph Analysis Results<no_relevant_code_snippets> |
||
useEffect(() => { | ||
globalStore.set(atoms.modalOpen, rtn.length > 0); | ||
}, [rtn]); | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
Guard against null clientData before reading
windowids
.ClientService.GetClientData()
can returnnull
/undefined
during startup or error scenarios, soclientData.windowids
will throw and abort the relaunch. Please restore the null-guard when building the window list.📝 Committable suggestion
🤖 Prompt for AI Agents
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.
null client data is a failure. nothing to do and it should be impossible as the backend will initialize a client before startup.
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.
@sawka, understood! If the backend guarantees client initialization before this code path executes, then the null-guard is indeed unnecessary. Thank you for the clarification on the startup sequence.
✏️ Learnings added