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

[UI Framework] [K7] KuiTabs component #13280

Merged
merged 4 commits into from Aug 2, 2017

Conversation

Projects
None yet
2 participants
@cjcenizal
Contributor

cjcenizal commented Aug 2, 2017

image

@cjcenizal cjcenizal added the :Design label Aug 2, 2017

@cjcenizal cjcenizal requested a review from snide Aug 2, 2017

@cjcenizal

This comment has been minimized.

Show comment
Hide comment
@cjcenizal

cjcenizal Aug 2, 2017

Contributor

@snide Take a look at the hover transition and let me know if you think it's too much.

Contributor

cjcenizal commented Aug 2, 2017

@snide Take a look at the hover transition and let me know if you think it's too much.

@snide

That was fast! Gave you some code to style it up a bit. We'll want to make these responsive as well. I can probably do that with just CSS in a follow up PR.

Also, how awesome is it that by sticking to this strict variable usage, that stuff works with theming immediately!

@@ -0,0 +1,52 @@
.kuiTabs {

This comment has been minimized.

@snide

snide Aug 2, 2017

Contributor

A little too much on hover. But you've got the right idea by wanting some animation here. The below code simplifies your markup slightly and adds an animation on the selection, rather than the hover. I also went ahead and added our standard K7 patterns for accessibility (both hover underlines and focus backgrounds).

I did also change out the tab color to a color. Think it was a little too plain with the black.

.kuiTabs {
  display: flex;
  border-bottom: $kuiBorderThin;
}

.kuiTab {
  @include kuiFontSize;

  position: relative;
  cursor: pointer;
  padding: ($kuiSize - $kuiSizeXS) $kuiSize;
  color: $kuiColorDarkShade;
  background-color: $kuiColorEmptyShade;
  transition: all $kuiAnimSpeedNormal $kuiAnimSlightResistance;

  &:hover:not(.kuiTab-isSelected) {
    color: $kuiTextColor;
    text-decoration: underline;
  }

  &:focus {
    background-color: $kuiFocusBackgroundColor;
    text-decoration: underline;
  }

  &.kuiTab-isSelected {
    cursor: default;
    color: $kuiColorSecondary;

    &:after {
      position: absolute;
      bottom: -1px;
      left: 0;
      content: ' ';
      width: 100%;
      height: $kuiBorderWidthThick;
      background-color: $kuiColorSecondary;
      animation: kuiTab $kuiAnimSpeedFast $kuiAnimSlightResistance;
    }
  }
}

  .kuiTab__content {
    display: block;
    transition: transform $kuiAnimSpeedFast $kuiAnimSlightBounce;
    transform: translateY(0);
  }


@keyframes kuiTab {
  0% {
    transform: scaleX(0);
  }
  100% {
    transform: scaleX(1);
  }
}
@snide

snide Aug 2, 2017

Contributor

A little too much on hover. But you've got the right idea by wanting some animation here. The below code simplifies your markup slightly and adds an animation on the selection, rather than the hover. I also went ahead and added our standard K7 patterns for accessibility (both hover underlines and focus backgrounds).

I did also change out the tab color to a color. Think it was a little too plain with the black.

.kuiTabs {
  display: flex;
  border-bottom: $kuiBorderThin;
}

.kuiTab {
  @include kuiFontSize;

  position: relative;
  cursor: pointer;
  padding: ($kuiSize - $kuiSizeXS) $kuiSize;
  color: $kuiColorDarkShade;
  background-color: $kuiColorEmptyShade;
  transition: all $kuiAnimSpeedNormal $kuiAnimSlightResistance;

  &:hover:not(.kuiTab-isSelected) {
    color: $kuiTextColor;
    text-decoration: underline;
  }

  &:focus {
    background-color: $kuiFocusBackgroundColor;
    text-decoration: underline;
  }

  &.kuiTab-isSelected {
    cursor: default;
    color: $kuiColorSecondary;

    &:after {
      position: absolute;
      bottom: -1px;
      left: 0;
      content: ' ';
      width: 100%;
      height: $kuiBorderWidthThick;
      background-color: $kuiColorSecondary;
      animation: kuiTab $kuiAnimSpeedFast $kuiAnimSlightResistance;
    }
  }
}

  .kuiTab__content {
    display: block;
    transition: transform $kuiAnimSpeedFast $kuiAnimSlightBounce;
    transform: translateY(0);
  }


@keyframes kuiTab {
  0% {
    transform: scaleX(0);
  }
  100% {
    transform: scaleX(1);
  }
}

This comment has been minimized.

@snide

snide Aug 2, 2017

Contributor

Also, you'll notice I did a subtraction to get that 12px value again to pad out the tabs a bit. I guess we should make that $kuiSizeM that value, heh.

@snide

snide Aug 2, 2017

Contributor

Also, you'll notice I did a subtraction to get that 12px value again to pad out the tabs a bit. I guess we should make that $kuiSizeM that value, heh.

This comment has been minimized.

@cjcenizal

cjcenizal Aug 2, 2017

Contributor

Looks good!

@cjcenizal

cjcenizal Aug 2, 2017

Contributor

Looks good!

@cjcenizal cjcenizal referenced this pull request Aug 2, 2017

Closed

K7 KUI #12807

36 of 51 tasks complete
@cjcenizal

This comment has been minimized.

Show comment
Hide comment
@cjcenizal

cjcenizal Aug 2, 2017

Contributor

@snide Updated with your suggestion.

Contributor

cjcenizal commented Aug 2, 2017

@snide Updated with your suggestion.

@snide

snide approved these changes Aug 2, 2017

Looks good.

@cjcenizal cjcenizal merged commit bac19b5 into elastic:k7-ui-framework Aug 2, 2017

1 check passed

CLA Commit author has signed the CLA
Details

@cjcenizal cjcenizal deleted the cjcenizal:k7/tabs branch Aug 2, 2017

cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Aug 16, 2017

cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Aug 26, 2017

cjcenizal added a commit that referenced this pull request Sep 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment