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

feat(icons): remove base64 transformation #1258

Merged
merged 9 commits into from
Dec 21, 2022
Merged

feat(icons): remove base64 transformation #1258

merged 9 commits into from
Dec 21, 2022

Conversation

mohamedMok
Copy link
Contributor

@mohamedMok mohamedMok commented Nov 14, 2022

I have read the contributing guidelines

  • Yes
  • No

Does this PR introduce a breaking change?

  • Yes
  • No

Describe the changes

This PR removes the postcss-base64 plugin from our packages, for the following reasons:

  • We want to make the use of our cssdevtools package optional
  • The postcss-base64 plugin has been out of date for several months causing security warnings
  • The use of base64 is not necessarily appropriate for inserting SVGs into CSS

Other information

Helpful links:

@mohamedMok mohamedMok changed the base branch from master to next November 14, 2022 11:55
@mohamedMok mohamedMok changed the title Feat inline icons feat(icons): remove base64 transformation Nov 14, 2022
@mohamedMok
Copy link
Contributor Author

@mohamedMok mohamedMok added the @next Anything related to the next major version label Nov 14, 2022
@mohamedMok mohamedMok self-assigned this Nov 14, 2022
@mohamedMok mohamedMok linked an issue Nov 14, 2022 that may be closed by this pull request
@tiloyi tiloyi added the scope:tools Anything related to tooling (css-dev-tools) label Dec 19, 2022
@tiloyi tiloyi added this to the Road to @mozaic-ds v2 milestone Dec 19, 2022

@return "data:image/svg+xml," + $svg;
}

/* stylelint-disable max-line-length */
@function inline-icons($icon, $fill) {
Copy link
Contributor

@tiloyi tiloyi Dec 20, 2022

Choose a reason for hiding this comment

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

Great job @mohamedMok ! 🔥 🔥 🔥 🔥

Do you think we could make the code of this function a little less verbose, by making only one return at the end of the function?

Something like :

@function inline-icons($icon, $fill) {
  $svg: '';

  @if $icon == "cross-16" {
    $svg: '<svg xmlns="http://www.w3.org/2000/svg" height="1rem" width="1rem" fill="#{$fill}" viewBox="0 0 16 16"><path d="M9.41 8l3.3-3.29a1 1 0 10-1.42-1.42L8 6.59l-3.29-3.3a1 1 0 00-1.42 1.42L6.59 8l-3.3 3.29a1 1 0 000 1.42 1 1 0 001.42 0L8 9.41l3.29 3.3a1 1 0 001.42 0 1 1 0 000-1.42z"/></svg>';
  }
  
  // ... 
  
  @return svg-encode($svg);
}

@tiloyi tiloyi linked an issue Dec 21, 2022 that may be closed by this pull request
3 tasks
@tiloyi tiloyi merged commit 0b3d47b into next Dec 21, 2022
@tiloyi tiloyi deleted the feat-inline-icons branch December 21, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@next Anything related to the next major version scope:tools Anything related to tooling (css-dev-tools)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tools] - the postcss-base64 seems outdated Remove base64 config from css-dev-tools
2 participants