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

[docs][material-ui][Card] CardMedia description is wrong #42859

Closed
arigoldx opened this issue Jul 5, 2024 · 17 comments · Fixed by #43067
Closed

[docs][material-ui][Card] CardMedia description is wrong #42859

arigoldx opened this issue Jul 5, 2024 · 17 comments · Fixed by #43067
Assignees
Labels
bug 🐛 Something doesn't work component: card This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@arigoldx
Copy link

arigoldx commented Jul 5, 2024

Related page

https://mui.com/material-ui/react-card/

Kind of issue

Unclear explanations

Issue description

The text for Card Media states:

Card Media: an optional container for displaying background images and gradient layers behind the Card Content.

but I cannot figure out how to make a background image using CardMedia. The image displays above the CardContent. CardMedia is not meant to be used as a background for the Card and it's description in the docs should be updated.

Screenshot 2024-07-10 at 15 04 10

Context

Simply put, I'd like to add a background image for the card. Something like this (which I found on the web after searching for "login screen with background image"):

image

Search keywords: card, media, background image

@arigoldx arigoldx added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: docs-feedback Feedback from documentation page labels Jul 5, 2024
@arigoldx
Copy link
Author

arigoldx commented Jul 5, 2024

Fwiw, I found a solution to adding a background image on a Card.

Again, this presumes that the "right" / "mui best practices" way to code a login page (as per the image above) with a background image is using a Card. Seems like something called "CardMedia" would be.

First you use require to get the image:

const heroImage = require('../../assets/images/hero-images/cool-image.jpg');

then you use the sx property on Card using backticks and the url css function:

<Card sx={{backgroundImage: `url(${heroImage})` }}>

I'm admittedly still learning the best way to get images when using webpack (which, I've learned, is bundled with create-react-app). Maybe it's an import? Maybe it's a require? Would love articles to read on that if you have 🙇.

@ZeeshanTamboli
Copy link
Member

You can check the demo examples for adding images with the Card Media component. Regarding the text about gradient layers behind the Card Content:

and gradient layers behind the Card Content

I agree it's confusing since the image is above the card content. This was added in #40346. Maybe @anle9650 can explain it better since he added it. I'll mark it as a bug for now.

@ZeeshanTamboli ZeeshanTamboli changed the title Help text on Card component is wrong/confusing regarding setting background images. [docs][material-ui][Card] Help text on Card component is wrong/confusing regarding setting background images. Jul 6, 2024
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work docs Improvements or additions to the documentation component: card This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: docs-feedback Feedback from documentation page labels Jul 6, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title [docs][material-ui][Card] Help text on Card component is wrong/confusing regarding setting background images. [docs][material-ui][Card] Help text on Card component is wrong/confusing regarding Card Media component Jul 6, 2024
@arigoldx
Copy link
Author

arigoldx commented Jul 6, 2024

You can check the demo examples for adding images with the Card Media component.

I think I already did but I couldn't find any examples of background images. In the examples, the images are never behind the content.

Tho maybe I missed something? 🤔

@ZeeshanTamboli
Copy link
Member

First, seeing your login page image above, I'm curious why you're using the Card component for your login page unless it's a separate container.

What you're looking for is similar to the Card Cover component from Joy UI, which isn't available in Material UI. I also saw now that the description text for Card Media was copied from the Joy UI Card Cover: https://mui.com/joy-ui/react-card/#introduction.

So, this is definitely a bug in the Material UI Card Media description text. For your use case of having card content above a background image, could you create a new issue so we can track it as a new feature?

@ZeeshanTamboli ZeeshanTamboli added the good first issue Great for first contributions. Enable to learn the contribution process. label Jul 7, 2024
@mi-na-bot
Copy link

What is the solution here? Removing the offending text from the docs suggesting Card Media has the same function as Joy UI Card Cover?

@aarongarciah aarongarciah self-assigned this Jul 8, 2024
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Jul 8, 2024

What is the solution here? Removing the offending text from the docs suggesting Card Media has the same function as Joy UI Card Cover?

No, Material UI's Card Media component is different from Joy UI's Card Cover component. The problem is that the description of Card Media is the same as the Card Cover in the documentation.

@arigoldx
Copy link
Author

arigoldx commented Jul 8, 2024

For your use case of having card content above a background image, could you create a new issue so we can track it as a new feature?

Of course & thanks for the updates. Just so I'm clear, you'd like me to create a new issue that will request adding the Joy UI's Card Cover component to Material UI?

@arigoldx
Copy link
Author

arigoldx commented Jul 8, 2024

In addition, how would that work? Aren't Joy UI and Material UI two totally different things? If that's the case, we couldn't add Joy UI features to Material UI, correct?

I'm admittedly new to all of this: Material Design (Google), MUI, Material-UI, Joy UI.

From my understanding -- and from the description of the core libraries:

Material UI
An open-source React component library that implements Google's Material Design.

Joy UI
An open-source React component library that implements MUI's own in-house design principles.

That would imply that "card background images" is quite simply not part of Material Design, unless MUI's Material UI components aren't complete or aren't a 1-1 match with Material Design? I'm not sure what component in Material Design would work -- maybe Backdrop? But that's in Version 2 -- I don't see anything similar in Version 3, tho I'm a bit 😖 by all of this. Maybe "background image" just isn't something that's part of Material Design?

edit
After reading through some Material Design documentation on imagery, I'm beginning to feel that the use case I'm looking for is quite simply not part of Material Design -- none of the examples on that page show a form surrounded by a background. Furthermore, that's for version two -- I can't find a similar description on the use of images for version three 🤔.

@ZeeshanTamboli
Copy link
Member

In addition, how would that work? Aren't Joy UI and Material UI two totally different things? If that's the case, we couldn't add Joy UI features to Material UI, correct?

I'm admittedly new to all of this: Material Design (Google), MUI, Material-UI, Joy UI.

An open-source React component library that implements Google's Material Design.

Joy UI
An open-source React component library that implements MUI's own in-house design principles.

@arigoldx Joy UI's Card Cover component is different. I meant you could create a new issue for a similar Card Cover component in Material UI.

Yes, Material UI and Joy UI are different libraries. Material UI implements Google's Material Design, but it's not an exact match anymore. So, we can add a Card Cover component. As you said, "card background images" aren't part of Material Design, which might be why it wasn't included initially. We don't strictly follow Material Design now. @DiegoAndai can clarify this better.

That would imply that "card background images" is quite simply not part of Material Design, unless MUI's Material UI components aren't complete or aren't a 1-1 match with Material Design? I'm not sure what component in Material Design would work -- maybe Backdrop? But that's in Version 2 -- I don't see anything similar in Version 3, tho I'm a bit 😖 by all of this. Maybe "background image" just isn't something that's part of Material Design?

Why are you using the Card component for this login page? I asked the same in this issue comment. Maybe you want to do this without any component usage?

@arigoldx
Copy link
Author

As I admitted earlier, I'm rather new to all of this (Material Design, MUI, etc). Maybe that's why I find the distinctions between "MUI" and "Joy UI" and "Material UI" and "Material Design" a little bit tricky to understand.

If Material UI is different than Material Design and Joy UI different than Material Design then why both? Maybe there's a "history" page that explains the evolution 🍿. That is, I would imagine that Material UI (MUI?) would strictly follow Material Design and that the non-Material Design work would all be contained in Joy UI.

(Pardon all the questions but I quite simply find this all fascinating. Thanks for all your work!)

To answer the question about not using a Card, as I mentioned earlier I wasn't sure that using a Card was even the correct thing to do.

Thanks for your example code, tho I'm pretty set on creating a component so that the elements are reusable. At this point, maybe it's because of fear -- our legacy codebase as over 30,000 lines of scss. Ergo, I'd really like to have the style and code as closely coupled as possible. Essentially: theme overrides > styled > sx, as I read about a little while back.

To be honest, maybe this points to the fact that we shouldn't be using a gigantic image as our background 🤔. That's why I don't feel all that qualified to create a new issue for a similar Card Cover component in Material UI. My instinct is that maybe Material Design doesn't have a Card Cover component for a Very Good Reason ™️.

@aarongarciah aarongarciah changed the title [docs][material-ui][Card] Help text on Card component is wrong/confusing regarding Card Media component [docs][material-ui][Card] CardMedia description is wrong Jul 10, 2024
@aarongarciah aarongarciah removed their assignment Jul 10, 2024
@aarongarciah
Copy link
Member

Thanks for the feedback, @arigoldx; we really appreciate it. We are aware of the confusion around the different projects and branding in general. We're working on improving it.

Would you like to open a PR to fix the description for the CardMedia component?

Screenshot 2024-07-10 at 15 04 10

@arigoldx
Copy link
Author

arigoldx commented Jul 11, 2024

'got a lil busy today but I'll do it pronto!

@aarongarciah aarongarciah self-assigned this Jul 11, 2024
@arigoldx
Copy link
Author

Same question -- tho maybe it's moot since @bdzb has taken over the PR? Happy to sit back and watch :) 🍿

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Jul 15, 2024

@bdzb Thanks for the pull request. Reviewed the PR.

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Jul 16, 2024

@bdzb account isn't available now so the related PR also got deleted. So, this issue is open for contribution.

@arigoldx
Copy link
Author

Aaaahh, that explains why I couldn't see the PR 😅.

I'll read the docs on one's first pull request and hop to it.

@arigoldx
Copy link
Author

Ooof, got stuck on

pnpm install

& saw this in the logs

.../.pnpm/[email protected]/node_modules/nx postinstall$ node ./bin/post-install
│ /Users/arigold/src/material-ui/node_modules/.pnpm/[email protected]/node_modules/nx/src/native/native-bindings.js:244
│ throw loadError
│ ^
│ Error: Cannot find module '@nx/nx-darwin-x64'

Googled for a sec but nothing stood out. I'll 👀 more in a bit, but first some job-work 🧑‍🔧.

Heh, maybe I can open a second PR adding help to the getting started doc :)

shahzaibdev1 added a commit to shahzaibdev1/material-ui that referenced this issue Jul 25, 2024
This is to update the issue mui#42859. [docs][material-ui][Card] CardMedia description is wrong 

This is my first contribution to open-source projects, please inform me if something's wrong by my side. I want to contribute to MUI as it's my fav project.

Signed-off-by: shahzaibdev1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: card This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
5 participants
@arigoldx @aarongarciah @ZeeshanTamboli @mi-na-bot and others