Conversation
elizabetdev
left a comment
There was a problem hiding this comment.
LGTM! I just add one small suggestion.
snide
left a comment
There was a problem hiding this comment.
Loaded locally, checked the docs and code.
| } @else if ($direction == 'x') { | ||
| mask-image: linear-gradient(to right, #{$gradient}); | ||
| } @else { | ||
| @warn "euiOverflowShadow() expects direction to be 'y' or 'x' but got '#{$direction}'"; |
| @include euiYScrollWithShadows; | ||
| } | ||
|
|
||
| .euiXScrollWithShadows { |
There was a problem hiding this comment.
I know it looks like we did this in the other one, but the rest of our utilities were written eui-. Wish these two weren't off pattern.
There was a problem hiding this comment.
We could also use a dual selector for now, and deprecate the other one. Obviously not a big deal, just something I noticed.
There was a problem hiding this comment.
Yeah so I also looked into the docs and the docs referenced this as .eui-yScrollWithShadows. I think at some point there was a bad merge that undid the change from .euiYScrollWithShadows to .eui-yScrollWithShadows. I then opted for the non-dash one, but for no particular reason but to not break that selector. I can setup a deprecation though instead.
snide
left a comment
There was a problem hiding this comment.
I'm sorry, I missed the rerequest for review on this one! LGTM.
f4ac48f to
5974565
Compare
Set `.euiYScroll…` for deprecation
5974565 to
010f631
Compare
Fixes #2215
This new mixin
euiXScrollWithShadowsand utility class.euiXScrollWithShadowscompliment the already existing mixineuiYScrollWithShadowsand utility class.euiYScrollWithShadows.Checklist
IE11and Firefox (IE doesn't support masks, and we're ok with no fallback)[ ] Props have proper autodocs[ ] Added or updated jest tests[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes