Skip to content

Conversation

@beefchimi
Copy link
Contributor

This PR safely moves color: $secondary-color into the svg block alongside fill: $primary-color.

Previously, the mixin was expected to be used like so:

.Component {
  color: blue;
  // more styles for the component
  @include recolor-icon(red, green);
}

Within the recolor-icon mixin, fill was scoped to to the svg selector... but color was outside of it. This meant that in the above example, my color: blue will get overridden with green, when really my intention is to have .Component use blue text, while making the primary color of the icon red and the secondary color of the icon green.

@beefchimi beefchimi added the Bug Something is broken and not working as intended in the system. label Oct 16, 2019
@beefchimi beefchimi requested review from dleroux and kaelig October 16, 2019 13:21
@beefchimi beefchimi self-assigned this Oct 16, 2019
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2019

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

No significant changes to src/**/*.tsx were detected.


This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

@danrosenthal danrosenthal removed their request for review October 16, 2019 14:07
@kaelig kaelig changed the title 🐛[SASS] Fix recolor-icon mixin 🐛[Sass] Fix recolor-icon mixin Oct 16, 2019
@kaelig
Copy link
Contributor

kaelig commented Oct 16, 2019

:shipit:

@beefchimi beefchimi merged commit ea9733d into master Oct 16, 2019
@beefchimi beefchimi deleted the recolor-svg branch October 16, 2019 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is broken and not working as intended in the system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants