Uds 2036 - update multiple card render for unity-react-core cards #1577
Conversation
|
Storybook deployed at https://unity-uds-staging.s3.us-west-2.amazonaws.com/pr-1577/index.html |
There was a problem hiding this comment.
@davidornelas11 I like this approach, it moves us forward without introducing a breaking change. Nice. Appears it will be adaptive for use with multiple column types based on where it's placed.
Only concern at this point is problematic rendering when images have different heights: https://unity-uds-staging.s3.us-west-2.amazonaws.com/pr-1577/@asu/unity-react-core/index.html?path=/story/components-image--three-images-arrangement
…nent The aspect ratio of 4:3 is enforced when the .uds-card-arrangement is wrapped around an image component. No changes to previous behavior
|
Per brand team, a 4:3 aspect ratio will be enforced with the styles when the card-arrangement is used. No changes to previous behavior. Webspark already crops and takes care of this so will only affect nonwebspark users |
| /** | ||
| * Array of card objects for rendering multiple cards | ||
| */ | ||
| cards: PropTypes.arrayOf( |
There was a problem hiding this comment.
Cards has same structure as BaseCard, might need to rearrange the file, but can we reuse the prop structure
cards: PropTypes.arrayOf(PropTypes.shape(BaseCard.propTypes))
| showBorders = true, | ||
| cardLink, | ||
| cards = [], | ||
| columns = "0", |
There was a problem hiding this comment.
I think we are moving towards responsive design, and we should not allow columns to be a property. Columns should be responsive and allow the card to adjust between the min and max widths provided by the brand team.
| tags, | ||
| showBorders = true, | ||
| cardLink, | ||
| cards = [], |
There was a problem hiding this comment.
Card feels like it is doing too much. Should we have a card-arrangement component that takes an array of 1 or more cards?
There was a problem hiding this comment.
I am ok with this temporarily if we have to keep it for WS2 purposes, but think it will cause issue down the line since it is a strange pattern
| cardLink, | ||
| cards = [], | ||
| columns = "0", | ||
| layout = "auto" |
There was a problem hiding this comment.
Does auto mean horizontal?
Instead of layout default to auto can we change layout prop to vertical = true or forceVertical = true?
There was a problem hiding this comment.
I see we already have horizontal prop. these props are getting confusing
4 participants
This adds the options for rendering multiple cards for the ranking cards and Card component in unity-react-core. Webspark will have to be updated to use this but the default/old behavior is still there and users will have to intentinally use the new prop to use the new render feature
Description
Checklist
Important Reminders
Links