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

changes when updating to 2.x in Material-UI #1391

Closed
eps1lon opened this issue Mar 1, 2021 · 3 comments
Closed

changes when updating to 2.x in Material-UI #1391

eps1lon opened this issue Mar 1, 2021 · 3 comments
Labels

Comments

@eps1lon
Copy link

eps1lon commented Mar 1, 2021

Describe the bug

  • plugins needed to be re-ordered to remove empty <g /> resulting from removeUselessStrokeAndFill (specifically removeUselessStrokeAndFill has to come before removeEmptyContainers
    Edit: Setting multipass to true does not require re-ordering. Did the default change?
  • Some paths have the following diff:
    -0-1.41l-2.83-2.83a.9959.9959 0 00-1.41
    +0-1.41l-2.83-2.83a.9959.9959 0 0 0-1.41
    This diff has no visible changes (search for accessalarm (with rounded selected) when using 1.3.2 vs 2.1.0)

To Reproduce
Update svgo to 2.1 and resolve documented breaking changes

Expected behavior
Unclear whether this is intended or not.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • SVGO Version: 2.1.0 (was 1.3.2)
  • NodeJs Version 12.20.0 (and 10.24.0
  • OS: Ubuntu 20.04.2 LTS

Additional context
mui/material-ui#25122

@TrySound
Copy link
Member

TrySound commented Mar 1, 2021

Some paths have the following diff. This diff has no visible changes

Not a bug. See #1353

plugins needed to be re-ordered to remove empty resulting from removeUselessStrokeAndFill (specifically removeUselessStrokeAndFill has to come before removeEmptyContainers

Might be related to series of fixes which prevents some optimisations in favour of correctness

Please tell if you'll notice any visual regressions.

@eps1lon
Copy link
Author

eps1lon commented Mar 2, 2021

Automated diffing did not spot any visual changes. If you don't consider the required multipass: true as a regression, feel free to close the issue.

@TrySound
Copy link
Member

TrySound commented Mar 2, 2021

Thanks! Hope soon multipass will not be necessary.

@TrySound TrySound closed this as completed Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants