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

Small (mostly style) fixes #1145

Merged
merged 4 commits into from
Sep 4, 2018
Merged

Small (mostly style) fixes #1145

merged 4 commits into from
Sep 4, 2018

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Aug 27, 2018

1. Button Fixes

a. Don’t let icon or spinner shrink

If whatever is containing a button forces the button to shrink, it will properly truncate the text. However, if the button also has an icon, the flex model will force the icon flex-item to shrink as well. This fixes that.

b. Allow empty buttons to truncate

c. Allow icon sizes to be passed to EuiButtonIcon (fixes #848)

Example sizes (in :focus state, to see full bounds of button)
image

2. Added focus state to anchor tags within EuiText

image

3. Fixed alignment and coloring of form control clear button

I noticed in Kibana, the (x) icon was always misaligned, but not in the EUI theme, so this fixes it in both instances.

It also darkens the background so it's a bit more AA compliant and also fixes the dark theme coloring.


  • Browser check.

@cchaos cchaos requested review from chandlerprall and snide August 27, 2018 17:25
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

changes LGTM!

&:focus {
text-decoration: underline;
outline: solid 3px transparentize($euiLinkColor, .9);
background-color: transparentize($euiLinkColor, .9);
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to the focus background color here?

$euiFocusBackgroundColor: tintOrShade($euiColorPrimary, 90%, 50%) !default;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I had just copied these from EuiLink

&:focus {
outline: solid 3px transparentize($color, .9);
background-color: transparentize($color, .9);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we make all 3 be the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. I'm guessing there was a good reason we did it with transparency in link.scss so it's likely ok to leave it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

cchaos added 3 commits September 4, 2018 11:11
- Don’t let icon or spinner shrink
- Allow empty buttons to truncate
- elastic#848 - Allow icon sizes to be passed to EuiButtonIcon
@cchaos cchaos merged commit 250df0f into elastic:master Sep 4, 2018
@cchaos cchaos deleted the small-fixes branch September 4, 2018 15:21
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.

Add a size prop to EuiButtonIcon
3 participants