patternfly
patternfly copied to clipboard
feat(icon): adds icon element
Draft - I have some questions including,
- Do we want to include the status colors
- Should we show that the spinner can go in the content, or only show it in the progress element? Do we want the progress element to work like this?
- Does the adjustment for SVGs work correctly; and is this related to a similar adjustment in patternfly-react? This talks about why this adjustment: https://blog.prototypr.io/align-svg-icons-to-text-and-say-goodbye-to-font-icons-d44b3d7b26b4
direct link to feature: https://patternfly-pr-5014.surge.sh/components/icon @mcoker @mcarrano @mmenestr @mceledonia
fixes #4665
Preview: https://patternfly-pr-5014.surge.sh
A11y report: https://patternfly-pr-5014-a11y.surge.sh
Do we want to include the status colors
I don't see why not!
Should we show that the spinner can go in the content, or only show it in the progress element? Do we want the progress element to work like this?
Not sure I understand what you're asking here - what do you mean by "in the content", and progress element to work like what?
Should we show that the spinner can go in the content, or only show it in the progress element? Do we want the progress element to work like this?Not sure I understand what you're asking here - what do you mean by "in the content", and progress element to work like what?
The way I did this was to make an element inside the main one that is "content" and another that is "progress" and by setting a modifier, the progress is shown and the content is hidden. But, the user could also just manually put a spinner into the content element and change it themselves using JS. They'd need to do this to swap, say, an error icon to a success one, so it's another way of doing it. I don't know if we want to show that, or just show using the progress element - or if we even want that progress element within this component or not.
I think it's useful to show the progress element here, since there are times where you might have a progress that is then replaced/leads to a success or error icon!
I agree it could be confusing, but IMO, it would be good to keep "wrapper" or "container" out of the component name if we can.
Here's an example of the same thing - https://build.washingtonpost.com/components/icon (https://github.com/washingtonpost/wpds-ui-kit/tree/main/ui/icon). We would just need to make it clear that a user needs to supply their own icon (as they would see in the examples) and link to the icons pages like you mentioned.
I updated the color to use currentcolor and apply it only to __content, and also moved the status modifiers into just the __content rather than __content and __progress since they are not needed.
:tada: This PR is included in version 4.213.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket: