patternfly-react icon indicating copy to clipboard operation
patternfly-react copied to clipboard

react-icons: Do not force square icons

Open ryelo opened this issue 6 years ago • 3 comments

After looking at these icons: Screen Shot 2019-04-01 at 1 19 53 PM

There is visual inconstancy when these are placed next to one another because the CloudIcons are wider than they are tall. This creates an empty space on the top and bottom of the icon.

For a more extreme example: Screen Shot 2019-04-01 at 1 18 49 PM

Since PF is consuming font-awsome icons (which don't put a constraint on fixed square icons), why are we forcing the icons to be square instead of having the same height? If you investigate any of the font awesome icons, you can see the only baseline they use is the height with a max-width

Screen Shot 2019-04-01 at 1 37 27 PM

If a user needs to place icons in a list and needs them to be the same width, then I propose to use something like font-awesome's fixed width icons as a prop on the icons.

I don't think it's fair to make the user write custom CSS with various height/width values in order to correct the visual inconsistency.

ryelo avatar Apr 01 '19 17:04 ryelo

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Jun 06 '19 14:06 stale[bot]

Need to review along with overall icon strategy - pinning open

dgutride avatar Jun 10 '19 17:06 dgutride

We noticed this inconsistency while updating the custom icons in core, and how react-icons scale down the icon height when the icon is wider than it is tall. An example of this is the FA warning-triangle icon. Here is the warning alert icon in core and react:

Screen Shot 2020-08-13 at 5 53 11 PM Screen Shot 2020-08-13 at 5 53 19 PM

Removing the inline width should resolve it, though that's likely breaking as it would impact the layout where an icon is used.

mcoker avatar Aug 13 '20 22:08 mcoker

Per discussions with @mcoker, we should also need to consider if React should use CSS tokens from core for the size values.

tlabaj avatar Oct 12 '22 18:10 tlabaj

Should consider as a possible optional feature but not change the default behavior of our icons.

mcarrano avatar Nov 01 '22 19:11 mcarrano