-
Notifications
You must be signed in to change notification settings - Fork 366
chore(card): add ability to hide radio button in example #11832
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
base: main
Are you sure you want to change the base?
chore(card): add ability to hide radio button in example #11832
Conversation
Preview: https://patternfly-react-pr-11832.surge.sh A11y report: https://patternfly-react-pr-11832-a11y.surge.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could also add some verbiage to both selectable examples to link to the respective "cards as tiles" example. @edonehoo wdyt? Basically something along the lines of "For selectable cards that hide the input element, see our [multi selectable tiles example](url here)
" (and then similar for the single selectable example).
Also @jenny-s51 would that ^ be helpful for shining more light on hiding those inputs? If so do you think we would still want to have the "single selectable" example have a way to toggle hiding the input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could also add some verbiage to both selectable examples to link to the respective "cards as tiles" example. @edonehoo wdyt? Basically something along the lines of "For selectable cards that hide the input element, see our [multi selectable tiles example](url here)" (and then similar for the single selectable example).
Also @jenny-s51 would that ^ be helpful for shining more light on hiding those inputs? If so do you think we would still want to have the "single selectable" example have a way to toggle hiding the input?
Good call @thatblindgeye! Linking to "Cards as Tiles" will effectively direct users to that pattern for hidden inputs from the selectable card examples.
I do think we should retain the toggle in the "single selectable" example as well. For users exploring "selectable cards" directly for this functionality (as we did initially), that immediate toggle is valuable since "Cards as Tiles" might not be the first place they'd look for selectable behavior.
So by toggling the radio buttons off, those examples essentially become "cards as tiles" too? Or is there anything else fundamentally different between the 2 (tile seems visually shorter still)? It feels a little redundant to have the toggle in the selectable example, but also have the tiles examples (unless there's a code difference). Unless we feel strongly about keeping Cards as tiles, we could:
thoughts? If we don't want to go that far, then I would +1 to Eric 'For more guidance on selectable cards with hidden input, see our cards as tiles examples.' |
We may have added the "cards as tiles" examples as a followup to deprecating tiles so that consumers had a way to mimic their behavior/look. Right now the only real different is probably the layout inside the cards for those "as tiles" examples? They're showing how to use an icon inside the CardHeader to visually be similar to some Tile examples. The only code difference looks to be that "cards as tiles" doesn't have Personally I think I'd be in favor of what you outlined @edonehoo, but just to clarify:
|
@thatblindgeye Ah yes, of course the selectable example includes multi-select 😆 consequences of me multi tasking too much I think you're right about the tile example being there to help with the deprecation. I'm second guessing my suggestion now, since the tile examples have icons, and icons aren't displayed in any other card examples. I would want to be sure that people know that's a clear option that's available For the sake of not holding this PR up more, maybe we just keep things as they are for now, and handle consolidation (if wanted) in a follow up issue? So, backtracking to your original rec, for now, we could just add the additional docs line to the selectable example? 'For more guidance on selectable cards with hidden input, see our cards as tiles examples.' |
Closes #11820