Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Svg.tsx, IconContainer.tsx] The top-level <svg> attribute "fill" is erased which affects visual representation #2684

Closed
siarheiyelin opened this issue Dec 10, 2024 · 2 comments
Labels
bug Something isn't working
Milestone

Comments

@siarheiyelin
Copy link
Collaborator

Description

The top-level attribute "fill" is erased when SVG is rendered using Svg.tsx/IconContainer.tsx components.

Example of SVG which has this issue:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 54 45" width="24" height="24" fill="none">
	<rect x="1" y="7.224" width="52" height="36" rx="3.438" stroke="currentColor" stroke-width="2"/>
	<rect x="37" y="14.224" width="9" height="22" rx="2.219" stroke="currentColor" stroke-linecap="round" stroke-dasharray="4 5"/>
	<rect x="7" y="14.224" width="24" height="22" rx="2.219" stroke="currentColor" stroke-linecap="round" stroke-dasharray="4 5"/>
	<path d="M5.438 2.996c0-.674.545-1.22 1.219-1.22h2.219c.673 0 1.219.546 1.219 1.22v4.178H5.437V2.996z" stroke="currentColor" stroke-width="2"/>
	<path d="M43.904 2.996c0-.674.546-1.22 1.22-1.22h2.219c.673 0 1.219.546 1.219 1.22v4.178h-4.658V2.996z" stroke="currentColor" stroke-width="2"/>
</svg>

Root cause

Svg.tsx passes fill={undefined} to native <svg> element if the "fillColor" property is not provided. As a result "fill" attribute is erased from *.svg

Actual result

Property "fill" is removed from top level <svg> element. As of now, we have to apply next workaround:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 54 45" width="24" height="24">
  <g fill="none">
    <rect x="1" y="7.224" width="52" height="36" rx="3.438" stroke="currentColor" stroke-width="2"/>
    <rect x="37" y="14.224" width="9" height="22" rx="2.219" stroke="currentColor" stroke-linecap="round" stroke-dasharray="4 5"/>
    <rect x="7" y="14.224" width="24" height="22" rx="2.219" stroke="currentColor" stroke-linecap="round" stroke-dasharray="4 5"/>
    <path d="M5.438 2.996c0-.674.545-1.22 1.219-1.22h2.219c.673 0 1.219.546 1.219 1.22v4.178H5.437V2.996z" stroke="currentColor" stroke-width="2"/>
    <path d="M43.904 2.996c0-.674.546-1.22 1.22-1.22h2.219c.673 0 1.219.546 1.219 1.22v4.178h-4.658V2.996z" stroke="currentColor" stroke-width="2"/>
  </g>
</svg>

Expected result

Property "fill" is not removed from top level <svg> element.

Environment

  • UUI version: 5.10.2
@siarheiyelin siarheiyelin added the bug Something isn't working label Dec 10, 2024
@AlekseyManetov AlekseyManetov moved this from Backlog to Ready to release in UUI Project Board Dec 10, 2024
@NatalliaAlieva
Copy link
Collaborator

Released in 5.12.0 ver.

@github-project-automation github-project-automation bot moved this from Ready to release to Closed in UUI Project Board Dec 17, 2024
@AlekseyManetov
Copy link
Collaborator

AlekseyManetov commented Dec 17, 2024

We had to revert this fix in version 5.12.1 because it turned out that many users relied on the previous behavior where the fill attribute is cleared, rather than manually handling this for icons exported from Figma. This use case seems to be more common than using icons with a built-in fill. Therefore, we will consider this behavior as the standard for our components for now.

In the future, we might explore alternative solutions that would not introduce such a significant breaking change. For example, we could consider enabling this fix based on a specific setting. However, currently, we do not see a strong demand for cases where an icon must retain its custom color. That said, this can still be achieved using the following workarounds:

  1. Wrap the icon to prevent the fill from being applied by our SVG component:
import { ReactComponent as MyIcon } from '@epam/assets/icons/common/myIcon.svg';
const iconWithColor = () => <MyIcon />;

export default function Component() {
    return (
        <Button icon={ iconWithColor } />
    );
}
  1. Move the fill attribute from the <svg> element to a nested element:
<svg fill='red'> .... content  
=> 
<svg> <g fill='red'>... content</g> </svg>

AlekseyManetov added a commit that referenced this issue Dec 17, 2024
…from 5.12.0 version. Because it turned out that many users relied on the previous behavior where the fill attribute was cleared by default. If you need to render icon with built-in fill, please look at this issue comment - #2684 (comment)
@NatalliaAlieva NatalliaAlieva added this to the 5.12.1 milestone Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Closed
Development

No branches or pull requests

3 participants