-
Notifications
You must be signed in to change notification settings - Fork 664
feat: add guided onboarding #1621
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e8a2339
feat: add guided onboarding tests and enhance onboarding logic
zhangmo8 d2ac422
chore: update
zhangmo8 479b1bf
fix: address onboarding ai review findings from #1621 (#1622)
Copilot 74de4cd
chore: update
zhangmo8 2e91de3
fix(ci): pass prcheck format
zerob13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Implementation Plan | ||
|
|
||
| ## Cause | ||
|
|
||
| `NewThreadPage` handles the `first-chat` confirm action by querying the contenteditable element and calling `HTMLElement.focus()`. The chat input is backed by a TipTap editor, so the editor-aware focus path should be invoked through the component itself. | ||
|
|
||
| ## Change | ||
|
|
||
| - Expose a `focusInput` method from `ChatInputBox` that focuses and scrolls the editor into view. | ||
| - Use that exposed method from `NewThreadPage` when the guided `first-chat` confirm action runs. | ||
| - Keep the existing DOM query as a fallback for older or stubbed component shapes. | ||
| - Add a focused renderer test for the confirm action. | ||
|
|
||
| ## Validation | ||
|
|
||
| - Run focused renderer tests for `NewThreadPage` onboarding and `ModelProviderSettings`. | ||
| - Run the repository-required `pnpm run format`, `pnpm run i18n`, and `pnpm run lint` checks. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # Guided Onboarding First Chat Confirm | ||
|
|
||
| ## Problem | ||
|
|
||
| During the guided `first-chat` step, clicking the confirm action can appear to do nothing because the handler only focuses the raw contenteditable element instead of invoking the chat input's editor-aware focus path. | ||
|
|
||
| ## User Story | ||
|
|
||
| As a user on the final onboarding step, I want the confirm action to move focus into the real chat editor so I can immediately start typing my first message. | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - When the guided `first-chat` overlay is visible, clicking the primary confirm action focuses the real chat input editor. | ||
| - The guide remains active until the first successful message send completes the onboarding step. | ||
| - A focused renderer test covers the confirm action calling the chat input focus path. | ||
|
|
||
| ## Non-goals | ||
|
|
||
| - No change to onboarding completion semantics for the first-chat step. | ||
| - No change to chat send behavior. | ||
| - No change to the final guide copy. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Tasks | ||
|
|
||
| - [x] Add SDD artifacts. | ||
| - [x] Expose an editor-aware chat input focus method. | ||
| - [x] Route the `first-chat` confirm action through the exposed focus method. | ||
| - [x] Add focused regression coverage for the confirm action. | ||
| - [x] Run format, i18n, lint, and focused tests. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Implementation Plan | ||
|
|
||
| ## Change | ||
|
|
||
| - Update shared onboarding step resolution fallback ordering. | ||
| - Add guarded step selection when `startGuidedOnboarding` receives a terminal requested step. | ||
| - Tighten renderer onboarding step finalization condition for nullish `currentStepId`. | ||
| - Sync provider settings tab selection from onboarding step in one shared helper used by both watchers. | ||
| - Add runtime response parsing in onboarding client using existing route schema. | ||
| - Update onboarding locale strings for the reviewed non-English locale files. | ||
| - Extend tests for onboarding route behavior and onboarding client response validation. | ||
|
|
||
| ## Validation | ||
|
|
||
| - Run format, i18n, lint, and focused onboarding-related tests for touched modules. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # PR #1621 AI Review Fixes | ||
|
|
||
| ## Problem | ||
|
|
||
| AI review on PR #1621 reported onboarding flow edge cases and missing onboarding localization in several locale files. These issues can leave onboarding on an invalid step, hide the intended settings tab during onboarding, or show untranslated onboarding copy. | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - Resuming current onboarding step prefers an `in_progress` step before `pending` when `currentStepId` is empty. | ||
| - Starting onboarding with a requested completed/skipped step falls back to the next pending step instead of pinning `currentStepId` to a terminal step. | ||
| - Guided onboarding finalization treats both `null` and `undefined` `currentStepId` as no active step. | ||
| - Provider settings tab selection remains aligned with onboarding steps during provider changes. | ||
| - Onboarding renderer client validates bridge responses and throws clear errors when response shape is invalid. | ||
| - Reviewed locale files in this scope no longer show English onboarding copy. | ||
|
|
||
| ## Non-goals | ||
|
|
||
| - No unrelated onboarding UX redesign. | ||
| - No broad localization rewrite beyond the reviewed onboarding strings. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # Tasks | ||
|
|
||
| - [ ] Add SDD issue artifacts. | ||
| - [ ] Fix onboarding logic edge cases (shared/main/renderer). | ||
| - [ ] Harden onboarding client bridge response handling. | ||
| - [ ] Update reviewed onboarding locale translations. | ||
| - [ ] Add/update tests for new behavior. | ||
| - [ ] Run validation commands. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # Plan | ||
|
|
||
| ## Diagnosis | ||
|
|
||
| The local PR Check reproduction fails at `pnpm run format:check` for `src/main/routes/onboarding/onboardingRouteSupport.ts`. | ||
|
|
||
| ## Approach | ||
|
|
||
| Run the repository formatter on the reported file, inspect the resulting diff, then rerun PR Check steps to confirm the failure is resolved. | ||
|
|
||
| ## Test Strategy | ||
|
|
||
| - `pnpm run format:check` | ||
| - `pnpm run i18n` | ||
| - `pnpm run lint` | ||
| - Continue to `pnpm run build` if earlier checks pass. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # PR Check Format Failure | ||
|
|
||
| ## User Story | ||
|
|
||
| As a contributor, I want the PR Check workflow to pass for onboarding route changes so review is not blocked by formatting drift. | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - `pnpm run format:check` passes locally. | ||
| - The workflow-equivalent checks continue past formatting without introducing behavior changes. | ||
| - The fix does not alter guided onboarding state semantics. | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| - No onboarding UX or storage behavior changes. | ||
| - No CI workflow changes unless the reproduced failure requires them. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - Keep the change minimal and compatible with existing formatter rules. | ||
| - Preserve existing user work in the branch. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Tasks | ||
|
|
||
| - [x] Reproduce the PR Check failure locally. | ||
| - [x] Apply the minimal formatting fix. | ||
| - [x] Rerun relevant PR Check commands. | ||
| - [x] Summarize the result and any remaining risks. |
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.