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

[Emotion lvl 2] Migrated section-label titles.jsx & story-promo-list #3904

Merged
merged 10 commits into from
Oct 17, 2020

Conversation

RichardPK
Copy link
Contributor

@RichardPK RichardPK commented Oct 16, 2020

Resolves #NUMBER

Overall change: A very high-level summary of easily-reproducible changes that can be understood by non-devs.

Code changes:

  • A bullet point list of key code changes that have been made.
  • When describing code changes, try to communicate how and why you implemented something a specific way, not just what has changed.

  • 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

@RichardPK RichardPK self-assigned this Oct 16, 2020
const FlexTextRow = styled(FlexRow).attrs({
const FlexTextRow = styled(FlexRow)``;

FlexTextRow.defaultProps = {
Copy link
Contributor Author

@RichardPK RichardPK Oct 16, 2020

Choose a reason for hiding this comment

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

What do you think - should this just be passed into the FlexTextRow component on ln 165, rather than using defaultProps?

I have used this strategy for the aria- values that were previously being passed in as attrs to IndexLinkCta and SectionLabelLink

Copy link
Contributor

@simonsinclair simonsinclair Oct 16, 2020

Choose a reason for hiding this comment

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

Yeah, I don't think this styled component FlexTextRow is actually needed, because there are no styles attached to it. FlexRow could be used directly with role="text" passed to it.

Copy link
Contributor

@simonsinclair simonsinclair Oct 16, 2020

Choose a reason for hiding this comment

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

I've pushed these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome thank you Simon!

…thub.com:bbc/psammead into emotion-migration-story-promo-list-section-label
@RichardPK RichardPK marked this pull request as ready for review October 16, 2020 14:58
@simonsinclair simonsinclair added the cross-team For visibility for both World Service teams (Engage & Media) label Oct 16, 2020
@simonsinclair simonsinclair added the technical-work Technical debt, support work and building new technical tools and features label Oct 16, 2020
@simonsinclair simonsinclair added this to the Migrate to Emotion milestone Oct 16, 2020
@simonsinclair simonsinclair merged commit 1e79931 into latest Oct 17, 2020
@simonsinclair simonsinclair deleted the emotion-migration-story-promo-list-section-label branch October 17, 2020 12:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cross-team For visibility for both World Service teams (Engage & Media) technical-work Technical debt, support work and building new technical tools and features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants