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

Add Grid example to Storybook #3693

Merged
merged 28 commits into from
Sep 16, 2019
Merged

Conversation

EinsteinNjoroge
Copy link
Contributor

@EinsteinNjoroge EinsteinNjoroge commented Sep 10, 2019

Resolves #3679

Overall change: Add Storybook stories for FrontPageMain and ArticleMain to display grid layout and catch future changes in chromatic.

Code changes:

  • Create a Storybook story for the FrontPagesMain component. this exposes frontpages main without analytics.
  • Add a story to ArticleMain to display an article main with Headline, Paragraph, timestamp, Portrait Image with Caption, Landscape Image with Caption and a square Image with Caption._
  • Refactored frontpage story

Screenshots:

FrontPagesMain
Screen Shot 2019-09-12 at 3 35 04 PM

ArticleMain
Screen Shot 2019-09-12 at 2 17 25 PM


  • I have assigned myself to this PR and the corresponding issues
  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@EinsteinNjoroge EinsteinNjoroge self-assigned this Sep 10, 2019
@EinsteinNjoroge EinsteinNjoroge added this to PR in Progress in Simorgh via automation Sep 10, 2019
@EinsteinNjoroge EinsteinNjoroge changed the title Refactor front-page story Add Grid example to Storybook Sep 10, 2019
@EinsteinNjoroge EinsteinNjoroge marked this pull request as ready for review September 10, 2019 15:02
@EinsteinNjoroge EinsteinNjoroge moved this from PR in Progress to Code review in Simorgh Sep 10, 2019
Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

Add Storybook stories for FrontPageMain and ArticleMain to display grid layout and catch future changes in chromatic.

I might be missing something, Is this not covered by the full page snapshots we have? As were trying to slim down the number of stories we run on chromaticqa (money), im not sure of the overlap

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.

Thanks for the PR, @EinsteinCarrey. I've suggested some changes here.

src/app/containers/FrontPage/index.jsx Outdated Show resolved Hide resolved
src/app/containers/ArticleMain/index.stories.jsx Outdated Show resolved Hide resolved
src/app/containers/FrontPageMain/index.jsx Outdated Show resolved Hide resolved
Simorgh automation moved this from Code review to PR in Progress Sep 11, 2019
@EinsteinNjoroge
Copy link
Contributor Author

Add Storybook stories for FrontPageMain and ArticleMain to display grid layout and catch future changes in chromatic.

I might be missing something, Is this not covered by the full page snapshots we have? As were trying to slim down the number of stories we run on chromaticqa (money), im not sure of the overlap

@dr3 currently, we don't have visual tests for the grid layout. The idea here is to display the entire FrontPageMain and ArticleMain in Storybook so that changes to the grid are shown in chromatic.

@EinsteinNjoroge EinsteinNjoroge moved this from PR in Progress to Code review in Simorgh Sep 12, 2019
@dr3
Copy link
Contributor

dr3 commented Sep 12, 2019

The idea here is to display the entire FrontPageMain and ArticleMain in Storybook

@EinsteinCarrey Is the FrontPageMain not shown in https://bbc.github.io/simorgh/?path=/story/pages-front-page--igbo as part of the page?

Simorgh automation moved this from Code review to PR in Progress Sep 12, 2019
@sareh
Copy link
Contributor

sareh commented Sep 12, 2019

@dr3 The Pages|FrontPage stories include the cookie and privacy banners and the Chromatic QA tests often jump around between each run, making them unreliable. Having stories for the Front Page Main container will ensure that the Chromatic QA snapshots are more reliable and therefore useful for viewing Grid Layout diffs.

@EinsteinNjoroge EinsteinNjoroge moved this from PR in Progress to Code review in Simorgh Sep 12, 2019
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.

👍 Looks great. Thanks for the updates.

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.

Great work 👍

Simorgh automation moved this from Code review to Ready for merge Sep 16, 2019
@EinsteinNjoroge EinsteinNjoroge merged commit f70797a into latest Sep 16, 2019
Simorgh automation moved this from Ready for merge to Done Sep 16, 2019
@EinsteinNjoroge EinsteinNjoroge deleted the add-grid-example-to-storybook branch September 16, 2019 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Add Grid example to Storybook
5 participants