Skip to content

USWDS - Icons: Remove style tags from indeterminate checkbox svgs #6162

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

Merged
merged 5 commits into from
Nov 6, 2024

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Oct 29, 2024

Summary

Removed style tags from indeterminate checkbox SVGs. These style tags were unnecessary and caused a conflict with Cypress automated testing.

Breaking change

This is not a breaking change.

Asset change

Users should update the checkbox-indeterminate.svg and checkbox-indeterminate-alt.svg files in their projects to prevent this issue.

Related issue

Closes #6011

Note

I have opened #6163 as a follow-up issue to run SVGO on all of our icons and include npm run fix:icons as part of our release process.

Related pull requests

Changelog PR

Preview link

Indeterminate checkbox story

Problem statement

The checkbox-indeterminate.svg and checkbox-indeterminate-alt.svg files contain unnecessary inline <style> tags that can make testing with Cypress difficult.

Solution

Running svgo on the checkbox-indeterminate SVG files removed the style tags and optimzed the files.

npx svgo ./packages/usa-checkbox/src/img/checkbox-indeterminate.svg
npx svgo ./packages/usa-checkbox/src/img/checkbox-indeterminate-alt.svg

Testing and review

  • Confirm that this update removes the inline <style> tags in the affected SVG code
  • Confirm that removing style tags will resolve the issue
  • Confirm no visual regressions with these icons
  • Confirm that there are no inline <style> tags in any other SVG files in the design system

amyleadem added 2 commits October 29, 2024 16:33
@amyleadem amyleadem marked this pull request as ready for review October 30, 2024 14:12
@amyleadem amyleadem requested review from mejiaj and mahoneycm October 30, 2024 15:52
Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

  • Confirm that this update removes the inline <style> tags in the affected SVG code
  • Confirm that removing style tags will resolve the issue (I'm basing this off of the reporters explanation. I wasn't sure of a good way to test locally with cypress without setting up a config and writing a script)
  • Confirm no visual regressions with these icons
  • Confirm that there are no inline <style> tags in any other SVG files in the design system

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No regressions found when testing, but we should remove the inline style rule that @mahoneycm mentioned. It doesn't match the other icons we provide.

<path class="st0" d="M2.9,35.9c0,1.1,0.4,2,1.2,2.8c0.8,0.8,1.7,1.2,2.8,1.2h7.9h42.3c1.1,0,2-0.4,2.8-1.2s1.2-1.7,1.2-2.8l0,0V28
c0-1.1-0.4-2-1.2-2.8S58.2,24,57.1,24H6.9c-1.1,0-2,0.4-2.8,1.2S2.9,26.9,2.9,28V35.9z"/>
</svg>
<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" viewBox="0 0 64 64"><path fill="#FFF" fill-rule="evenodd" d="M2.9 35.9c0 1.1.4 2 1.2 2.8s1.7 1.2 2.8 1.2h50.2c1.1 0 2-.4 2.8-1.2s1.2-1.7 1.2-2.8V28c0-1.1-.4-2-1.2-2.8S58.2 24 57.1 24H6.9c-1.1 0-2 .4-2.8 1.2S2.9 26.9 2.9 28z" clip-rule="evenodd"/></svg>
Copy link
Contributor Author

@amyleadem amyleadem Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced style="fill-rule:evenodd;clip-rule:evenodd;fill:#fff" with clip-rule="evenodd", fill-rule="evenodd", and fill: #fff in 0f68f96.

@amyleadem amyleadem requested a review from mejiaj November 1, 2024 16:53
@thisisdano thisisdano merged commit 3838f8e into develop Nov 6, 2024
5 checks passed
@thisisdano thisisdano deleted the al-remove-svg-style-tags branch November 6, 2024 22:14
RachelCorsino added a commit that referenced this pull request Nov 7, 2024
@thisisdano thisisdano mentioned this pull request Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USWDS - Bug: SVG images in 3.8.0+ release interfere with Cypress DOM snapshot style injection
4 participants