Add banner prop to EuiFlyoutBody

[andreadelrio]

Summary

In order to build the drilldowns feature, we'd like have a borderless callout at the top of a flyout. For that reason this PR:

• Adds a prop called banner to EuiFlyOutBody which takes a node
• Adds a prop called borderless to EuiCallOut which removes the usual border from callouts

Checklist

• Check against all themes for compatibility in both light and dark modes
• Checked in mobile
• Checked in IE11 and Firefox
• Props have proper autodocs
• Added documentation examples
• Added or updated jest tests
  - [ ] Checked for breaking changes and labeled appropriately
  - [ ] Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[cchaos]

Hmm, part of this PR worries me a bit. Having two different styles of callouts (bordered and not) will cause some major inconsistencies in our applications. Mainly I'm thinking because devs won't necessarily know which one they should choose and pick entirely on aesthetics (which really is the only reason there's a difference).

Looking at the flyout version, it does look nice and I can think of several places that would benefit from this design. Which brings up another thing to consider. Should those other places get updated to reflect this new design (again for consistency)?

So my main question is, does the need for the callout to extend to the sides of the flyout (which is just an aesthetic choice at the moment) outweigh the possibilities of inconsistent callout types throughout the applications?

---

There's also another approach you could take that would ease the concern of inconsistent callouts, in which you apply a custom className to the provided banner, which in this case would be an EuiCallout and only when that banner className is used in conjunction with .euiCallout do you then remove the border. This would keep stand alone callout's always the same but only in this very specific instance would it remove the border.

---

If we do move forward with this, something to consider is that the overflow mask in the flyout body is causing the top portion of the callout to fade out as it approaches the header border.

[andreadelrio]

@cchaos Thanks for bringing up all these points. I hadn't fully considered the risk of inconsistencies in the use of callouts and I can understand the concern. To answer your question, I think the need for a "full width" callout in a flyout does not warrant the risk of inconsistent callout use. Having said that though I do think we should allow for a "full width" banner in flyouts, it just doesn't have to mean be add a borderless prop to callout.

The flyout with a regular callout as banner looks like this:

I don't think this looks too bad which makes me wonder if we'd need to remove the border at all. We could always leave it up to the consumer to remove the border in a callout when using it in a flyout? Is that bad practice?

[cchaos]

I don't think this looks too bad which makes me wonder if we'd need to remove the border at all.

I'll agree that it doesn't look too "bad", but obviously the non-bordered looks a bit better. I'll leave it up to you to decide if you'd want to keep the border in the flyout or not. However, I'd just not add it as a direct option of EuiCallout, but a custom override with classes.

We could always leave it up to the consumer to remove the border in a callout when using it in a flyout? Is that bad practice?

By leave it up to the consumer, do you mean letting them do the overrides or using a prop on EuiCallout? Consumers can override anything they want really, we just don't usually add docs around this specifically. However, adding this override into EUI would be as simple as:

• Wrapping the banner element in a div with a custom class like:

<div className="euiFlyout__banner">{banner}</div>`. 

• In the css, watch for .euiCallOut as being a child of .euiFlyout__banner and then remove the banner if so:

.euiFlyout__banner .euiCallOut {
  border: none;
}

[andreadelrio]

@cchaos Agreed about not adding borderless prop to callouts anymore. Yeah I know consumers can override anything, I guess I was wondering if it'll be weird for them to pass a "regular" callout as banner and see it doesn't have a border. But I guess that's not a big deal and I could always make a note in the docs that callouts get their border removed in flyouts.

---

If we do move forward with this, something to consider is that the overflow mask in the flyout body is causing the top portion of the callout to fade out as it approaches the header border.

About this, I also noticed it and wasn't sure how to handle it. Would we push the banner down a bit,

or remove mask-image?

I like how it looks without mask-image better but don't know how strict we are when it comes to allowing for this type of modifications. Or is there another way to handle it?

[cchaos]

I would consider creating an alternate mask-image that just masks the bottom portion and use that.

[andreadelrio]

I would consider creating an alternate mask-image that just masks the bottom portion and use that.

@cchaos My plan for this would be to modify euiOverflowShadow to also take a parameter called type which would default to both, but could also take top and bottom. We'd use it with bottom for this case. Let me know if this sounds like a good idea so I can proceed.

[andreadelrio]

@cchaos I made the changes we discussed. I also updated the docs to reflect the changes on euiOverflowShadow. I did that for vertical scrolling, not sure if I should do it for horizontal scrolling too as I don't want that portion of the docs to get repetitive

Updated flyout with banner and bottom shadows:

[cchaos]

@andreadelrio and I talked about not needing the granularity of extra mixins and utility classes that the new params to euiOverflowShadow provides. But she'll throw a link directly to this mixin into the documentation at the very least.

[cchaos]

May want to adjust this comment now that there's no such thing as a borderless callout.

[cchaos]

Add a comment in here why you're altering a different component

[cchaos]

Remember that BEM naming suggests that modifiers should use a -- and you might want to say it hasBanner instead otherwise it sounds like it's trying to target the banner itself.

This class also shouldn't be the one used to target the callout or else any callouts within the rest of children will also get the border removed. So instead, your element would be:

// If banner exists (= true), then spit out the wrapper plus the banner element
{banner &&
  <div class="euiFlyoutBody__banner">
    {banner}
  </div>
}

[cchaos]

Nice! 🎉

[cchaos]

If you want to further reduce some repetition here. You could create two top variables of:

$gradientStart:
  transparentize(red, .9) 0%,
  transparentize(red, 0) $hideHeight;
$gradientEnd: 
  transparentize(red, 0) calc(100% - #{$hideHeight}),
  transparentize(red, .9) 100%;

And then use those in the if statement:

  @if ($side == 'both') {
      $gradient: $gradientStart, $gradientEnd;
    } @else if ($side == 'start') {
      $gradient: $gradientStart;
    } @else {
      $gradient: $gradientEnd;
    }

[cchaos]

All the changes look good! I would also add a new docs example for the banner prop and a use-case for it.

[andreadelrio]

@cchaos I added the example with the banner. I also removed references to onSwitchChange and
isSwitchChecked from the other examples as we are not using them, as far I can tell.

[cchaos]

🎉 Awesome! Thanks for also cleaning up those unused states and adding a snippet.