Conversation
|
Preview: https://patternfly-pr-6203.surge.sh A11y report: https://patternfly-pr-6203-a11y.surge.sh |
There was a problem hiding this comment.
re: the dupe id's, looks like you just need something like this on the 3 tile examples with the PF svgs
tile--id="stacked-pf-logo-[default/selected/disabled]"
| @@ -1,17 +1,19 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
You can remove this - isn't needed if the SVG is inline in the page. Only needed if the SVG loads as its own external asset.
| </g> | ||
| </g> | ||
| </svg> No newline at end of file | ||
| <svg width="1em" height="1em" viewBox="0 0 40 40" fill="none" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
can remove this, too
| <svg width="1em" height="1em" viewBox="0 0 40 40" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
| <svg width="1em" height="1em" viewBox="0 0 40 40" fill="none"> |
|
Also in the other svg's we can remove a bunch of stuff: Only needs to be: Only needs to be: |
There was a problem hiding this comment.
A couple of nits, but LGTM! If it's easy, it would also be nice to figure out what's going on with the PF logo SVG, it's wider than it needs to be. All can be follow ups if you'd prefer!
| } | ||
|
|
||
| &:active, // for demo purposes - will remove | ||
| &:active, |
There was a problem hiding this comment.
seems like this was left in on accident - wdyt about removing it?
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| justify-content: start; |
There was a problem hiding this comment.
Just a nit, but you shouldn't need this
|
|
||
| flex-direction: column; | ||
| align-items: flex-start; | ||
| justify-content: initial; |
There was a problem hiding this comment.
This shouldn't be needed, either.
|
lgtm! thanks sarah! mentioned on the group review call that we should consider a follow up issue for making tiles a variant of the card component. |
|
🎉 This PR is included in version 6.0.0-alpha.55 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #6197
Note: A11y checks fail for duplicate ID, which is inside the inline SVG. @mcoker @mnolting @lucia do you have other ideas/a better way to include the SVG?