-
Notifications
You must be signed in to change notification settings - Fork 1.3k
chore: Storybook 8 using parcel builder #8272
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
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # .storybook-s2/docs/Illustrations.jsx # .storybook-s2/docs/Intro.jsx # .storybook-s2/preview.tsx # package.json # packages/@react-spectrum/badge/chromatic-fc/Badge.stories.tsx # packages/@react-spectrum/badge/chromatic/Badge.stories.tsx # packages/@react-spectrum/calendar/stories/Calendar.stories.tsx # packages/@react-spectrum/calendar/stories/RangeCalendar.stories.tsx # packages/@react-spectrum/illustratedmessage/chromatic/IllustratedMessage.Languages.stories.tsx # packages/@react-spectrum/illustratedmessage/chromatic/IllustratedMessage.stories.tsx # yarn.lock
This reverts commit 225c07b.
let { | ||
UNSAFE_className, | ||
UNSAFE_style, | ||
slot, | ||
'aria-label': ariaLabel, | ||
'aria-hidden': ariaHidden, | ||
styles, | ||
// props are already merged with context via rac's useContextProps in generateS2IconIndex |
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.
potentially problematic for any prop sent over context if it's not meant to end up on an svg
element, it'll be both in ctx
and in props
because of this
so where do we want it? should we even do prop merging in generateS2IconIndex?
@@ -12,6 +12,9 @@ | |||
"@svgr/plugin-jsx": "^8.1.0", | |||
"@svgr/plugin-svgo": "^8.1.0" | |||
}, | |||
"devDependencies": { | |||
"prettier": "^3.3.3" |
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.
it's a peer of svgo plugin... and results in an install warning
will hopefully go away when we switch to parcel handling svgo
@@ -13,6 +13,9 @@ | |||
"@svgr/core": "^8.1.0", | |||
"fs-extra": "^11.0.0" | |||
}, | |||
"devDependencies": { |
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.
same, peer of svgr for some reason
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
# Conflicts: # package.json
Build successful! 🎉 |
Closes
Can be reviewed, new
dev
packages are actually from parcel-bundler/storybook#5once that's merged, I'll update this PR with the new versions
Note, due to [Bug]: a11y addon shows false positives on page load stories in v3 show an error on initial page load. They go away if you change stories and come back.
S2 seems to be protected from this, not sure why, possibly another addon is interfering
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: