Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Move to storybook 5 #323

Merged
merged 20 commits into from
Mar 14, 2019
Merged

Move to storybook 5 #323

merged 20 commits into from
Mar 14, 2019

Conversation

dr3
Copy link
Contributor

@dr3 dr3 commented Mar 5, 2019

Overall change: Move to storybook 5

Code changes:

  • Swap the readme addon for the notes one
  • Update webpack config

'import/no-extraneous-dependencies': 'off'

So, awkward, turns out this never worked... If you go on latest and log paths on line 71 of node_modules/eslint-plugin-import/lib/rules/no-extraneous-dependencies.js it logs

[ '/Users/mcmild07/dev/psammead/package.json',
  '/Users/mcmild07/package.json' ]

instead of what we expected;

[ '/Users/mcmild07/dev/psammead/packages/components/psammead-image/package.json',
  '/Users/mcmild07/dev/psammead/package.json' ]

I will raise an issue to fix this properly (#340), if possible, but for now theres no harm disabling it as it pretty much already is


  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@dr3 dr3 self-assigned this Mar 5, 2019
@dr3 dr3 requested a review from a team as a code owner March 5, 2019 21:16
brandTitle: 'BBC Pssammead',
brandUrl: 'https://github.com/bbc/psammead',
brandImage:
'https://user-images.githubusercontent.com/11341355/54079666-af202780-42d8-11e9-9108-e47ea27fddc5.png',
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 have added this as without it you lose the word 'Psammead' from the top left

Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

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

This looks great locally. ✨ I've just added some comments about updating links in Readmes and tidying up the notes implementation.

.storybook/config.js Outdated Show resolved Hide resolved
.storybook/config.js Show resolved Hide resolved
packages/components/psammead-brand/src/index.stories.jsx Outdated Show resolved Hide resolved
packages/components/psammead-brand/src/index.stories.jsx Outdated Show resolved Hide resolved
sareh
sareh previously approved these changes Mar 11, 2019
twinlensreflex
twinlensreflex previously approved these changes Mar 12, 2019
@dr3 dr3 dismissed stale reviews from twinlensreflex and sareh via 85cb247 March 12, 2019 16:22
Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

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

👍 Works great locally.

@dr3 dr3 requested a review from twinlensreflex March 12, 2019 16:53
Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@ChrisBAshton
Copy link
Contributor

Does this PR solve #266?

@dr3
Copy link
Contributor Author

dr3 commented Mar 13, 2019

@ChrisBAshton Nope

@dr3
Copy link
Contributor Author

dr3 commented Mar 14, 2019

Tested locally. All acts as expected except for the amp stories which require a refresh, howeve that was the case before.

@dr3 dr3 merged commit 5d2e19c into latest Mar 14, 2019
@dr3 dr3 deleted the Storybook5 branch March 14, 2019 12:26
@thekp thekp mentioned this pull request Jul 12, 2019
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants