Skip to content
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
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

chore: update storybook #1487

wants to merge 27 commits into from

Conversation

Mohammer5
Copy link
Contributor

@Mohammer5 Mohammer5 commented Apr 29, 2024

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 root package.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 root package.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') or cy.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.visited
See 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 @^7
This 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.

Copy link

cypress bot commented May 29, 2024

106 failed tests on run #3422 ↗︎

106 478 0 0 Flakiness 0

Details:

chore: remove legacy scripts and GH workflow bandaids
Project: ui Commit: f51318f1f5
Status: Failed Duration: 09:08 💡
Started: Jun 12, 2024 3:21 AM Ended: Jun 12, 2024 3:30 AM
Failed  components/alert/src/alert-bar/features/api.feature • 1 failed test • e2e

View Output

Test Artifacts
AlertBar API > AlertBar will call the onHidden cb when hidden Test Replay Screenshots
Failed  components/alert/src/alert-bar/features/hide.feature • 1 failed test • e2e

View Output

Test Artifacts
Hiding the AlertBar > The hidden prop is toggled Test Replay Screenshots
Failed  components/header-bar/src/features/the_headerbar_can_display_online_status.feature • 1 failed test • e2e

View Output

Test Artifacts
The HeaderBar can display online status > The HeaderBar displays an offline status when offline Test Replay Screenshots
Failed  components/tooltip/src/features/visibility_toggling.feature • 1 failed test • e2e

View Output

Test Artifacts
Tooltip visibility toggling > Showing when Tooltip is on top of element Test Replay Screenshots
Failed  components/select/src/multi-select/features/position.feature • 3 failed tests • e2e

View Output

Test Artifacts
Position of MultiSelect menu dropdown > Flipped rendering when insufficient space below Test Replay Screenshots
Position of MultiSelect menu dropdown > Shifting into view when insufficient space below and above Test Replay Screenshots
Position of MultiSelect menu dropdown > Staying in position when the window is scrolled Test Replay Screenshots

The first 5 failed specs are shown, see all 80 specs in Cypress Cloud.

Review all test suite changes for PR #1487 ↗︎

@Mohammer5 Mohammer5 force-pushed the update-storybook branch 2 times, most recently from 5a34f76 to 41b04bd Compare June 18, 2024 01:44
@Mohammer5 Mohammer5 changed the title Update storybook chore: update storybook Jun 18, 2024
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Jun 18, 2024

🚀 Deployed on https://pr-1487--dhis2-ui.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify June 18, 2024 09:09 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 19, 2024 00:49 Inactive
@Mohammer5 Mohammer5 force-pushed the update-storybook branch 2 times, most recently from c149536 to 647633f Compare July 1, 2024 14:34
@Mohammer5 Mohammer5 force-pushed the update-storybook branch 2 times, most recently from 68c9172 to a9532ff Compare July 11, 2024 07:43
@dhis2-bot dhis2-bot temporarily deployed to netlify July 11, 2024 07:58 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify July 11, 2024 11:29 Inactive
@Mohammer5 Mohammer5 marked this pull request as ready for review July 17, 2024 10:36
@Mohammer5 Mohammer5 requested a review from a team as a code owner July 17, 2024 10:36
@dhis2-bot dhis2-bot temporarily deployed to netlify July 17, 2024 11:59 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify July 22, 2024 12:35 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants