Material-ui: [Hidden] Remove component

Created on 14 Feb 2020  ·  6Comments  ·  Source: mui-org/material-ui

  • [x] The issue is present in the latest release.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The Hidden component documentation states: "Any other props supplied will be provided to the root element (native element)." (see https://material-ui.com/api/hidden/)
However, the typescript declaration file for the component does not declare all the native underlying div properties, and the properties are not passed down, except for the className (which is not declared anyway for typescript users).
I use the CssHidden implementation of the component.

Expected Behavior 🤔

I should be able to add custom styles to the underlying div element using className or style properties.

Steps to Reproduce 🕹

Very simple: create a typescript project, use <Hidden mdUp implementation="css" className="whatever">, it does not compile.

Context 🔦

I want to add custom style to the div element generated by the Hidden component, using either className or style property (I need height="100%" at each level here).

I made this workaround for now:

const FixHidden = Hidden as React.ComponentType<HiddenProps & {className: string}>
breaking change Hidden

Most helpful comment

@fromi Thank you for opening this issue! I think that it's a great opportunity to take a step back and to reconsider what problem we wish this component to solve and how it fits into the global picture.

I have been wondering about removing this component in v5. Basically, developers could migrate way from this component.

  1. Replace HiddenCss with the system. For instance:
<Hidden mdDown implementation="css"><A /></Hidden>

would turn into:

<Box display={{ xs: 'none', lg: 'block' }} /><A /></Box>

This is documented in https://material-ui.com/system/display/#hiding-elements and comes from prior-art in:

  1. Replace HiddenJs with useMediaQuery. For instance:
<Hidden lgUp implementation="js"><A /></Hidden>

would turn into:

const hidden = useMediaQuery(theme => theme.breakpoints.up('lg')))

return hidden ? null : <A />;

So in this context, I think that we could keep pushing forward in a direction that gets ride of Hidden. Meaning, a deprecation in v4 and removal in v5. What do you guys think?

All 6 comments

I don't have the full history for this component. For HiddenCSS we could spread the props to the div. However, the types need to reflect that only <Hidden implementation="css" /> supports this. <Hidden implementation="js" /> does not support this.

That's right, but it can be covered in the documentation isn't it ? It would not be the first component to expose properties without effect depending on another properties' value.

That's right, but it can be covered in the documentation isn't it ?

Docs should have a warning as well, yes.

Though it is technically correct. Props are spread to the root element in both cases. One case just doesn't have on ;)

@fromi Thank you for opening this issue! I think that it's a great opportunity to take a step back and to reconsider what problem we wish this component to solve and how it fits into the global picture.

I have been wondering about removing this component in v5. Basically, developers could migrate way from this component.

  1. Replace HiddenCss with the system. For instance:
<Hidden mdDown implementation="css"><A /></Hidden>

would turn into:

<Box display={{ xs: 'none', lg: 'block' }} /><A /></Box>

This is documented in https://material-ui.com/system/display/#hiding-elements and comes from prior-art in:

  1. Replace HiddenJs with useMediaQuery. For instance:
<Hidden lgUp implementation="js"><A /></Hidden>

would turn into:

const hidden = useMediaQuery(theme => theme.breakpoints.up('lg')))

return hidden ? null : <A />;

So in this context, I think that we could keep pushing forward in a direction that gets ride of Hidden. Meaning, a deprecation in v4 and removal in v5. What do you guys think?

I think it would cover every use cases. I was using HiddenCss to avoid a blinking issue with server side rendering, but the "Box display" way should do the same.

for what it's worth, i vote to remove this 🤷‍♀️

Was this page helpful?
0 / 5 - 0 ratings

Related issues

iamzhouyi picture iamzhouyi  ·  3Comments

finaiized picture finaiized  ·  3Comments

activatedgeek picture activatedgeek  ·  3Comments

revskill10 picture revskill10  ·  3Comments

pola88 picture pola88  ·  3Comments