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

Media Indicator - Pass script #3075

Merged
merged 6 commits into from
Feb 6, 2020
Merged

Media Indicator - Pass script #3075

merged 6 commits into from
Feb 6, 2020

Conversation

DenisHdz
Copy link
Contributor

@DenisHdz DenisHdz commented Feb 6, 2020

Resolves #NUMBER

Overall change:
Pass script to Media Indicator to apply the right typography.

Code changes:

  • Pass script prop to the MediaIndicator component
  • Update stories
  • Update tests

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

@DenisHdz DenisHdz added ws-home Tasks for the WS Home Team ws-front-page-layout labels Feb 6, 2020
@DenisHdz DenisHdz self-assigned this Feb 6, 2020
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 this fix. When looking locally in Storybook, I see the script-specific font-size and line-height applied.

e.g. http://localhost:8180/?path=/story/components-mediaindicator-video--video-with-duration
Screenshot Media indicator with duration text

Copy link
Contributor

@thekp thekp left a comment

Choose a reason for hiding this comment

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

https://github.com/bbc/psammead/pull/3075/files#diff-9caff7f606acc564ecc5cea4ea7173e6R37-R41

in this example ^ should we add an example of importing scripts and passing it as a prop, since it is a required prop?

@DenisHdz
Copy link
Contributor Author

DenisHdz commented Feb 6, 2020

https://github.com/bbc/psammead/pull/3075/files#diff-9caff7f606acc564ecc5cea4ea7173e6R37-R41

in this example ^ should we add an example of importing scripts and passing it as a prop, since it is a required prop?

Done :)

@DenisHdz DenisHdz requested a review from thekp February 6, 2020 13:49
@thekp
Copy link
Contributor

thekp commented Feb 6, 2020

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ws-front-page-layout ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants