Make onboarding optional, able to be ignored on desktop (take 2)#6628
Make onboarding optional, able to be ignored on desktop (take 2)#6628
Conversation
and update it to not use pixel color checks, and use fixtures.
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
…ttyCAD/modeling-app into franknoirot/adhoc/optional-onboarding
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
franknoirot
left a comment
franknoirot
left a comment
There was a problem hiding this comment.
Alright this is ready
| if (!isOnboardingSubPath(newStatus)) { | ||
| return new Error( | ||
| `Failed to navigate to invalid onboarding status ${newStatus}` | ||
| ) | ||
| } |
There was a problem hiding this comment.
This is what will cause the E2E test to fail if a bad onboarding path is ever constructed. This is the only place in the code base that the onboarding status setting is set to something dynamic. If somehow the user sees this error state they will still be able to use the app by dismissing the onboarding.
|
|
||
| const previousOnboardingStatus: OnboardingStatus = | ||
| previousStep ?? ONBOARDING_SUBPATHS.INDEX | ||
| const nextOnboardingStatus: OnboardingStatus = nextStep ?? 'completed' |
There was a problem hiding this comment.
In #6564 this line was
const nextOnboardingStatus: OnboardingStatus = nextStep + ONBOARDING_SUBPATHS.INDEXFor some inexplicable reason. The OnboardingStatus type def added provides red squiggles in development, in addition to the runtime error handling mentioned elsewhere.
franknoirot
changed the title
CodSpeed Instrumentation Performance ReportMerging #6628 will not alter performanceComparing Summary
|
franknoirot
force-pushed
the
franknoirot/adhoc/optional-onboarding
branch
from
8d926eb to
1ee345a
Compare
pierremtb
left a comment
pierremtb
left a comment
There was a problem hiding this comment.
Gtg once https://github.com/KittyCAD/modeling-app/pull/6628/files?diff=split&w=1#r2070857418 is resolved!
Tested locally on desktop, looks great!
3 participants
Resurrection of #6564 after reverting in #6610. Fixes #6609 which was caused by silly path assembly error. Prevents it in the future by adding proper typing to that path construction, and failing the navigation to a new onboarding step that doesn't match the
OnboardingStatustype, so the existing E2E test will fail noisily in this case.Original PR description below:
Closes #6542. Makes onboarding tutorial optional on both desktop and web, removing all redirection behavior.
Both platforms
Desktop
Screenshare.-.2025-04-29.9_46_32.AM-compressed.mp4
Browser
Screenshare.-.2025-04-29.9_49_57.AM-compressed.mp4
Tests
I consolidated all tests within onboarding-tests.spec.ts into one longer test. Several of those tests—like the "avatar text" tests—checked behavior that will become irrelevant this week when we update the onboarding to no longer mention all the menus and UI. Some of them were redundant, each checking that the code reset and the model was loaded. This test checks that, clicks through all onboarding steps and back, and verifies the "always create a new project" behavior on desktop in several ways. I left a TODO comment in that test that another test will need to be written for the divergent web onboarding behavior when we support browser E2E tests again.
Other
This PR removes one of our circular dependencies.