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

Change display of program cards in group3 & update stories to include 4 cards #3184

Merged
merged 14 commits into from
Mar 2, 2020

Conversation

hotinglok
Copy link
Contributor

@hotinglok hotinglok commented Feb 27, 2020

Resolves #3134

Overall change:
Changes the grid layout of the component in group 3 from having the 4th card overflow to another row to having a 2x2 layout instead. Also updated stories to have 4 cards rather than 3.
Originally we were to hide a card so the layout would be:
image
Now UX have agreed on this layout so we don't hide any information just on one breakpoint:
image

Code changes:

  • Changed group3 of programCardGridProps from 2 to 3 in order to make 2x2 layout.
  • Just added onDemand twice to stateTypes to add another card easily to stories and changed the order to be correct.
  • Updated tests to use just the 3 unique state types.
  • Added roles to each grid from Alias radio schedule to be a list #3181 (comment)

  • 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

@hotinglok hotinglok added ws-home Tasks for the WS Home Team radio-schedules labels Feb 27, 2020
@hotinglok hotinglok added this to the Psammead 3.0 milestone Feb 27, 2020
@hotinglok hotinglok self-assigned this Feb 27, 2020
@hotinglok hotinglok marked this pull request as ready for review February 27, 2020 12:29
@@ -3,7 +3,11 @@
<!-- prettier-ignore -->
| Version | Description |
|---------|-------------|
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Left this after conflict resolving 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohno

@@ -11,7 +11,7 @@ export const sentenceCase = text =>
.charAt(0)
.toUpperCase() + text.substring(1);

export const stateTypes = ['onDemand', 'live', 'next'];
export const stateTypes = ['live', 'onDemand', 'onDemand', 'next'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Since now we have two onDemands here, in ProgramCard test should render correctly for ${state} where we go through each state type, we will have two identical snapshots for 'onDemand'. Can you please fix that to avoid repetition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, fixed now

Copy link
Contributor

@OlgaLyubin OlgaLyubin left a comment

Choose a reason for hiding this comment

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

Thank you for the changes, looks good 👍

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.

LGTM

@hotinglok hotinglok merged commit 78cffae into latest Mar 2, 2020
@hotinglok hotinglok deleted the onDemandGroup3 branch March 2, 2020 12:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
radio-schedules ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Radio Schedules: Hide oldest on Demand card on Group 3
3 participants