Material-ui: [core] The hidden prop does not always visually hide elements

[shtengel]

Im using the hidden prop of Container to show only the current Tab and to hide the rest (copied from the Tabs example)
was working fine until i upgraded to version 4.9.9 from version 4.9.1 (could be higher version but i think 4.9.1)

• [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 😯

<Container hidden={tabIndex != 1}> </Container>

The Container is not hidden even though tabIndex = 1

Expected Behavior 🤔

Container should not be visible

Steps to Reproduce 🕹

<Container hidden={tabIndex != 1}>sdfsdfsdfds</Container>

https://codesandbox.io/s/empty-browser-n8fwh?fontsize=14&hidenavigation=1&theme=dark

• Works on 4.9.1 *

Steps:

1. upgrade to version 4.9.9
2. use a Container with hidden prop as True

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.9.9 |
| React | v16.12.0 |
| Chrome | |

[eps1lon]

I think this is a general issue with overriding display. Setting a default display means that the user agent styling for [hidden] will be overridden.

[oliviertassinari]

@shtengel What's the use case for using hidden, why not changing the styles?
I can count the usage of display: block; in 10+ other Material-UI components.

[eps1lon]

What's the use case for using hidden, why not changing the styles?

hidden has different semantics. It's also a web standard so supporting that isn't really something that needs to be questioned.

[oliviertassinari]

hidden has different semantics. It's also a web standard so supporting that isn't really something that needs to be questioned.

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden. We had a pass discussion on the topic in #15178, luckily, we solved the issue without even trying :).

However, we also have these components to consider: AppBar, Avatar, Badge, ButtonBase, BottomNavigation, Table, Stepper, ListItem, Switch, StepIcon, Slider, InputLabel, CircularProgress, CardMedia, CardActionArea, and SvgIcon that ignore the hidden attribute (I have likely missed a couple others).

[shtengel]

@oliviertassinari what do you mean changing the style?
i am using hidden to hide and show Tabs
if you go to https://codesandbox.io/s/empty-browser-n8fwh?fontsize=14&hidenavigation=1&theme=dark
and change the material-core version to 4.9.1 you'll see that the hidden feature works fine.
its somewhere between 4.9.1 and 4.9.9

[oliviertassinari]

@shtengel Any reason for using hidden

<Container hidden={{A ? true : undefined }} />

over

<Container style={{ display: A ? 'none' : undefined }} />

or

{A ? <Container /> : null}

?

[shtengel]

@oliviertassinari Not a major reason, just according to here: https://reactjs.org/docs/faq-styling.html
CSS classes are generally better for performance than inline styles so i try to avoid using style wherever i can. if Container is using styles then it won't make much difference

[oliviertassinari]

In https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/hidden, we have:

Appropriate use cases for hidden include:

1. Content that isn't yet relevant but may be needed later
2. Content that was previously needed but is not any longer
3. Content that is reused by other parts of the page in a template-like fashion
4. Creating an offscreen canvas as a drawing buffer

Inappropriate use cases include:

1. Hiding panels in a tabbed dialog box
2. Hiding content in one presentation while intending it to be visible in others

I think that we should ignore 2. 3. and 4. I think that we should also ignore the use case to only set a node invisible (encourage inline style or CSS instead).

I have a concern with 1. and especially with React suspense: https://github.com/oliviertassinari/react-swipeable-views/issues/453#issuecomment-417939459, it might be important.

On the other hand, a. I haven't seen any progress of the hidden prop in React, b. developers could always add an extra DOM node, c. what's the overhead of supporting hidden (we have 30+ components to update)?

Out of this tradeoff equation, I'm leading toward recognizing the problem (it exists) but ignoring it (we have more to lose). @eps1lon What do you think?

[shtengel]

@oliviertassinari im using the hidden prop as suggested here https://material-ui.com/components/tabs/
but instead of <Typography> i decided to use <Container>

[oliviertassinari]

@shtengel Oh, ok, perfect, so it's simply a leftover from https://github.com/mui-org/material-ui/commit/0369bfc885316a7db89eeb375dcf939f2017ddeb. I will update it, so you don't see this pattern anymore.

[shtengel]

@oliviertassinari Ok :) Thanks for the quick reply!
so to use style instead of hidden?
or to wait for new version of material-ui/core

[oliviertassinari]

so to use style instead of hidden?

@shtengel I recommend you to use <Container style={{ display: A ? 'none' : undefined }} /> or {A ? <Container /> : null} unless you have a good reason to use hidden instead.