feat(Responsive): add component

[layershifter]

Why?

This component is the idea picked from my production project. Breakpoint uses syntax from Grid's device visibility, but has two important differences:

• it doesn't require Grid markup, so two close items can have a different visibility
• it doesn't render children when they doesn't visible

Warning

Breakpoint adds a listener to window that can cause a performance issue, however it will be solved by #1733.

---

P.S. Better ideas about the component's name are welcome.

[codecov-io]

Codecov Report

Merging #1872 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1872      +/-   ##
==========================================
+ Coverage   99.76%   99.76%   +<.01%     
==========================================
  Files         148      149       +1     
  Lines        2561     2584      +23     
==========================================
+ Hits         2555     2578      +23     
  Misses          6        6

Impacted Files	Coverage Δ
src/addons/Responsive/Responsive.js	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f39631...8474bba. Read the comment docs.

[layershifter]

Sinon should be updated to support value:

sandbox.stub().value()

[levithomason]

Where does this.point get defined?

_.invoke(this, point))

[layershifter]

It equals to this[point](), while point is an element of array that is splitted from only prop. The idea is to call breakpoint matchers (methods) until match.

[layershifter]

I will prefix breakpoints with is, it will be easier to read

[levithomason]

Let's prefix bools isVisible

[layershifter]

Also need merge with master

[layershifter]

@levithomason I've prefixed all methods that returns boolean with is. It will be easier to understand what it makes.

[levithomason]

Naming

Per the request for naming, I've renamed this component:

<Responsive breakpoints={} />

I think this makes sense as many devs should be familiar with Bootstrap's "responsive utilities", which are classes for the same purpose.

breakpoints

I'm not sure about this prop. It seems it would be most helpful to set this at the LESS/CSS level instead. If we did allow overriding a specific <Responsive /> element with different values, what about something like this instead?

<Responsive minWidth={400} />
<Responsive maxWidth={800} />

What is the use case for overriding the value of mobile, etc?

[layershifter]

Naming

Agree about Responsive.

breakpoints

I've add it because widths that SUI uses are variables and it can be possible, that someone override them in own theme. Thats why I think we allow to change used values.

[levithomason]

Oh, I see. I was mistaken in thinking the only='mobile' here was relying on a mobile className and a CSS media query for visibility. I see now that this is only mimicking the CSS visibility styles.

The only issue I see here is that users with custom CSS breakpoints in their theme would expect this component to respect them, but it won't. Imagine I set my theme to have a mobile breakpoint of 400px:

<Column only='mobile' />      // only <400px (uses my LESS/CSS value)
<Responsive only='mobile' />  // only <300px (uses defaultProps value)

I'm worried about adding complexity and configuration to the theming process. It is quite cumbersome already.

My next suggestion was going to be that we should add CSS support for these utilties, however, it looks like SUI core isn't fond of responsive visibility or responsive utilities. There is a design decision to not permit them. While I don't agree, chances of getting into CSS core are minimal.

I'm still not comfortable merging this while we have two components with only='device' which would render differently. We need some kind of way to ensure all components use the same values.

[layershifter]

You justly paid attention to these things, but unfortunately I see no other way to solve this problem.

I want to solve the problem with useless rendering, Grid renders all elements in contrast to this component. It's very usefull for navbars and other responsive things.

[levithomason]

What if we remove the only prop and go with min/max width props instead? Users can create the same values locally:

const mediaQueries = { tablet: { minWidth: 768, maxWidth: 992 }  }

<Responsive {...mediaQueries.tablet} />

I would be OK with this as it doesn't suggest to the user that there is any relation between Responsive's only prop and Grid's only prop.

[layershifter]

I will think about this, but <Responsive only='mobile' /> looks more intuitive.

[levithomason]

You're right, it is very intuitive. If it used the same CSS as the Grid it would be a no brainer. Since they will produce 2 different behaviors, I think it becomes unintuitive.

One hack I didn't want to mention was somehow creating a ui grid element and checking the styles. I would rather not go down that road, but it may get us to where we need to be.

[layershifter]

@levithomason I refactored it to minWidth and maxWidth and added breakpoints as static properties. Also, it now uses eventStack and RAF for resize handler.

[levithomason]

Cool, seems like a good middle ground 👍 Thanks much for the cooperative spirit.

[levithomason]

Released in semantic-ui-react@0.73.0