Skip to content

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

Merged
thisisdano merged 5 commits into
developfrom
al-remove-svg-style-tags
Nov 6, 2024
Merged

USWDS - Icons: Remove style tags from indeterminate checkbox svgs#6162
thisisdano merged 5 commits into
developfrom
al-remove-svg-style-tags

Conversation

@amyleadem

@amyleadem amyleadem commented Oct 29, 2024

Copy link
Copy Markdown
Contributor

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 mahoneycm and mejiaj October 30, 2024 15:52

@mahoneycm mahoneycm left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread packages/usa-checkbox/src/img/checkbox-indeterminate-alt.svg Outdated

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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> No newline at end of file

@amyleadem amyleadem Nov 1, 2024

Copy link
Copy Markdown
Contributor Author

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
@thisisdano thisisdano deleted the al-remove-svg-style-tags branch November 6, 2024 22:14
CTGM-Bixal 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