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

Use mask-image for overflow shadows #2088

Merged
merged 6 commits into from
Jul 8, 2019

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 3, 2019

I know this is like the hundredth iteration of this thing, but it came to my attention that the overflow shadows can be hideous if you temporarily change the background color of the containing element:

@flash1293 Had a great idea to use mask-image instead of actual drop shadows. The only lack of support is in IE, but since this is really just a flourish anyway, 🤷‍♀ meh.

Example from EuiSelectable:

Dark mode

Modals, flyouts, selectable, CSS utility

This new overflow pattern has been updated where the original was used and still needs those extra elements so it was really just a matter of updating the shadow mixin. However, there were also duplicative styles being applied in most of those instances for adding the scrollbar and overflow properties, so I added a new SASS mixin for that as well: euiOverflowYScroll() and a CSS utility class .euiOverflowYScroll and documented both.


Also, fixed some padding on modals

Before, there was always a lot of padding between the body and the footer of the modal because it didn't take into account if both were present. This just reduces the top and bottom padding of the body by default, but adds more bottom padding if the body is the last element. If there is no body, it will reduce the top padding of the modal footer so it doesn't get doubled up from the header.

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • This was checked in dark mode
  • [ ] Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • [ ] This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • [ ] This was checked against keyboard-only and screenreader scenarios
  • [ ] This required updates to Framer X components

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Spot checked changed components locally. Looks like I'd expect.

CL entry still needed

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Small naming suggestion on your mixin. Otherwise looks good. Clever solution.

@@ -54,6 +54,14 @@
}
}

// The full overflow shadow plus scrollbars
@mixin euiOverflowYScroll {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe euiYScrollWithShadows would be a little more descriptive?

/**
* Overflow scrolling
*/
.euiOverflowYScroll {
Copy link
Contributor

Choose a reason for hiding this comment

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

And then might want to change it here if you change it.

@cchaos cchaos merged commit a61339b into elastic:master Jul 8, 2019
@cchaos cchaos deleted the overflow-fixes-again branch July 8, 2019 22:38
@snide snide mentioned this pull request Jul 12, 2019
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.

3 participants