- Notifications
You must be signed in to change notification settings - Fork1k
chore: fix storybook flakes#19366
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
I'm not so sure what thereactRouterOutlet change does, but the removal of random data inputs and randomized workspace names both seem reasonable to me 👍
I'd like a +1 from@aslilac before merge.
taking a look now |
| data:Array.from({length:30}).map((_,i)=>{ | ||
| constdate=newDate(2024,0,i+1); | ||
| return{ | ||
| date:date.toISOString().split("T")[0], | ||
| amount:5+Math.floor(Math.random()*15), | ||
| }; | ||
| }), |
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.
thank you for just unrolling this loop. 😄 this implementation was clever but is far less understandable at a glance, despite being like 1/3rd the lines of code.
| exportconstgenerateWorkspaceName=()=>{ | ||
| if(isChromatic()){ | ||
| return"chromatic-workspace"; |
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.
I like the idea of just returning a constant here, but could it still be one of our actual random names just for fun?
I just got "yellow-bird-23" as a suggestion which is pretty cute 😌
| constmeta={ | ||
| title:"pages/WorkspaceSchedulePage", | ||
| component:WorkspaceSchedulePage, | ||
| component:WorkspaceSettingsLayout, |
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.
I'd prefer that thiscomponent stay "correct", but the way the react-router storybook addon works really confuses me, so I don't really have a better idea.
BrunoQuaresmaAug 15, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
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.
Me too. The issue was, when using the nested layout function, the react-router decorator was load before any other local decorators, because of how we set it in the preview config, injecting the workspace settings layout before the dashboard provider that is required to make the it work.
IMO, we should simplify the global decorators and let the stories define what they want to use. Improving performance and make easier to identify the story dependencies.
bed4e12 intomainUh oh!
There was an error while loading.Please reload this page.
Close#19365