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

[KOA-6229] Add viewbox to all JS icons and spinners #224

Merged
merged 3 commits into from
Aug 10, 2023
Merged

[KOA-6229] Add viewbox to all JS icons and spinners #224

merged 3 commits into from
Aug 10, 2023

Conversation

anambl
Copy link
Contributor

@anambl anambl commented Aug 10, 2023

In order for SVGs to scale, we need to add a viewBox attribute. Due to an issue with svgo library, the removeViewBox plugin which is enabled by default cannot be disabled. This plugin only runs if the viewBox width and height values are equal to the width and height attributes on the svg and so one solution is to remove the width and height attributes altogether. We are still using the style attribute to set the width and height so it should not have any impact visually.

Changes:

  • Remove width and height attributes from all JS icons
  • Add viewBox attribute to all LG icons (SM icons already have a viewBox)
  • Add viewBox and style to all JS spinners
  • Replace width and height attributes on SVG spinners with viewBox (this matches all SVG icons)

Remember to include the following changes:

@anambl anambl marked this pull request as ready for review August 10, 2023 09:15
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why this exists, but it is not used anywhere.

@anambl anambl merged commit 262809c into main Aug 10, 2023
1 check passed
@anambl anambl deleted the KOA-6229 branch August 10, 2023 13:48
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.

2 participants