Responsive refactors and addition of helpers #909

cchaos · 2018-06-06T21:48:58Z

Deprecation warning for $breakpoints and any current responsive mixins

Refactored responsive breakpoints

Reasoning: The naming between the breakpoints map and mixins were misleading and the mixin screenSmallMediumLarge() was too much of a hack. The refactor ensure the breakpoints you specify are the breakpoints you get.

$breakpoints map is now $euiBreakpoints and starts at 0-1200px+:

$euiBreakpoints: (
  "xs": 0px,
  "s":  575px,
  "m":  768px,
  "l":  992px,
  "xl": 1200px
) !default;

The map has also been moved out of the mixins/_responsive.scss to a corresponding variables/_responsive.scss file.

There is now one euiBreakpoint() mixin that accepts an array of named break points match those in the $euiBreakpoints map.

Example usage:

@include euiBreakpoint('xs','s') {
  display: none;
}

will result in:

@media only screen and (max-width: 574px) {
  display: none;
}
@media only screen and (max-width: 767px) and (min-width: 575px) {
  display: none;
}

@include euiBreakpoint('xs','s') is also a direct replacement for @include screenXSmall(). Be mindful that the breakpoint name to px values have changed in the new map.

EUI components should all now be updated to use the new map and mixin.

Added responsive utilities

Classes

The responsive classes utilize the map and mixin to create .euiShowFor--[size] and .euiHideFor--[size] classes.

Example compiled

@media only screen and (max-width: 574px) {
   .euiHideFor--xs {
     display: none !important;
   }
}

Components

EuiShowFor and EuiHideFor components take a sizes prop as an array of named sizing which correlate to the same SASS map. They wrap the child element in a <span> with the corresponding utility class applied.

cjcenizal · 2018-06-06T22:44:51Z

CHANGELOG.md

@@ -5,6 +5,12 @@
 - Added `resize` prop to `EuiTextArea` that defaults to ‘vertical’ (only height) ([#894](https://github.com/elastic/eui/pull/894))
 - Added multiple style-only adjustments to `EuiFormControlLayout` buttons/icons ([#894](https://github.com/elastic/eui/pull/894))
 - Shifted `readOnly` inputs to not have left padding unless it has an icon ([#894](https://github.com/elastic/eui/pull/894))
+- Added responsive helpers in the form of `EuiShowFor` and `EuiHideFrom` components and corresponding CSS classes. ([#909](https://github.com/elastic/eui/pull/909))


Just a small suggestion: EuiShowWithin and EuiHideWithin might reduce the cognitive overhead a bit, because I had to double-check and compare "for" vs. "from" mentally, and then I realized they actually mean the same thing. And maybe it's just me, but I usually think of a breakpoint being within a min and a max.

I hear ya with the naming, though I think "within" sounds odd. How about both of them just using "For"? This is usually how other frameworks handle it.

Sounds good to me!

cjcenizal · 2018-06-06T23:06:12Z

src/components/responsive/hide_from.test.js

+import { EuiHideFrom } from './hide_from';
+
+describe('EuiHideFrom', () => {
+  test('is rendered', () => {


Can we also loop through and test all of the sizes like we do in https://github.com/elastic/eui/blob/master/src/components/button/button.test.js#L91 ?

- Changed `EuiHideFrom` to `EuiHideFor` - Looping through sizes in tests - Fixed typo in `EuiShowFor` classname

snide

This is great. Had some small questions, but address them how you think best.

snide · 2018-06-07T20:55:39Z

src/global_styling/utility/_utility.scss

+}
+
+@each $size in $euiBreakpointKeys {
+  .euiHideFor--#{$size} {


As utilities should we use the same prefixing as our other css utilities? eui-. Know you're using these in the component themselves, so I could see why we'd name them this way too, but I'd probably err to uniformity against the CSS stuff since that's something you tend to memorize? Oh it's a utility, i know it starts with...etc.

As long as you're ok using the utility class naming style in the components, I'm fine with it too and can change it.

Yeah. I think it's fine to use them there.

snide · 2018-06-07T21:02:59Z

src/global_styling/mixins/_responsive.scss

+// A sem-complicated mixin for breakpoints, that takes any number of
+// named breakpoints that exists in $euiBreakpoints.
+
+@mixin euiBreakpoint($sizes...) {


This is a really nice function and so much better. Bravo.

snide · 2018-06-07T22:27:12Z

src-docs/src/views/responsive/responsive_example.js

+          {sizes.euiBreakpointKeys.map(function (size, index) {
+            return renderSizes(size, index);
+          })}
+        </EuiCodeBlock>


I would love to see this using our "blue" wrapper boxes to show people the actual sizing. It might help visualize the sizing more quickly. Even possibly listing out the more common scenarios (tablet and below is... size={xs,s}...etc.

Don't think it needs to be for this PR, and I'm happy to set it up after you merge since you did all the leg work.

Ah, I can do that.

@snide Here's the problem I'm running into with your suggestion. The screen and especially that side of the page never gets large enough to show the full width of anything above size 's'. Unless I'm just not understanding what you mean.

Didn't think about this. One thing we could do is make some page outlines that were fixed positon, or just take these bars and fix them to the bottom of the page (maybe make them thinner).

Always something we can do a bit later. Don't want to hold up your PR on this.

Yeah, ok, I'll push this idea off for later.

cchaos added 4 commits June 5, 2018 20:08

Added basic responsive utility components and classes

5ca395e

Displaying $breakpoints in docs

9af775a

Converting to new $euiBreakpoints

06a48cc

Updated component usages of now deprecated SASS

13737da

cchaos added style only feature request labels Jun 6, 2018

cchaos requested a review from snide June 6, 2018 21:49

cchaos added 2 commits June 6, 2018 17:50

0px not rem

857a6df

Changelog and added deprecation warning to old mixins

17ec96a

cjcenizal reviewed Jun 6, 2018

View reviewed changes

cchaos and others added 2 commits June 7, 2018 12:24

Merge branch 'master' into responsive-helpers

a0fb1e4

Addressing PR feedback

2d8c321

- Changed `EuiHideFrom` to `EuiHideFor` - Looping through sizes in tests - Fixed typo in `EuiShowFor` classname

snide approved these changes Jun 7, 2018

View reviewed changes

cchaos and others added 4 commits June 8, 2018 11:02

Adjusting map explanation a bit

198cee5

Renaming classes using the utility css naming schemed eui-

cf86a40

Merge branch 'master' into responsive-helpers

091d76e

Merge branch 'master' into responsive-helpers

505503d

cchaos merged commit 2f7d21f into elastic:master Jun 8, 2018

cchaos mentioned this pull request Jun 8, 2018

[META] Remove deprecated items by 2.0.0 #910

Closed

2 tasks

cchaos deleted the responsive-helpers branch June 8, 2018 16:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Responsive refactors and addition of helpers #909

Responsive refactors and addition of helpers #909

cchaos commented Jun 6, 2018 •

edited

Loading

cjcenizal Jun 6, 2018

cchaos Jun 7, 2018

cjcenizal Jun 7, 2018

cjcenizal Jun 6, 2018

snide left a comment

snide Jun 7, 2018

cchaos Jun 8, 2018

snide Jun 8, 2018

snide Jun 7, 2018

snide Jun 7, 2018

cchaos Jun 8, 2018

cchaos Jun 8, 2018

snide Jun 8, 2018

cchaos Jun 8, 2018

Responsive refactors and addition of helpers #909

Responsive refactors and addition of helpers #909

Conversation

cchaos commented Jun 6, 2018 • edited Loading

Refactored responsive breakpoints

Example usage:

Added responsive utilities

Classes

Example compiled

Components

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos commented Jun 6, 2018 •

edited

Loading