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

Bolder, smaller badges with onclick events and custom colors #381

Merged
merged 10 commits into from
Feb 13, 2018

Conversation

snide
Copy link
Contributor

@snide snide commented Feb 9, 2018

This changes the visuals of badges to be a little bolder with a smaller footprint.

image

image

TODO:

  • Make prop switch between anchor, span or button tags.
  • Add iconOnClick prop for onclicks on the icon itself (closing for tags / pills...etc).
  • Allow custom hex to be passed as color.
  • Document how to use EuiFlexGroup to prevent smooshed second line wrapping in tables.

@uboness
Copy link
Contributor

uboness commented Feb 13, 2018

Sorry to bring this up here... But why is this component named "badge".. The more common term for this is a "pill". A badge is usually a... well... a badge you attach/place on other components (like the little number that is places on the email icon indicating how many unread emails you have in your mailbox.... or a "fork me" badge on an open source web page)

@snide
Copy link
Contributor Author

snide commented Feb 13, 2018

@uboness I try to stick to convention when I can and usually look at the "Big Two" when considering naming. While similar "Pills" normally describe input level or navigational elements (A "pill bar" for example). Both Bootstrap and Foundation use the badge terminology.

Our usage is definitely in the "badge" vein.

Pills used for navigation terminology

@snide
Copy link
Contributor Author

snide commented Feb 13, 2018

I'll admit the naming is murky out in the wild across all these systems. I think the terms are fairly synonymous in most cases.

@snide
Copy link
Contributor Author

snide commented Feb 13, 2018

Hilariously, "pill" looks sometimes to be used to describe the border-radius. Circular borders are pills, and squared ones are badges.

Welcome to frontend?

In the world of Bootstrap, you can even have Badge-Pills! Which goes to show you that it must have been a heated topic 😄

image

@snide snide requested review from nreese and cchaos February 13, 2018 01:09
@snide snide changed the title Bolder, smaller badges Bolder, smaller badges with onclick events and custom colors Feb 13, 2018
@uboness
Copy link
Contributor

uboness commented Feb 13, 2018

naming is and will always be hard.. the way I look at it - search google images for these terms and get a feel of what's more common the not:

badge ui - https://images.google.com/?q=badge%20ui
pill ui - https://images.google.com/?q=pill%20ui
tag ui - https://images.google.com/?q=tag%20ui

from the above, as I expected, a badge more commonly serves as an attachment to another component or to the page. Interestingly enough though, pill is not really a thing, and tag seems to be the one things that most commonly represent what we're building here... so maybe the right term is simply tag?

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks for making these improvements.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

LGTM, just a small typo in the docs.

}],
text: (
<p>
Badges can have onClick events applied to the badge itself or the icon within the tab.
Copy link
Contributor

@cchaos cchaos Feb 13, 2018

Choose a reason for hiding this comment

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

"...icon within the badge."

};

EuiBadge.propTypes = {
children: PropTypes.node,
className: PropTypes.string,

/**
* Accepts any string from our icon library
Copy link
Contributor

Choose a reason for hiding this comment

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

Clever, I didn't realize that these comments were then translated into the Notes section of the props documentation.

@snide snide merged commit 30e079f into elastic:master Feb 13, 2018
@snide snide deleted the style/badges branch February 13, 2018 17:05
@snide
Copy link
Contributor Author

snide commented Feb 13, 2018

@uboness Merge, gonna keep named how they are mostly because they're already out in the wild in use in Cloud / Kibana and causes us less headache. We can always switch it around later, but people haven't had too much trouble finding them.

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.

4 participants