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(Storybook): migrate to version 6 #801

Merged
merged 74 commits into from
Aug 22, 2022
Merged

Conversation

eszthoff
Copy link
Contributor

@eszthoff eszthoff commented Aug 9, 2022

Pending issues after the upgrade:

  • Console logs no longer redirected to Actions tab. Addon doesn't support v6. No solution at this time
  • migrating text only stories (ArrangingOfZIndices) is incomplete. Tried to move to .mdx file, but was not loaded to the UI.
  • should we update AutosuggestDeprecated story -> no

@antipin
Copy link
Contributor

antipin commented Aug 11, 2022

Previous attempt: #679

@eszthoff eszthoff force-pushed the 268-upgrade-storybook branch from 4a131cb to b00ce2d Compare August 18, 2022 08:29
@eszthoff eszthoff requested a review from mukiienko August 18, 2022 15:05
.storybook/main.js Outdated Show resolved Hide resolved
src/components/DatePicker/DatePicker.tsx Show resolved Hide resolved
src/components/List/List.tsx Show resolved Hide resolved
const { getRules } = require('../scripts/build/webpack.config');

module.exports = {
stories: ['../stories/**/*.@(js|jsx|ts|tsx)'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the old version loading files was implemented via index.ts and we had a right overriding because stories were ordered in the right way in the index.ts. In the current implementation we still can control order by items in the array stories, but first we need to group our stories into subgroups/subfolders Concepts, Theme, Atoms etc (in the same way as we have for preview.js), for example:

 stories: ['../stories/theme/*.@(js|jsx|ts|tsx)', '../stories/**/*.@(js|jsx|ts|tsx)'],

It fixed the issue in Pagination story:
Screenshot 2022-08-19 at 13 19 22

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure I tried this before, and it didn't work. But you are right, and it does work. Nice one!!!
I'll push a commit with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

@mukiienko mukiienko Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have a problem with overriding styles for Pagination, because these styles come from Themeroller that comes after Pagination, we need to put it before the high-level components:

    stories: [
        '../stories/theme/*.@(js|jsx|ts|tsx)',
        '../stories/atoms/*.@(js|jsx|ts|tsx)',
        '../stories/molecules/*.@(js|jsx|ts|tsx)',
        '../stories/organisms/*.@(js|jsx|ts|tsx)',
        '../stories/packages/*.@(js|jsx|ts|tsx)',
        '../stories/**/*.@(js|jsx|ts|tsx)',
    ],

After resolving this feel free to move the task forward

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not convinced that the theme should be at the beginning. Its whole purpose is to overwrite the styles, and then storybook will showcase the components as they will look like if adding that theme to the application. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@eszthoff eszthoff force-pushed the 268-upgrade-storybook branch from fe3f148 to 924bc49 Compare August 19, 2022 11:51
@eszthoff eszthoff merged commit 0513dd1 into master Aug 22, 2022
@eszthoff eszthoff deleted the 268-upgrade-storybook branch August 22, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants