[UI Framework] [K7]: Buttons and loader components

[snide]

Still working through this one.

[cjcenizal]

Looks fantastic! Just had a few suggestions.

[cjcenizal]

We can remove this right?

[cjcenizal]

I think the physics of the shadow are slightly off... as the K nears its apex, more light should hit the ground below it, causing the shadow to shrink, fade, and blur:

What do you think? Here are the styles I used to get this effect:

.kuiLoading {
  position: relative;
  display: inline-block;

  &:before,
  &:after {
    position: absolute;
    content: "";
    width: 90%;
    left: 50%;
    transform: translateX(-50%);
    border-radius: 50%;
    opacity: 0.2;
    transform-origin: -50% -50%;
    z-index: 1;
  }

  &:before {
    box-shadow: 0 0 8px $kuiColorFullShade;
    animation: 1s kuiLoadingPulsateAndFade $kuiAnimSlightResistance infinite;
  }

  &:after {
    background-color: $kuiColorFullShade;
    animation: 1s kuiLoadingPulsate $kuiAnimSlightResistance infinite;
  }
}

.kuiLoading--medium {
  &:before,
  &:after {
    height: $kuiSizeXS * $kuiLoadingKibanaShadowScale;
    bottom: -$kuiSizeXS;
  }
  .kuiLoading__icon {
    z-index: 999;
    animation: 1s kuiLoadingBounceMedium $kuiAnimSlightResistance infinite;
  }
}

.kuiLoading--large {
  &:before,
  &:after {
    height: $kuiSizeS * $kuiLoadingKibanaShadowScale;
    bottom: -$kuiSizeS;
  }
  .kuiLoading__icon {
    animation: 1s kuiLoadingBounceLarge $kuiAnimSlightResistance infinite;
  }
}

.kuiLoading--xLarge {
  &:before,
  &:after {
    height: ($kuiSize - $kuiSizeXS) * $kuiLoadingKibanaShadowScale;
    bottom: -($kuiSize - $kuiSizeXS);
  }
  .kuiLoading__icon {
    animation: 1s kuiLoadingBounceXLarge $kuiAnimSlightResistance infinite;
  }
}

@keyframes kuiLoadingBounceMedium {
  50% {
    transform: translateY(-$kuiSizeS);
  }
}

@keyframes kuiLoadingBounceLarge {
  50% {
    transform: translateY(-($kuiSize - $kuiSizeXS));
  }
}

@keyframes kuiLoadingBounceXLarge {
  50% {
    transform: translateY(-$kuiSize);
  }
}

@keyframes kuiLoadingPulsateAndFade {
  0% {
    opacity: 0;
  }

  50% {
    transform: scale(0.5);
    opacity: 0.1;
  }

  100% {
    opacity: 0;
  }
}

@keyframes kuiLoadingPulsate {
  0% {
    opacity: 0.15;
  }

  50% {
    transform: scale(0.5);
    opacity: 0.05;
  }

  100% {
    opacity: 0.15;
  }
}

_index.scss:

$kuiLoadingKibanaShadowScale: 0.75;

@import 'loading';
@import 'loading_chart';
@import 'loading_spinner';

[snide]

Awesome improvement.

[cjcenizal]

Would you mind renaming this to loading_kibana?

[cjcenizal]

I think this modifier should be on the root component, to be consistent with how other modifiers work:

.kuiButton--reverse {
  .kuiButton__inner {
    // styles
  }
}

[cjcenizal]

Can we order the classes like this (per the guidelines):

.kuiButton {
}

  // Children, unnested and indented once.
  .kuiButton__icon {
  }

// Modifiers.
.kuiButton--fill {
  .kuiButton__icon {
  }
}

[snide]

Went with nesting here as we discussed.

[cjcenizal]

Is there a setting you can apply to your editor to ensure only a single empty line at the end of the file?

[cjcenizal]

I think the transition into and out of this focus state is cool and clearly communicates focus, but it gets weird when you click the button. It feels really uncomfortable to me because I've just pushed the button and it reacts to the push, but then looks like it got stuck. We don't need to address this in this PR, but I think later on we might need to look at an alternative to get around this behavior.

[snide]

I agree. I don't like it either. I'll work on something better in a different PR.

[snide]

Solved this by using an animation instead. Now it will pop back. Might still add an outline to it as well, but should be decent for now.

[cjcenizal]

This is better, but now when you click the button, the "reaction" is delayed until the mouseup event. This means if your click is a little bit slower, the reaction feels out of sync with your action.

I tried playing with different pseudoselectors (:active, :focus:not(:active)), but couldn't find a good solution. Personally, I think maybe we could do something different to draw the eye to a focused button, e.g. a pulse effect (not looping, just a single pulse).

[snide]

Yeah. It's something we can do in another PR. I agree there's something cooler we can do.

[cjcenizal]

Should we call this .kuiButton__content? Based on your suggestion in the Table PR: #13249 (comment)

[snide]

Yes! I brought this over, but this makes sense.

[snide]

@cjcenizal OK. Adjusted as requested. Rather than use that shadow size var, I just went ahead and used some $size -1 stuff where it was needed. It really should be extremely rare we do that kind of thing, and likely only in animations or things that require precision movement. I figure it's a decent tradeoff with readability. When we do that kind of thing (do pixel math) I'd like to enforce that it always needs to be commented. Even if it's just to say... "requires pixel math", so we know it's not someone cheating.

[cjcenizal]

Nice! Just a couple suggestions.

[cjcenizal]

Hmmm... maybe try rebasing again? Something went wrong to cause this file to show up in the diff. I can also help you clean up the commit history if you want to pair.

[cjcenizal]

This should be:

    const classes = classNames(
      'kuiButton',
      typeToClassNameMap[type],
      sizeToClassNameMap[size],
      className,
      {
        'kuiButton--fill': fill,
        'kuiButton--reverse': iconReverse,
      },
    );