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

Storybook version update and other dependencies #594

Closed
wants to merge 8 commits into from

Conversation

j-pendlebury
Copy link
Contributor

@j-pendlebury j-pendlebury commented Jun 7, 2019

Resolves N/A

Overall change:
Use version 5.1.3 of Storybook. This is version contains a fix for a bug when viewing components in a new tab canvas. More info here: storybookjs/storybook#6800

Updated:


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

@j-pendlebury j-pendlebury added ws-home Tasks for the WS Home Team ws-articles Tasks for the WS Articles Team simorgh-core-stream labels Jun 7, 2019
@j-pendlebury j-pendlebury self-assigned this Jun 7, 2019
Copy link
Contributor

@DenisHdz DenisHdz left a comment

Choose a reason for hiding this comment

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

CHANGELOG needs to be updated :)

@j-pendlebury
Copy link
Contributor Author

@DenisBBC I didn't bump the version of Psammead, which means no need for CHANGELOG update. Unless I need to bump version of Psammead as well? 🤔

@j-pendlebury
Copy link
Contributor Author

@DenisBBC done now 😄

DenisHdz
DenisHdz previously approved these changes Jun 7, 2019
dr3
dr3 previously approved these changes Jun 7, 2019
Copy link
Contributor

@jamesbhobbs jamesbhobbs left a comment

Choose a reason for hiding this comment

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

There are a bunch of other storybook updates available, e.g. #603 Suggest they are all done at once.

@j-pendlebury j-pendlebury dismissed stale reviews from dr3 and DenisHdz via 8d66351 June 10, 2019 15:05
@j-pendlebury j-pendlebury changed the title Storybook version => 5.1.3 Storybook version update and other dependencies Jun 10, 2019
Co-Authored-By: Drew McMillan <[email protected]>
Copy link
Contributor

@DenisHdz DenisHdz left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@andrew-nowak andrew-nowak left a comment

Choose a reason for hiding this comment

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

This introduces a warning in the console in storybook:
The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".
But this is due to a known issue in storybook: storybookjs/storybook#6998 and I don't think blocks this going forward :)

Otherwise lgtm but needs code owner approval

viewBox={`${svgs[svgName].viewbox.width} ${
svgs[svgName].viewbox.height
}`}
viewBox={`${svgs[svgName].viewbox.width} ${svgs[svgName].viewbox.height}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

🤩

@j-pendlebury
Copy link
Contributor Author

After updating versions of all the dependencies in this PR, the inputProvider doesn't update values for different stories, e.g.
image
The SVG width doesn't update from the news one, which makes the SVG look tiny

image
Headline text doesn't update to Afrique, stays English instead.

Will raise a new PR to update to version 5.1.4 of Storybook, which includes some core-js bug fixes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
simorgh-core-stream ws-articles Tasks for the WS Articles Team ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants