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

Bump most read tov4 #6029

Closed
wants to merge 15 commits into from
Closed

Bump most read tov4 #6029

wants to merge 15 commits into from

Conversation

thekp
Copy link
Contributor

@thekp thekp commented Mar 30, 2020

Partially Resolves BBC-archive/psammead#3233

Overall change:
Most read going to be bumped in this PR: #5739 but it got held up in UX, therefore, I'm going to bump read separately in this PR.

Code changes:

  • Bump most read, update container to use new props (columnLayout)

  • I have assigned myself to this PR and the corresponding issues
  • I have added labels to this PR for the relevant pod(s) affected by these changes
  • I have assigned this PR to the Simorgh project

Testing:

  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • If necessary, I have run the local E2E non-smoke tests relevant to my changes (CYPRESS_APP_ENV=local CYPRESS_SMOKE=false npm run test:e2e:interactive)
  • This PR requires manual testing

@thekp thekp added ws-home most-read Tasks to create Most Read component labels Mar 30, 2020
@thekp thekp added this to the Simorgh 3.0 milestone Mar 30, 2020
@thekp thekp self-assigned this Mar 30, 2020
@thekp thekp marked this pull request as ready for review April 1, 2020 10:18
Copy link
Collaborator

@hotinglok hotinglok left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@EinsteinNjoroge EinsteinNjoroge left a comment

Choose a reason for hiding this comment

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

Looks good 👍
This might need UX/A11y review since its a major change.

@thekp
Copy link
Contributor Author

thekp commented Apr 2, 2020

This might need UX/A11y review since its a major change.

@EinsteinNjoroge
Before the merges in Psammead, I got a UX review from Laura.

@PriyaKR PriyaKR self-assigned this Apr 3, 2020
@PriyaKR PriyaKR removed their assignment Apr 3, 2020
@thekp
Copy link
Contributor Author

thekp commented Apr 3, 2020

This PR: #5739 has become active again and includes the major bumps to most read. So I'll be closing this PR.

@thekp thekp closed this Apr 3, 2020
@thekp thekp deleted the bumpMostReadTov4 branch April 3, 2020 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
most-read Tasks to create Most Read component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce width of double digit for pashto and persian
4 participants