-
Notifications
You must be signed in to change notification settings - Fork 13
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
chore: update storybook #1487
Open
Mohammer5
wants to merge
27
commits into
master
Choose a base branch
from
update-storybook
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
chore: update storybook #1487
Conversation
This file contains 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
458ef4d
to
5e00bd3
Compare
106 failed tests on run #3422 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
AlertBar API > AlertBar will call the onHidden cb when hidden |
Test Replay
Screenshots
|
components/alert/src/alert-bar/features/hide.feature • 1 failed test • e2e
Test | Artifacts | |
---|---|---|
Hiding the AlertBar > The hidden prop is toggled |
Test Replay
Screenshots
|
components/header-bar/src/features/the_headerbar_can_display_online_status.feature • 1 failed test • e2e
Test | Artifacts | |
---|---|---|
The HeaderBar can display online status > The HeaderBar displays an offline status when offline |
Test Replay
Screenshots
|
components/tooltip/src/features/visibility_toggling.feature • 1 failed test • e2e
Test | Artifacts | |
---|---|---|
Tooltip visibility toggling > Showing when Tooltip is on top of element |
Test Replay
Screenshots
|
components/select/src/multi-select/features/position.feature • 3 failed tests • e2e
The first 5 failed specs are shown, see all 80 specs in Cypress Cloud.
Review all test suite changes for PR #1487 ↗︎
5a34f76
to
41b04bd
Compare
🚀 Deployed on https://pr-1487--dhis2-ui.netlify.app |
c149536
to
647633f
Compare
68c9172
to
a9532ff
Compare
a9532ff
to
252c30e
Compare
252c30e
to
d12fed7
Compare
fb23f06
to
713a70b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Implements LIBS-588
Description
This PR updates storybook to the latest version, which allows us to use node v20 and get rid of the
*:legacy
scripts in the rootpackage.json
.This PR changes A LOT of files, so I suppose it doesn't make sense to check the changed lines.
Here's what I did and needs to be reviewed / approved (I omitted some changes, you can look at the commit history for more details):
Changed production story file names
Storybook now expects that files containing stories end with
.stories.js
, which breaks our system of having.stories.e2e.js
and.stories.js
. Simply renaming the test stories to.e2e.stories.js
would include them in our demo storybook.So the files extensions are now
.e2e.stories.js
for the test storybook and.prod.stories.js
for our live demo storybook.New storybook api
The start commands had to be adjusted. So basically every
package.json
was touched.React version
I aligned the react versions in the
package.json
files with the version defined in the rootpackage.json
.The root one had a higher version defined, so that's the one I used.
Markup / CSS changes
The new version of storybook uses a slighly different markup and injects more html into the stories than previously.
Because of that I had to change some css rules (e.g. like this) and selectors in cypress tests (like this or this, most of the time this was the case for selectors like
cy.get('a')
orcy.get('button')
).Circular dependencies
I fixed some circular dependencies where imports went through a sibling/parent
index.js
, see here.storybook references in docs removed
See here. Thanks to @kabaros who figured this out. This was necessary because the build step was failing because of this.
TODO: This needs a follow-up ticket because I think we're hiding previously accessible documentation from the user now.
Removed legacy scripts and GH workflow bandaids
See here.
These were added when upgrading cypress & the nodejs version, with the idea that this PR will remove them again.
Adjust e2e story names and how they're
cy.visit
edSee here. This is a rather large change, but it should be enough to take a look at the first few files.
Open issues
storybook-addon-jsx
Not working since @^7This addon hasn't been updated since breaking changes in the addon api were introduced in version 7.
There is an open issue which described possible solutions I haven't explored yet: storybookjs/addon-jsx#184
TODO: This needs a follow-up ticket, someone needs to investigate whether there's an alternative approach we can use to get the same / similar results
The "Code" tab is loading forever
Right now it just says "Loading source...". This is what previously was the "Story" tab, not sure why it has a different label. On the README of that plugin, you can still see "Source" as the label
TODO: This needs a follow-up ticket. Maybe it happens because we have custom story paths, but I haven't figured it out.
The "Accessibility" tab is loading forever
Right now it just says "Please wait while the accessibility scan is running ..."
TODO: This needs a follow-up ticket. Just like the issue above, this could happen because we have custom story paths, but that's just an uneducated guess.