From eaf26490dce94fbe9f1b2cdcd3c3875384d601af Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 29 May 2026 05:24:45 -0700 Subject: [PATCH] Remove onboarding detour reset effect (#3122) * Remove onboarding detour reset effect * Document onboarding detour effect cleanup --- docs/reference/react-performance-audit.md | 8 +++++--- src/renderer/src/App.tsx | 16 +++++++++------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/reference/react-performance-audit.md b/docs/reference/react-performance-audit.md index 458e5c209..55167aab8 100644 --- a/docs/reference/react-performance-audit.md +++ b/docs/reference/react-performance-audit.md @@ -47,13 +47,13 @@ Initial inventory: ## Coverage Ledger -Current count after low-risk PRs #3038, #3041, #3042, #3044, #3051, #3052, #3053, #3054, #3055, #3056, #3058, #3059, #3060, #3062, #3063, #3064, #3065, #3066, #3067, #3068, #3069, #3083, #3087, #3091, #3100, and #3104: 927 Effect hook call sites. +Current count after low-risk PRs #3038, #3041, #3042, #3044, #3051, #3052, #3053, #3054, #3055, #3056, #3058, #3059, #3060, #3062, #3063, #3064, #3065, #3066, #3067, #3068, #3069, #3083, #3087, #3091, #3100, #3104, and #3122: 926 Effect hook call sites. -Open medium-risk PRs #3070, #3073, #3077, #3089, #3093, #3102, #3108, #3110, #3112, and #3118 each project to 926 Effect hook call sites on the current merged baseline; open medium-risk PRs #3097, #3106, #3116, and #3120 each project to 925; open medium-risk PR #3114 projects to 924; open medium-risk PR #3095 projects to 923; open medium-risk PR #3079 projects to 917; open high-risk PR #3075 projects to 923; and open high-risk PR #3081 projects to 919. These are not counted in the merged baseline until reviewed and merged. +Open medium-risk PRs #3070, #3073, #3077, #3089, #3093, #3102, #3108, #3110, #3112, and #3118 each project to 925 Effect hook call sites on the current merged baseline; open medium-risk PRs #3097, #3106, #3116, and #3120 each project to 924; open medium-risk PR #3114 projects to 923; open medium-risk PR #3095 projects to 922; open medium-risk PR #3079 projects to 916; open high-risk PR #3075 projects to 922; and open high-risk PR #3081 projects to 918. These are not counted in the merged baseline until reviewed and merged. | Area | Files / signal | Scan status | Notes | | ------------------------------ | -------------------------------------------------------------------------------------------------------- | --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | -| Renderer app shell | `src/renderer/src/App.tsx`, root components | Inventory complete, manual review in progress | Confirmation dialog active request mirror covered by #3091. Continue checking global listeners, beforeunload, media-query, sidebar resize, active-tab repair. | +| Renderer app shell | `src/renderer/src/App.tsx`, root components | Inventory complete, manual review in progress | Confirmation dialog active request mirror covered by #3091; onboarding settings detour reset covered by #3122. Continue checking global listeners, beforeunload, media-query, sidebar resize, active-tab repair. | | Terminal / PTY | `components/Terminal.tsx`, `components/terminal-pane/**`, `components/terminal/**` | Inventory complete, manual review pending | High-risk area: xterm lifecycle, scrollback, remote/mobile parity, focus, WebGL, resize. | | Browser pane | `components/browser-pane/**` | Inventory complete, manual review in progress | Highest Effect density: 62 sites in `BrowserPane.tsx`. Browser ref mirrors covered by #3081; continue with driver sync, address bar derived state, find state, webview lifetime. | | Editor / markdown / Monaco | `components/editor/**` | Inventory complete, manual review in progress | Untitled rename reset covered by #3083; copy feedback timers covered by #3097; combined diff local state repairs covered by #3120. Check editor model cleanup, preview scroll restore, search debounce, generated decorations. | @@ -128,6 +128,7 @@ These are candidate batches, not final conclusions. Each item needs code inspect | PR AY | GitHub Project column state | Project view column visibility and width preferences were reloaded in Effects after project/view scope changes. | `ProjectViewList.tsx` covered by #3116 | Medium | | PR AZ | Pet overlay size clamp | Pet overlay position was repaired in an Effect after the persisted overlay size changed. | `PetOverlay.tsx` covered by #3118 | Medium | | PR BA | Combined diff local state repairs | File-tree highlight and clear-notes dialog state were repaired in Effects after diff entry or note-count changes. | `CombinedDiffViewer.tsx` covered by #3120 | Medium | +| PR BB | Onboarding settings detour reset | The App-root onboarding detour flag was cleared in an Effect after Settings navigation or onboarding eligibility changed. | `App.tsx` covered by #3122 | Low | ## Merge Risk Scale @@ -186,6 +187,7 @@ These are candidate batches, not final conclusions. Each item needs code inspect | #3116 | `nwparker/react-perf-project-view-columns` | GitHub Project column hidden/width state reloads during render on scope changes | Medium | Open | `pnpm exec oxlint src/renderer/src/components/github-project/ProjectViewList.tsx`; `pnpm run typecheck:web`; `git diff --check`; AST Effect count 927 -> 925. | | #3118 | `nwparker/react-perf-pet-size-clamp` | Pet overlay position clamps during render on size changes | Medium | Open | `pnpm exec oxlint src/renderer/src/components/pet/PetOverlay.tsx`; `pnpm run typecheck:web`; `git diff --check`; AST Effect count 927 -> 926. | | #3120 | `nwparker/react-perf-combined-diff-state` | Combined diff file-tree highlight and clear-notes dialog repair during render | Medium | Open | `pnpm exec oxlint src/renderer/src/components/editor/CombinedDiffViewer.tsx`; `pnpm run typecheck:web`; `git diff --check`; AST Effect count 927 -> 925. | +| #3122 | `nwparker/react-perf-onboarding-detour` | App onboarding settings detour reset moves out of an Effect | Low | Merged | `pnpm exec oxlint src/renderer/src/App.tsx`; `pnpm run typecheck:web`; `git diff --check`; AST Effect count 927 -> 926. | ## Reproduction Commands diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 0c0986e27..f7f171b38 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -428,6 +428,14 @@ function App(): React.JSX.Element { const featureTipsPromptedThisSessionRef = useRef(false) const featureTipsSuppressedByOnboardingThisSessionRef = useRef(false) const [onboardingSettingsDetour, setOnboardingSettingsDetour] = useState(false) + const shouldRenderOnboarding = onboarding !== null && shouldShowOnboarding(onboarding) + const onboardingSettingsDetourActive = + onboardingSettingsDetour && activeView === 'settings' && shouldRenderOnboarding + if (onboardingSettingsDetour && !onboardingSettingsDetourActive) { + // Why: the settings detour is valid only while Settings is onscreen; clear + // it during render so onboarding can resume without a follow-up Effect pass. + setOnboardingSettingsDetour(false) + } // Subscribe to IPC push events useIpcEvents() @@ -497,12 +505,6 @@ function App(): React.JSX.Element { settings ]) - useEffect(() => { - if (activeView !== 'settings' || !shouldShowOnboarding(onboarding)) { - setOnboardingSettingsDetour(false) - } - }, [activeView, onboarding]) - const beginOnboardingSettingsDetour = useCallback(() => { setOnboardingSettingsDetour(true) }, []) @@ -1728,7 +1730,7 @@ function App(): React.JSX.Element { - {onboarding && shouldShowOnboarding(onboarding) && !onboardingSettingsDetour ? ( + {onboarding && shouldRenderOnboarding && !onboardingSettingsDetourActive ? (