feat(card): add pf-m-display-lg and pf-m-rounded #5389
Conversation
|
PF4 preview: https://patternfly-react-pr-5389.surge.sh |
There was a problem hiding this comment.
Nice parametrization of the examples code @redallen!
Your change it of course totally fine codewise, I just wonder why PF wants to expose to its consumers that isRounded property. It causes a really subtle difference, and I feel that the PF designers should decide if the cards should have rounded corners or not and not let the consumers make decisions about it.
After all, the reason that we all use PF and not custom styling is in order that all RH products have the same Ui experience.
So, this PR is fine to go in but I really don't see the usecase for it.
|
@KKoukiou This thread has the context behind pf-m-rounded: patternfly/patternfly#3670 . I'd ask @mcarrano its purpose. |
|
You ask a valid question @KKoukiou . This originated from the web properties design team wanting to use PatternFly to present more marketing oriented content. @mmenestr do you know if we have plans to add this guidance to our design guidelines for Cards? I agree that we don't want designers to just arbitrarily pick styling. Products and web applications should stick with the squared corners. |
|
@mcarrano I don't believe we do. This is the first I've heard of this! Just to make sure I have the info right, is it just a modifier to give cards the capability of having rounded edges? And we want to recommend that this only be done for marketing purposes and that products keep using non-rounded cards? |
|
@mcoker Unfortunately since we've already released |
|
Can we also add something to both prop descriptions (isLarge and isCompact) for clarity? Like |
redallen
force-pushed
the
feat/large-card
branch
from
06fa679
to
4bbbf49
Compare
|
@kmcfaul I added that to the prop description. |
There was a problem hiding this comment.
I really like the modifier example! It's really neat to be able to play with all of the different modifiers. Can we add the keyboard interaction to the selectable example though? I currently can't select it by keyboard or screen reader the way I could with the previous selectable example. (I'm hoping that by adding the keyboard interaction, it'll work through screen reader as well.)
|
@jessiehuff I've removed the |
| ouiaId, | ||
| ouiaSafe = true, | ||
| ...props | ||
| }: CardProps) => { | ||
| const Component = component as any; | ||
| const ouiaProps = useOUIAProps(Card.displayName, ouiaId, ouiaSafe); | ||
| if (isCompact && isLarge) { | ||
| // eslint-disable-next-line no-console |
What: Closes #5334
Additional issues: I made an interactive modifiers example to showcase it