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

Beta badges on panels #888

Merged
merged 5 commits into from
Jun 1, 2018
Merged

Beta badges on panels #888

merged 5 commits into from
Jun 1, 2018

Conversation

snide
Copy link
Contributor

@snide snide commented May 29, 2018

Replaces #884

Feature request of @nreese.

  • Moved the beta badge sass from EuiCard to a more generic mixin, then imported into cards.
  • Added badges to EuiPanel

image

@snide snide mentioned this pull request May 29, 2018
@snide
Copy link
Contributor Author

snide commented May 29, 2018

@cchaos Sorry, had a git screwup. Addressed feedback, fixed my copy/pasta and rewrote the mixin to be more specific. Will now also size appropriately against the different panel paddings.

@snide snide requested a review from cchaos May 29, 2018 23:40
@snide snide changed the title beta badges on panels Beta badges on panels May 30, 2018
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.

Looks better. I just had one comment about the width stuff.

// Specific
@each $modifier, $amount in $euiPanelPaddingModifiers {
.euiPanel.euiPanel--#{$modifier} {
padding: $amount;

// Overwrite @hasBetaBadge max-width depending upon padding
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just make all the width stuff it's own mixin? I can possibly see instances where we may not even want it to have a min-width, so it might be nice to leave it as an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm gonna leave this as is for the moment. It's the only usage so making another mixin feels like overkill (for now). If we start using it more we probably should, but for now the @each was there anyway, so it's pretty simple.

@snide snide merged commit c78d60b into elastic:master Jun 1, 2018
@snide snide deleted the feature/pbadge branch June 1, 2018 21:31
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.

2 participants