-
-
Notifications
You must be signed in to change notification settings - Fork 10k
Onboarding: Guided tour checklist #32795
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 1 commit
d581a3b
4c9e929
e662368
b0fbf3c
0ff85f0
5110838
e7ca5c1
b91e701
3b384b4
f473e21
69cc7ba
ce5ebc0
4cd3631
bd55f1e
c89bce5
02c9aca
d685d12
489113d
e23d3f9
b7e60c5
46081e5
b92f735
2e65137
6c633c4
91a3519
770baf7
84440e2
36e688a
385066d
7c51785
1a917a3
36df52d
69eac58
5d1b37e
9f8bf34
7db2e15
0e109db
4289e0e
000cae7
076adee
c5cc465
0f5f1a6
894f311
f5f0ad8
dcfe2fc
49492f0
2a2eb6c
b82881f
4c0104f
8030fc3
230577e
405b934
1eea810
8e471bf
c0eb34b
94c5c62
5f1bd57
5ba33eb
9d28bb0
cd97967
10917ad
589f1f0
624bfc6
5845f57
1411869
70914fe
af5996a
571eb37
da9c7b8
8a27b33
2adfb83
870ab94
d6e6f09
85b9fa1
8c5c660
02917c4
073778c
bc7ac39
9aef937
e311a22
1f79889
1c8bb66
056a80f
f0dfab7
d9df904
2fd404c
bb87ae0
7591d75
88afdc1
64c5d96
c2eaf69
c051458
760764f
e49e0ba
f367ae4
990d21e
eb45f76
3cac5c1
e6456e2
bc804e3
aad4c79
48c0efb
a38f6f0
d8293cb
6327559
f329817
858d2a1
7217660
d8e826e
2977758
aecbc3b
8990d19
9dcb0da
8189bb1
ab25430
5a83618
31255a1
05d50a0
e54c910
4af5c3a
d7e0a57
32ee70d
c75648e
0ffd1f7
659e860
a3b63a3
90a716c
5fa9a01
b51a36d
2e80f98
505e7ec
df04cb7
48a1bc8
2eda64c
b9437f2
81cf1f2
5be3a1f
b753270
e00e1ee
46cc469
1defe4c
fb4f186
331d34d
35fa041
f0ac445
8f91857
50b98b6
135a375
72e49a8
68ef860
2e876cc
703de3e
21e2d4a
f4de5b2
baa9d8b
87b19f3
ec29889
2df3d62
cba209c
a585c45
15cc514
5d7f60a
941600c
4e8d31d
499a8a9
d88ac22
839b6a7
0baca2b
3a02ac2
7eaa90f
e0ea357
b29a44a
75f3867
9b509a7
6122ed5
1760149
499b583
cbbeb47
4caa1e1
d6f26a6
44543da
50bba49
d0cb3b7
131ecbb
27a8514
4baf5db
679e4a6
dc8603e
c141c98
dec8cfc
c71d37f
26b351c
89faea2
c5f7c39
bff2d46
a08e8d0
0f548b3
6925f08
1cfaf81
2d2f717
cfccdc6
08bdbd0
a37edcb
8a9fa97
5d02fb8
d79a162
d42319a
bf693a3
5f29a50
a65a429
2ff8d29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,8 +17,15 @@ const userSettingSchema = z.object({ | |
| init: z.object({ skipOnboarding: z.boolean().optional() }).optional(), | ||
| checklist: z | ||
| .object({ | ||
| muted: z.union([z.boolean(), z.array(z.string())]).optional(), | ||
| values: z.record(z.enum(['accepted', 'done', 'skipped'])).optional(), | ||
| values: z | ||
| .record( | ||
| z.strictObject({ | ||
| status: z.enum(['open', 'accepted', 'done', 'skipped']), | ||
| mutedAt: z.number().optional(), | ||
| }) | ||
| ) | ||
| .optional(), | ||
| widget: z.object({ disable: z.boolean().optional() }).optional(), | ||
| }) | ||
| .optional(), | ||
|
Comment on lines
+27
to
+57
Member
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. I do wonder if this is the optimal data format for this.. This data format allows for invalid data. A I suspect it would even make state management easier, as far less array mapping would be required.
Member
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. Still relevant |
||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,8 @@ import { logger } from 'storybook/internal/node-logger'; | |
| import { telemetry } from 'storybook/internal/telemetry'; | ||
|
|
||
| import { globalSettings } from '../../cli'; | ||
| import type { ItemState } from '../../shared/checklist-store'; | ||
| import { | ||
| type ItemStatus, | ||
| type StoreEvent, | ||
| type StoreState, | ||
| UNIVERSAL_CHECKLIST_STORE_OPTIONS, | ||
|
|
@@ -42,17 +42,17 @@ export async function initializeChecklist() { | |
| globalSettings().then((settings) => { | ||
| const checklist = settings.value.checklist; | ||
| const state = { | ||
| muted: checklist?.muted ?? false, | ||
| values: checklist?.values ?? {}, | ||
| widget: checklist?.widget ?? { disable: false }, | ||
| }; | ||
| const setState = ({ | ||
| muted = state.muted, | ||
| values = state.values, | ||
| widget = state.widget, | ||
| }: { | ||
| muted?: boolean | string[]; | ||
| values?: Record<string, ItemStatus>; | ||
| values?: typeof state.values; | ||
| widget?: typeof state.widget; | ||
| }) => { | ||
| settings.value.checklist = { muted, values }; | ||
| settings.value.checklist = { values, widget }; | ||
| settings.save(); | ||
| }; | ||
|
Comment on lines
+37
to
+46
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. Avoid unhandled rejections from Both You can keep the fire‑and‑forget behavior but explicitly swallow/log failures inside the persistence lambdas: - const setState = ({
- items = state.items,
- widget = state.widget,
- }: {
- items?: typeof state.items;
- widget?: typeof state.widget;
- }) => {
- settings.value.checklist = { items: items, widget };
- settings.save();
- };
+ const setState = ({
+ items = state.items,
+ widget = state.widget,
+ }: {
+ items?: typeof state.items;
+ widget?: typeof state.widget;
+ }) => {
+ settings.value.checklist = { items, widget };
+ void settings.save().catch((error) => {
+ logger.error('Failed to persist user checklist state');
+ logger.error(error);
+ });
+ };
@@
- const setState = ({ items }: Pick<StoreState, 'items'>) => cache.set('state', { items });
+ const setState = ({ items }: Pick<StoreState, 'items'>) => {
+ void cache.set('state', { items }).catch((error: unknown) => {
+ logger.error('Failed to persist project checklist state');
+ logger.error(error);
+ });
+ };
@@
- saveProjectState({ items: projectValues });
- saveUserState({ items: userValues, widget: state.widget });
+ saveProjectState({ items: projectValues });
+ saveUserState({ items: userValues, widget: state.widget });This keeps the callback sync from the store’s perspective while making persistence failures explicit and non‑fatal. Also applies to: 48-52, 77-78 🤖 Prompt for AI Agents |
||
| return [state, setState] as const; | ||
|
|
@@ -67,32 +67,35 @@ export async function initializeChecklist() { | |
|
|
||
| store.setState((value) => ({ | ||
| ...value, | ||
| muted: userState.muted, | ||
| muted: | ||
| userState.widget.disable === true || | ||
| Object.entries(userState.values) | ||
| .filter(([_, value]) => value.mutedAt) | ||
| .map(([id]) => id), | ||
| values: { ...userState.values, ...projectState.values }, | ||
| loaded: true, | ||
| })); | ||
|
|
||
| store.onStateChange((state: StoreState, previousState: StoreState) => { | ||
| // Split values into user (accepted, skipped) and project (done) for storage | ||
| const userValues: Record<string, Extract<ItemStatus, 'accepted' | 'skipped'>> = {}; | ||
| const projectValues: Record<string, Extract<ItemStatus, 'done'>> = {}; | ||
|
|
||
| // Split values into project-local (done) and user-local (accepted, skipped) persistence | ||
| const projectValues: Record<string, ItemState> = {}; | ||
| const userValues: Record<string, ItemState> = {}; | ||
| Object.entries(state.values).forEach(([id, value]) => { | ||
| if (value === 'accepted' || value === 'skipped') { | ||
| userValues[id] = value; | ||
| } else if (value === 'done') { | ||
| if (value.status === 'done') { | ||
| projectValues[id] = value; | ||
| } else if (value.status === 'accepted' || value.status === 'skipped') { | ||
| userValues[id] = value; | ||
| } | ||
| }); | ||
| saveProjectState({ values: projectValues } as Pick<StoreState, 'values'>); | ||
| saveUserState({ muted: state.muted, values: userValues }); | ||
| saveProjectState({ values: projectValues }); | ||
| saveUserState({ values: userValues, widget: state.widget }); | ||
|
|
||
|
ghengeveld marked this conversation as resolved.
|
||
| const changedValues = Object.entries(state.values).filter( | ||
| ([key, value]) => value !== previousState.values[key] | ||
| ); | ||
| telemetry('onboarding-checklist', { | ||
| ...(!equals(state.muted, previousState.muted) ? { muted: state.muted } : {}), | ||
| ...(changedValues.length > 0 ? { values: Object.fromEntries(changedValues) } : {}), | ||
| ...(!equals(state.widget, previousState.widget) ? { widget: state.widget } : {}), | ||
| }); | ||
| }); | ||
| } catch (err) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,10 +50,12 @@ const meta = { | |
| beforeEach: async () => { | ||
| mockStore.setState({ | ||
| loaded: true, | ||
| muted: false, | ||
| accepted: ['controls'], | ||
| done: ['render-component'], | ||
| skipped: ['viewports'], | ||
| widget: {}, | ||
| values: { | ||
| controls: { status: 'accepted' }, | ||
| 'render-component': { status: 'done' }, | ||
| viewports: { status: 'skipped' }, | ||
| }, | ||
| }); | ||
| }, | ||
|
Comment on lines
+53
to
+64
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. 🧩 Analysis chainVerify that mockStore state is properly isolated between tests. The 🏁 Script executed: #!/bin/bash
# Description: Check if internal_universalChecklistStore has reset/cleanup mechanisms
# Search for reset, cleanup, or clear methods in the store implementation
rg -n -A5 -B2 'internal_universalChecklistStore|universalChecklistStore' code/core/src/manager/manager-stores.mock.ts code/core/src/manager-api/stores/checklist.ts
# Look for other beforeEach usages with this store for patterns
rg -n -B2 -A5 'beforeEach.*mockStore|beforeEach.*checklistStore' --type=tsLength of output: 1593 🏁 Script executed: #!/bin/bash
# Find experimental_MockUniversalStore implementation
rg -n -A20 'class experimental_MockUniversalStore|export.*experimental_MockUniversalStore' --type=ts
# Check Menu.stories.tsx for complete beforeEach/afterEach hooks and imports
rg -n -B5 -A15 'beforeEach|afterEach' code/core/src/manager/components/sidebar/Menu.stories.tsx
# Search for setState method on mock store
rg -n -B3 -A8 'setState' code/core/src/manager/manager-stores.mock.tsLength of output: 8577 🏁 Script executed: #!/bin/bash
# Find MockUniversalStore implementation and setState method
rg -n -A30 'class MockUniversalStore' code/core/src/shared/universal-store/mock.ts
# Search for setState method specifically in mock store
rg -n -B2 -A10 'setState\s*\(' code/core/src/shared/universal-store/mock.ts
# Check if there are other story files with beforeEach/afterEach patterns for comparison
fd -e stories.tsx -x rg -l 'beforeEach.*afterEach|afterEach' {} \; | head -5Length of output: 1674 🏁 Script executed: #!/bin/bash
# Check Checkbox.stories.tsx for proper beforeEach/afterEach pattern
rg -n -B10 -A10 'beforeEach|afterEach' code/core/src/components/components/Form/Checkbox.stories.tsx
# Look at the full context around Menu.stories.tsx beforeEach to understand file structure
cat code/core/src/manager/components/sidebar/Menu.stories.tsx | head -70 | tail -30Length of output: 1219 Update beforeEach hook to return a cleanup function that resets mockStore state. The Instead of: Use: This provides proper cleanup and prevents state from persisting across story renders. 🤖 Prompt for AI Agents |
||
| } satisfies Meta<typeof SidebarMenu>; | ||
|
|
||
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.
Add defensive checks and timeout for store loading.
This promise-based store check has two critical issues:
items.onboardingSurveywithout existence check could throw a runtime error if the property is missing or undefined.loadedstate, this promise will hang indefinitely, blocking the addon registration.Apply this diff to add defensive checks and a timeout:
📝 Committable suggestion