fix(card): update clickable to use button and anchor#6949
fix(card): update clickable to use button and anchor#6949thatblindgeye merged 7 commits intopatternfly:v6from
Conversation
|
Preview: https://patternfly-pr-6949.surge.sh A11y report: https://patternfly-pr-6949-a11y.surge.sh |
There was a problem hiding this comment.
Will try linking the content of this PR to React locally shortly, just wanted to leave a couple quick comments. In addition to the file comment below, it looks like the focus styling is a bit odd.
When focusing the actionable (button) card, the corners have that faint "being overlapped" effect we've seen elsewhere. In light theme:
Looks worse in dark theme:
The actionable (link) cards don't seem to have any focus styling, though. Lastly looks like the "Actionable secondary style" example would need the update to use buttons or anchors.
| <a class="{{pfv}}card__clickable-action{{#if card--actionable--IsDisabled}} pf-m-disabled{{/if}}" {{#if card--actionable--IsDisabled}}tabindex="-1"{{/if}} href="{{ternary card--actionable--href card--actionable--href '#'}}"></a> | ||
| {{else}} | ||
| <button {{#if card--actionable--IsDisabled}}disabled{{/if}} type="button" class="{{pfv}}card__clickable-action"></button> |
There was a problem hiding this comment.
So (the?) one downside to having these placed wherever then absolutely positioned to fill the entire card for that "fully clickable card" functionality is consumers will probably need to pass appropriate aria labeling.
For our examples we could probably just use aria-labelledby and link to the title in each card since each of our examples has a title. In React we may need to add a couple new props then do a check if any of them are passed for an actionable card, and throw a warning if not.
|
Just a question - on https://patternfly-pr-6949.surge.sh/components/card/#actionable-button the current (clicked) card changes border color on hover, whereas the other selected variants do not. I'm not sure which is more correct; I think we did choose not to change the selected hover color but maybe it's better to. |
|
Nice catch @srambach! Updated 🐸👍 |
|
🎉 This PR is included in version 6.0.0-alpha.210 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #6948
Ran visual regressions and only the new/renamed examples failed (lack of a reference image to compare against)
Changes:
:is()so they're in a single selector instead of multiple (should be no visual change from that).#{$card}__clickable-action) in the:is()groups where it's needed, just a couple of themTo test this in a local react dev environment:
yarn build-patternfly@patternfly/patternflyat./distin the core repo@patternfly/patternflydir in the react repo where your dev server is runningmv node_modules/\@patternfly/patternfly node_modules/\@patternfly/patternfly.baknode_modules/@patternfly/patternflyin your react repoln -s ../patternfly/dist node_modules/\@patternfly/patternfly. You can also specify the full paths to those dirs if that's more clear, likeln -s /path/to/the/core-repo/dist /path/to/the/patternfly-react-repo/node_modules/\@patternfly/patternflyYou may need to restart the react dev server for these changes to take effect? If you want to make modifications to the core CSS, you can just edit files in the core repo (currently checked out to this PR's branch), and when you're done, just run
yarn build-patternfly, which will publish the changes to the ./dist dir. When that's done, the react dev server should auto-reload with the core changes.