Skip to content

Conversation

devchris
Copy link
Contributor

@devchris devchris commented Oct 22, 2020

WHY are these changes introduced?

To keep add more functionality to the MediaCard similar to what regular Cards are already doing. So far, MediaCards do not accept ReactNodes as a title. If you do, the console log throws a bunch of warnings that you shouldn't nest stuff in paragraphs.

Also, when using a MediaCard, you are forced to set a primaryAction. This is not always wanted behavior. Maybe you just want to display something.

WHAT is this pull request doing?

It allows the MediaCard title to accept a ReactNode. In addition, it makes the primaryAction optional.

🎩 checklist

@devchris
Copy link
Contributor Author

@alex-page Could someone please review this pull request?

@alex-page
Copy link
Member

alex-page commented Oct 23, 2020

Love this @devchris 👏. Just for shared context are there any examples of reasons you needed this flexibility? I see you added the Badge to the test.

@alex-page alex-page requested a review from BPScott October 23, 2020 14:53
@alex-page alex-page self-assigned this Oct 23, 2020
Co-authored-by: Alex Page <alex@alexpage.com.au>
@devchris
Copy link
Contributor Author

devchris commented Oct 23, 2020

Love this @devchris 👏. Just for shared context are there any examples of reasons you needed this flexibility? I see you added the Badge to the test.

I was using it with a Stack in my app but used a Badge here for simplicity 👍

For the optional primaryAction, I used a custom wrapper around the card so when you click anywhere on the card, it will navigate you to a different page.

Co-authored-by: Alex Page <alex@alexpage.com.au>
Co-authored-by: Ben Scott <227292+BPScott@users.noreply.github.com>
@devchris
Copy link
Contributor Author

@alex-page can this be merged please? 😊

@alex-page alex-page merged commit 7cf6184 into Shopify:master Oct 25, 2020
@ghost
Copy link

ghost commented Oct 25, 2020

🎉 Thanks for your contribution to Polaris React!

sylvhama pushed a commit that referenced this pull request Mar 26, 2021
…ction (#3552)

Co-authored-by: Alex Page <alex@alexpage.com.au>
Co-authored-by: Ben Scott <227292+BPScott@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants