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

fix: fixing button issues #1252

Closed
wants to merge 1 commit into from
Closed

fix: fixing button issues #1252

wants to merge 1 commit into from

Conversation

rengare
Copy link
Contributor

@rengare rengare commented Jul 14, 2020

Related Issue

Realted to #1237

Description

The PR removes fd-reset from fd-icon since it broke button's example

Screenshots

Before:

Check the issue

After:

image
image

Please check whether the PR fulfills the following requirements

@rengare rengare added Bug Something isn't working 0.11.0 labels Jul 14, 2020
@rengare rengare added this to the Sprint 41 - Nessebar milestone Jul 14, 2020
@rengare rengare requested a review from droshev July 14, 2020 11:02
@rengare rengare self-assigned this Jul 14, 2020
@rengare rengare requested a review from a team July 14, 2020 11:02
@droshev droshev mentioned this pull request Jul 14, 2020
3 tasks
@netlify
Copy link

netlify bot commented Jul 14, 2020

Deploy preview for fundamental-styles ready!

Built with commit 431c3df

https://deploy-preview-1252--fundamental-styles.netlify.app

Copy link
Contributor

@jbadan jbadan left a comment

Choose a reason for hiding this comment

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

Icons should still have a reset - you can override whatever style from the reset is causing the problem with style: inherit

@salarenko
Copy link
Contributor

salarenko commented Jul 15, 2020

Icons should still have a reset - you can override whatever style from the reset is causing the problem with style: inherit

I don't think inherit can be successfully used in this flat structure. The things we can try:

  • Lowering selector specificity changing icon implementation from this:
$block: sap-icon;

[class*="#{$block}"] {
  @include fd-reset();
  @include fd-icon-base();

  display: inline-block;
}

.#{$block} {
  @each $key, $value in $fd-icons {
    &--#{$key} {
      @include fd-icon-glyph($key);
    }
  }
}

To this:

%icon-base {
  @include fd-reset();
  @include fd-icon-base();

  display: inline-block;
}

.#{$block} {
  @each $key, $value in $fd-icons {
    &--#{$key} {
      @extend %icon-base;
      @include fd-icon-glyph($key);
    }
  }
}

But this still won't resolve all of the problems (file order import and selector strength "draw").

@droshev droshev closed this Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants