Semantic-Org / Semantic-UI

Semantic is a UI component framework based around useful principles from natural language.
http://www.semantic-ui.com
MIT License
51.09k stars 4.95k forks source link

[Icon] Margin should be consistent inside a button #7032

Open samuel-knutson opened 3 years ago

samuel-knutson commented 3 years ago

Steps

Render a button with an "arrow right" icon and text, like this:

<button class="ui button">
  <i class="icon arrow right"></i>Next
</button>

Expected Result

All icons have a consistent margin when placed inside a button.

Actual Result

Any "right" icon has the margin flipped, which results in the icon being pushed up right against the text:

image

Version

2.4.2

Testcase

https://jsfiddle.net/ynd1kspb/

samuel-knutson commented 3 years ago

Here is the relevant code:

.ui.button:not(.icon) > .icon:not(.button):not(.dropdown) {
  margin: @iconMargin;
}
.ui.button:not(.icon) > .right.icon:not(.button):not(.dropdown) {
  margin: @rightIconMargin;
}

I tracked it as far back as this commit. (Load the diff for button.less and look at lines 507-509.) Not sure what happened prior to this big commit.

.ui.button > .icon {
  opacity: @iconOpacity;
  margin: @iconMargin;
  transition: @iconTransition;
  vertical-align: @iconVerticalAlign;
}
.ui.button > .right.icon {
  margin: @rightIconMargin;
}

According to the documentation, it is valid to have an icon and content in a Button:

image

My best guess is that this code was put in place for scenarios like this one, where the right arrow icon is placed to the right of the text content:

image

It seems to me that semantic ui is getting confused by the right and left classes. This code implies that we should use right and left to tell semantic ui how to pad an icon within a button. But that conflicts with some icons that have right and left in the name. Moreover, this use of left and right is not documented.

I think this margin: @rightIconMargin; code should be removed. Am I missing something?

samuel-knutson commented 3 years ago

Workaround

For now, I am simply using flipped arrow left instead of arrow right, to avoid the problematic right class.