Semantic-ui-react: Responsive: add `fireOnMount` prop and utils

[ryardley]

This may be an issue for the Grid component API but it may be something that should probably be better solved in the context of the Responsive add on. Hopefully I am just misguided and there is a simple way to accomplish what I am trying to do that requires no changes to the library.

Currently for me to get responsive column text alignment within a Grid.Column component I need to write my own bespoke function copying the functionality of the Responsive add on in order to select the textAlign value of the column.

Shouldn't there be some kind of utility function for this sort of thing?

The one I made I happened to call Responsive.value. My code looks a little like this:

class ResponsiveGridExample extends Component {
  this.onResizeHandler = () => this.forceUpdate();
  render() {
    return (
      <Responsive as={Grid} onUpdate={this.onResizeHandler}>
        <Grid.Column 
          mobile={16} 
          computer={4} 
          textAlign={Responsive.value({mobile: 'center', tablet: 'left'})} >
          {/* content */}
        </Grid.Column>
        {/* other columns */}
      </Responsive>
    );
  }
}

I am not necessarily suggesting this as a syntax this is just my initial idea of how to solve this.

The code for the function is a little hairy at around 40 lines without using lodash or ramda and is the kind of thing I would expect the library to handle for me instead of having to implement it myself especially as the logic is already used within the Responsive component.

In this particular instance perhaps extending the Grid API to accomodate responsive text alignment makes sense but I am sure there are other instances where having some kind of standard utility would be appreciated.

I also appreciate this brings up questions about how to handle responsive props other than size as so far the convention has been to use the breakpoints as prop names and trying to add responsive other properties might lead to inconsistency.

[brianespinosa]

I'd recommend using @media queries in CSS to do this versus conditionally setting props on a component based on viewport. There are potentially a lot of performance issues with that, depending on how you implement it.

[ryardley]

I kind of agree however resize events don't actually happen much in the wild mainly when the phone gets turned at which point a user expects a short lag. Also I would prefer to have my layout code all in one place. Ideally there would be a semantic-ui responsive class for say for text alignment and the grid component would take advantage of this but (correct me if I am wrong) we are not there yet and the Responsive component already implements visibility based on fixed breakpoints. Having a utility that can do the same for values does not hurt. It would also be a better component design as this logic is already happening within the Responsive component.

[layershifter]

See #1872, there was the discussion about the naming, so mobile, computer and etc. aren't possible. However, I think that we need fireOnMount prop there it will allow to do something like below:

class ResponsiveExample extends Component {
  state = {}

 onResizeHandler = (e, { width }) => this.setState({ width })

  render() {
    const { width } = this.state

    return (
      <Responsive as={Grid} fireOnMount onUpdate={this.onResizeHandler}>
        <Grid.Column 
          mobile={16} 
          computer={4} 
          textAlign={width >= Responsive.computer.minWidth ? 'left' : 'right'} >
          {/* content */}
        </Grid.Column>
        {/* other columns */}
      </Responsive>
    );
  }
}

PR is coming, it will also include a working doc example.

[ryardley]

@layershifter that looks a rather complex way to do something that feels like it should be simpler but it might do for now. I understand technically but I am wondering semantically why we should have to listen to the event in the first place. Why we should need to track screen width. We have a utility that determines visibility based on JS. Why not have it determine a value conditionally? fireOnMount is not very intuitive as to what it is doing and it also sounds a little like something that should happen anyway. I understand the need to add a property instead of making a breaking change to the API however.

[layershifter]

Responsive.value({mobile: 'center', tablet: 'left'})

It's not possible in any way, see discussion in Responsive PR. We can't use semantic names (mobile, tablet, etc.) because they don't relly correspond CSS values. So we will come to:

Responsive.value() // returns `window.innerWidth`

And it's useless. fireOnMount is prop that already exists in API of Visibility, it is reasonable to use it here.

[ryardley]

Don't we end up in a situation in this particular case where React renders twice? Once where width is undefined and once where it is the window.innerWidth? That would mean there would be a flash of un-laidout content which would not be correct in this situation. I am not sure this proposal necessarily solves this problem which was to select/determine responsive values in JS (cf original title of this issue).

[ryardley]

Static methods that actually do the checking might be better?

Responsive.is({minWidth:500, maxWidth:700}) ? 'left' : 'right'

// which might ideally translate to

Responsive.is(Responsive.computer) ? 'left' : 'right'

[layershifter]

@levithomason Let me know what you think about proposals above.

[levithomason]

I've had this need several times my self, needing to set different prop values based on responsive conditions.

Why not CSS? Changing a prop value changes the component behavior and logic, which is beyond the scope of CSS. I think this feature would apply to these cases.

Reusing the logic in Responsive seems like a good approach to me. I'm not sure on the best API here yet, but I think this is the right direction. We should get a list of requirements we need to solve and base the API on that.

I do like this proposal:

// Responsive currently defines:
// static onlyMobile = { minWidth: 320, maxWidth: 767 }

size={Responsive.is(Responsive.onlyMobile) ? 'small' : 'large'}

I think multiple values will be easier to handle with the object syntax, though. Such as needing to adjust the number of grid columns. I find myself battling Grid column count and available responsive values quite a bit.

columns={Responsive.value({ onlyMobile: 1, tablet: 3, computer: 'equal' })}

This would also allow easily defining and using your own values:

Responsive.uhd = { minWidth: 7680 }

<Button size={Responsive.value({ uhd: 'massive' })} />

[levithomason]

Forgot to note, I think fireOnMount should also be the default behavior. Is there a use case for turning this off?

[layershifter]

Forgot to note, I think fireOnMount should also be the default behavior. Is there a use case for turning this off?

Same as Sticky, it will be breaking change for someone. I don't think that fire onUpdate after mount should be default behaviour.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.