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

refactor(core-icons): removes division from icons #1196

Merged
merged 5 commits into from
Apr 8, 2022

Conversation

breardon2011
Copy link
Contributor

removes division from icons so that icons injested by momentum-react-v2 are compatible with dart-sass where division is deprecated

relevant cantina issue is spark 309091

Description

Basically take the division to a constant in each case. 2em/16 = .125em, etc. There is no actual change to the style, just removing division in a supported way.

Related Issue

Deprecation Warning: Sass Slashes #1176. http://github.com/momentum-design/momentum-ui/issues/1176#issue-1166023599

Motivation and Context

This allows Momentum React V2 to be upgraded to Dart Sass. Momentum React V2 imports the icons from the Momentum UI node module. The deprecated division throws an error while building the proposed changes to Momentum React V2.

How Has This Been Tested?

This is a simple change, since both are constants. Validated that the change is valid css, built with no errors.

Types of changes

  • [ X] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code style update (formatting, local variables)
  • [ X] Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Checklist:

  • [ X] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • [ X] I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • [ X] All new and existing tests passed.

removes division from icons so that icons injested by momentum-react-v2 are compatible with dart-sass where division is deprecated

relevant cantina  issue is spark 309091
@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #1196 (28116e9) into master (08604b7) will decrease coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1196      +/-   ##
==========================================
- Coverage   94.28%   94.17%   -0.11%     
==========================================
  Files         132      132              
  Lines       17368    17368              
  Branches     2582     2576       -6     
==========================================
- Hits        16375    16357      -18     
- Misses        993     1011      +18     
Impacted Files Coverage Δ
...mponents/src/components/draggable/DraggableItem.ts 93.75% <0.00%> (-6.25%) ⬇️
web-components/src/components/tabs/Tab.ts 94.60% <0.00%> (-5.40%) ⬇️
web-components/src/mixins/RovingTabIndexMixin.ts 93.63% <0.00%> (-3.64%) ⬇️
web-components/src/components/theme/Theme.ts 87.13% <0.00%> (-1.76%) ⬇️
web-components/src/components/menu/MenuItem.ts 97.50% <0.00%> (-1.25%) ⬇️
web-components/src/components/tooltip/Tooltip.ts 87.61% <0.00%> (-0.48%) ⬇️
web-components/src/mixins/ResizeMixin.ts 90.14% <0.00%> (+1.40%) ⬆️
...eb-components/src/components/help-text/HelpText.ts 100.00% <0.00%> (+1.66%) ⬆️
web-components/src/components/icon/Icon.ts 81.54% <0.00%> (+1.78%) ⬆️
web-components/src/components/label/Label.ts 100.00% <0.00%> (+2.22%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08604b7...28116e9. Read the comment docs.

@robstax robstax merged commit b2cd24a into momentum-design:master Apr 8, 2022
@breardon2011 breardon2011 deleted the remove-division branch April 11, 2022 20:18
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.

3 participants