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

Update styled components in all packages #713

Merged
merged 17 commits into from
Jul 2, 2019
Merged

Conversation

ryanmccombe
Copy link
Contributor

@ryanmccombe ryanmccombe commented Jun 26, 2019

Dependency for bbc/simorgh#1910

Overall change: Update all the packages that use styled-components to use the same, latest, version

Code changes:

  • Update the package.json, package-log and changelog of every package that used styled components

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

@ryanmccombe ryanmccombe changed the title Change font family based on product Update styled components in all packages Jun 27, 2019
@ryanmccombe ryanmccombe self-assigned this Jun 27, 2019
@ryanmccombe ryanmccombe marked this pull request as ready for review June 27, 2019 12:57
Copy link
Contributor

@pjlee11 pjlee11 left a comment

Choose a reason for hiding this comment

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

Previous typo in const that we can clear whilst making this change

@@ -65,7 +65,7 @@ const helmetFontStyles = {
`,
};

const nessimPersian = `font-family: NassimPersian, Arial, Verdana, Geneva, Helvetica, sans serif`;
const nessimPersian = `font-family: NassimPersian, Arial, Verdana, Geneva, Helvetica, sans serif;`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const nessimPersian = `font-family: NassimPersian, Arial, Verdana, Geneva, Helvetica, sans serif;`;
const nassimPersian = `font-family: NassimPersian, Arial, Verdana, Geneva, Helvetica, sans serif;`;

Also can you check where else this is using the wrong spelling please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 searched a repo and "nessim" only appears in this file - updated it to nassim

@ryanmccombe ryanmccombe requested a review from pjlee11 June 27, 2019 16:20
package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@pjlee11 pjlee11 left a comment

Choose a reason for hiding this comment

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

Nice work 😎

@ryanmccombe
Copy link
Contributor Author

This is ready for QA

Styled components are used in every component in Psammead

The update wasn't denoted as having any breaking changes, but may be worth a quick validation anyway

@PriyaKR PriyaKR self-assigned this Jul 2, 2019
@PriyaKR
Copy link
Contributor

PriyaKR commented Jul 2, 2019

@ryanmccombe Does this PR only fixes only this PR bbc/simorgh#1910 or does it update the styles for other services as well?

Looking at the psammead-styles change log i can only see addition of Nassim persian fonts.

@PriyaKR
Copy link
Contributor

PriyaKR commented Jul 2, 2019

I have done a regression on the storybook which looks good...all the components using psammead-styles are using the latest version 1.0.0 looking at the file changes but the Nassim fonts for persian are not seen for any component as there needs another issue to be fixed for the Persian services top pick up the Nassim fonts.

@ryanmccombe
Copy link
Contributor Author

@PriyaKR this PR doesn't add the Persian fonts - it was just an initial PR, as having the components using different versions of the the styled-components library was making work on the Persian ticket (and other tickets) a lot more difficult

bbc/simorgh#1910 will be resolved in a separate PR once this is merged

@PriyaKR PriyaKR changed the title Update styled components in all packages DO NOT MERGE - Update styled components in all packages Jul 2, 2019
@PriyaKR
Copy link
Contributor

PriyaKR commented Jul 2, 2019

Copying the comment from the dependency PR Note: the Mozart Headers have been deployed up to test (Mozart Routing Release 1240) but not yet to live. So any PR relating to this issue should not be merged until that has been deployed first - otherwise we'll have console errors on the Live environment.

@sareh
Copy link
Contributor

sareh commented Jul 2, 2019

@PriyaKR The Mozart Routing work is only relevant for the Simorgh application, not for Psammead. This is because it adds Content Security Policy headers for the World Service font domain. So this PR is not dependent on the Mozart Routing work.

@PriyaKR
Copy link
Contributor

PriyaKR commented Jul 2, 2019

thanks @sareh

@PriyaKR PriyaKR changed the title DO NOT MERGE - Update styled components in all packages Update styled components in all packages Jul 2, 2019
@ryanmccombe ryanmccombe merged commit a1df445 into latest Jul 2, 2019
@ryanmccombe ryanmccombe deleted the dynamic-font-family branch July 2, 2019 12:49
| 0.1.0-alpha.3 | [PR#672](https://github.com/BBC/psammead/pull/672) Add margin to top of unordered list |
| 0.1.0-alpha.2 | [PR#534](https://github.com/BBC/psammead/pull/534) Remove Timestamp padding. |
| 0.1.0-alpha.1 | [PR#486](https://github.com/BBC/psammead/pull/486) Create initial package. |
| 1.0.1 | [PR#713](https://github.com/bbc/psammead/pull/713) Update `styled-components` to 4.3.2 |
Copy link
Contributor

Choose a reason for hiding this comment

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

This change has overwritten the 1.0.0 version, so we will need to bump again the Story Promo List to add another record in the CHANGELOG.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants