FCT-808: Create CustomIcon component

[ByronDWall]

Summary

Based on our discussion about how LeadingIcon handles non-ui-kit icons in this issue, this PR introduces a new CustomIcon component that handles displaying non-ui-kit SVG's in a consistent manner.

Description

The CustomIcon is similar to the LeadingIcon, but is intended to display non-ui-kit icons. See the linked issue for more details.

[changeset-bot]

🦋 Changeset detected

Latest commit: b31a7a0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 96 packages

Name	Type
@commercetools-uikit/icons	Minor
@commercetools-uikit/calendar-utils	Minor
@commercetools-uikit/collapsible-panel	Minor
@commercetools-uikit/data-table-manager	Minor
@commercetools-uikit/data-table	Minor
@commercetools-uikit/field-label	Minor
@commercetools-uikit/link	Minor
@commercetools-uikit/notifications	Minor
@commercetools-uikit/pagination	Minor
@commercetools-uikit/primary-action-dropdown	Minor
@commercetools-uikit/tag	Minor
@commercetools-uikit/password-field	Minor
@commercetools-uikit/async-creatable-select-input	Minor
@commercetools-uikit/async-select-input	Minor
@commercetools-uikit/checkbox-input	Minor
@commercetools-uikit/creatable-select-input	Minor
@commercetools-uikit/date-input	Minor
@commercetools-uikit/date-range-input	Minor
@commercetools-uikit/date-time-input	Minor
@commercetools-uikit/input-utils	Minor
@commercetools-uikit/localized-money-input	Minor
@commercetools-uikit/localized-multiline-text-input	Minor
@commercetools-uikit/localized-rich-text-input	Minor
@commercetools-uikit/localized-text-input	Minor
@commercetools-uikit/money-input	Minor
@commercetools-uikit/multiline-text-input	Minor
@commercetools-uikit/radio-input	Minor
@commercetools-uikit/rich-text-input	Minor
@commercetools-uikit/rich-text-utils	Minor
@commercetools-uikit/search-text-input	Minor
@commercetools-uikit/select-input	Minor
@commercetools-uikit/select-utils	Minor
@commercetools-uikit/selectable-search-input	Minor
@commercetools-uikit/time-input	Minor
@commercetools-frontend/ui-kit	Minor
@commercetools-uikit/async-creatable-select-field	Minor
@commercetools-uikit/async-select-field	Minor
@commercetools-uikit/creatable-select-field	Minor
@commercetools-uikit/date-field	Minor
@commercetools-uikit/date-range-field	Minor
@commercetools-uikit/date-time-field	Minor
@commercetools-uikit/localized-multiline-text-field	Minor
@commercetools-uikit/localized-text-field	Minor
@commercetools-uikit/money-field	Minor
@commercetools-uikit/multiline-text-field	Minor
@commercetools-uikit/number-field	Minor
@commercetools-uikit/radio-field	Minor
@commercetools-uikit/search-select-field	Minor
@commercetools-uikit/select-field	Minor
@commercetools-uikit/text-field	Minor
@commercetools-uikit/time-field	Minor
@commercetools-uikit/fields	Minor
@commercetools-uikit/inputs	Minor
@commercetools-uikit/search-select-input	Minor
@commercetools-uikit/number-input	Minor
@commercetools-uikit/password-input	Minor
@commercetools-uikit/text-input	Minor
@commercetools-uikit/toggle-input	Minor
@commercetools-uikit/design-system	Minor
@commercetools-uikit/calendar-time-utils	Minor
@commercetools-uikit/hooks	Minor
@commercetools-uikit/i18n	Minor
@commercetools-uikit/localized-utils	Minor
@commercetools-uikit/utils	Minor
@commercetools-uikit/accessible-hidden	Minor
@commercetools-uikit/avatar	Minor
@commercetools-uikit/card	Minor
@commercetools-uikit/collapsible-motion	Minor
@commercetools-uikit/collapsible	Minor
@commercetools-uikit/constraints	Minor
@commercetools-uikit/field-errors	Minor
@commercetools-uikit/field-warnings	Minor
@commercetools-uikit/grid	Minor
@commercetools-uikit/label	Minor
@commercetools-uikit/loading-spinner	Minor
@commercetools-uikit/messages	Minor
@commercetools-uikit/progress-bar	Minor
@commercetools-uikit/stamp	Minor
@commercetools-uikit/text	Minor
@commercetools-uikit/tooltip	Minor
@commercetools-uikit/view-switcher	Minor
@commercetools-uikit/accessible-button	Minor
@commercetools-uikit/flat-button	Minor
@commercetools-uikit/icon-button	Minor
@commercetools-uikit/link-button	Minor
@commercetools-uikit/primary-button	Minor
@commercetools-uikit/secondary-button	Minor
@commercetools-uikit/secondary-icon-button	Minor
@commercetools-uikit/dropdown-menu	Minor
@commercetools-uikit/spacings-inline	Minor
@commercetools-uikit/spacings-inset-squish	Minor
@commercetools-uikit/spacings-inset	Minor
@commercetools-uikit/spacings-stack	Minor
@commercetools-uikit/buttons	Minor
@commercetools-uikit/spacings	Minor
visual-testing-app	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
ui-kit	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 14, 2024 0:32am

[CarlosCortizasCT]

I think it would be better for this icon to not have size attributes.
I believe the intention is that the size of the svg is controlled by the icon size so it grows or shrinks with it.

[ByronDWall]

This is meant to be an example of a non-standard "custom" svg that a consumer of the library would pass to this component.

To me "custom" means that we do not override the properties that the user specified. If the user wanted the svg to scale, all they would have to do is remove the width and height from the svg.

[ByronDWall]

I can remove the width/height here if you'd prefer that for the docs examples though

[CarlosCortizasCT]

Thanks for sharing your reasoning, Byron 👍

The way I see it, the main point of this component is to make sure the icons size is consistent across applications, not matter if they are from ui-kit or custom. I actually think we should update the description of the component to better explain about this.

If users provide a custom icon with a specific size, we won't get the expected behaviour because, as in the current state with the fixture svg, making the CustomIcon component bigger does not make the svg bigger, which I think that's what we want.

So yes, I would suggest to remove the size properties of the fixture SVG and also to update the description of the component (we can ask Filip for help).

[FilPob]

@ByronDWall thanks for tackling this! I found one issue with the border. If the border is turned on, the underlaying svg seems to cut through the border-radius.

[ByronDWall]

@ByronDWall thanks for tackling this! I found one issue with the border. If the border is turned on, the underlaying svg seems to cut through the border-radius.

@FilPob thanks for pointing that out, the issue should be fixed now

[FilPob]

@ByronDWall thanks, border looks good now. One more thing I noticed is that sometimes the custom svg doesn't scale if you change the size. It could be that this is just a storybook issue cause if you toggle the use stringified svg for icon prop on and off then suddenly it works fine.

[ByronDWall]

@ByronDWall thanks, border looks good now. One more thing I noticed is that sometimes the custom svg doesn't scale if you change the size. It could be that this is just a storybook issue cause if you toggle the use stringified svg for icon prop on and off then suddenly it works fine.

@FilPob I believe this is an issue with storybook, the stringified svg is using the scale InlineSvg sizing prop, which is used elsewhere without issue. Also, I don't believe this component will be dynamically sized in consuming apps the way it is in the storybook, so this should not be an issue.

[CarlosCortizasCT]

@ByronDWall thanks, border looks good now. One more thing I noticed is that sometimes the custom svg doesn't scale if you change the size. It could be that this is just a storybook issue cause if you toggle the use stringified svg for icon prop on and off then suddenly it works fine.

@FilPob I believe this is an issue with storybook, the stringified svg is using the scale InlineSvg sizing prop, which is used elsewhere without issue. Also, I don't believe this component will be dynamically sized in consuming apps the way it is in the storybook, so this should not be an issue.

@ByronDWall I think Filip is not talking about an inline svg issue but an icon one. If you use the icon and change its size, we can see we still have some extra space between the icon and the border, but this works if you change from icon to inline svg and back to icon (to be honest, I don't know why).

I believe the solution is to change the flex styles from the wrapping div and just use a display: inline-block style:

[ByronDWall]

@ByronDWall I think Filip is not talking about an inline svg issue but an icon one. If you use the icon and change its size, we can see we still have some extra space between the icon and the border, but this works if you change from icon to inline svg and back to icon (to be honest, I don't know why).

This was an interesting issue, because I could not reproduce it in firefox. Was able to in chrome. Fixed

[CarlosCortizasCT]

@FilPob I think UI wise this is ready.

Could you please take another look when you have the time? 🙏

[FilPob]

Looks perfect now! thanks @CarlosCortizasCT @ByronDWall :)

[CarlosCortizasCT]

Thanks for your contribution, Byron 👍