toBeVisible not working as expected with display:none set on parent component, when using material-ui Grid

[sorinpav]

• @testing-library/jest-dom version: ^5.16.1
• node version: v14.15.1
• npm (or yarn) version: 6.14.8
• react-testing-library version: ^12.1.2

Relevant code or config:

//material-ui grid used
  <Grid
   {...gridProps}
    container
    className={clsx({
      [cssHidden]: condition, // sets display:"none" based on the condition
    })}
  >
      <Grid
        item
        className={clsx({
          //some additional CSS, not setting any display:none or anything to do with visibility
        })}
      >
        <Field
        label="One Field"
         {...fieldProps}
        />
      </Grid>
      </Grid>

What you did:

I tried to test the fact that the Field is hidden, using the .toBeVisible matcher. The assertion looked something like this:

      expect(
        screen.getByLabelText("One Field")
      ).not.toBeVisible()

The assertion fails even if I'm using getByRole.

What happened:

Received element is visible:
  <input aria-describedby="oneField-helper-text" aria-invalid="false" class="MuiInputBase-input MuiOutlinedInput-input MuiInputBase-inputMarginDense MuiOutlinedInput-inputMarginDense" id="oneField" maxlength="50" name="oneField" readonly="" type="text" value="" />

I have also noticed that if instead of display:none, I use visibility:hidden or opacity:0, it works as expected, it's just with the display:"none" that it doesn't work. Unfortunately, it is impossible for me to change the code to use the any of the two options mentioned, as they have a different behaviour. I need display:none in this case.

Reproduction:

https://codesandbox.io/s/react-testing-library-tobevisible-69djj0

Problem description:

As per the documentation, .toBeVisible should be working even if display:none on a parent component is used, and it appears it is not.

Suggested solution:

This should just be working as per docs. I'm referencing the docs from https://github.com/testing-library/jest-dom#tobevisible

[gnapse]

I'm inclined to think that this is related to the fact that CSS is often not mounted/attached to the document when running in a test environment. So the grid element around that field has a CSS class name that's supposed to make it display: none. But if all the test has is <div class="hidden"> as the grid element, but there's no CSS stylesheet attached that would define .hidden { display: none }, then the test would not work. I wrote more about this in #113 (comment). Let me know if that helps.

The one thing in your issue report that makes me doubt that this is the reason, is that you mention that if you switch things over to visibility: hidden or opacity: 0 it works. The issue I described with the absence of stylesheets in test mode would affect all three cases (assuming you applied those two via the className attribute and not directly as style={{ visibility: 'hidden' }}).

As you hinted, having an example app with tests showing this happening would be great.

[sorinpav]

@gnapse thanks for the reply and sorry for the delay in answering.
I think your reply relates more to attaching CSS / not attaching CSS to the setup. I think my scenario here is a bit different, as I seem to attach the stylesheets properly. I also do see my "cssHidden" class in the screen.debug() console log.

I managed to create a reproduction of this, and it is here: https://codesandbox.io/s/react-testing-library-tobevisible-69djj0 I'll update the issue description as well.

Discovering more stuff about this, it appears to me that the assertion works if we're using normal divs, but not if I'm using material-ui Grid container and Grid item.

Is this not meant to work? I may be missing something here. The repro should contain all details.

Any help is appreciated, thanks so much!

[sorinpav]

I've identified that the assertion actually does not pass in 2 cases:

• when using a Grid, and a css class using display:none
• when using a Grid, and a css class using visibility:hidden.
  Both of these, plus some success cases should be shown now in this codesandbox.

[gnapse]

I also do see my "cssHidden" class in the screen.debug() console log

First, I wanted to clarify that the issue I thought this was about never implied that the class names are not set in the html. They always are. The issue I suspected is about the fact that, depending on how your CSS system works, you may not have the <style> elements attached to the dom, so even though you get a class="cssHidden" in your screen.debug() output, this class would not have any styles attached.

However, thanks to your sandbox example, I have confirmed that this is not the case here.

This is a real head scratcher. I have no idea what's going on there. To give you an idea of the things I tried, I was able to break the test1 case that's normally passing, by modifying the HTML to look like this:

      <div className={`MuiGrid-root ${cssDisplayNone} MuiGrid-container`}>
        <div className="MuiGrid-root MuiGrid-item">
          <div data-testid="test1">Test1</div>
        </div>
      </div>

This mimics the markup you get when you use Grid in the test2 example. So for some reason, having those extra class names there breaks stuff.

At this point, the only thing I can think of to do is to run this locally with a local version of jest-dom that would have some debugging output in place to see where is the logic failing.

I'm not sure when I'll be able to dedicate significant time to this. Any help to further pinpoint the issue, even if not fully solving it, is appreciated. Also, feel free to ping me in a week or two if I have not come back with more comments here.

[sorinpav]

@gnapse thanks so much for your input! I'll keep at it myself as well, and try to come up with more investigation notes in the next few days. In the meantime, I say we'll keep this open in case anyone else wants to contribute!

Thanks again!

[gnapse]

One last thought that came to my mind before I switch off of this issue for now. It occurs to me that there could be a bug or issue in jsdom with the way different CSS rules are combined, not following the specs in terms of the order in which CSS rules are processed.

Specifically, take a look at the styles of these divs when inspecting them in a real browser:

Clearly, the display: none found in the ._default-cssDisplayNone-1 class overrides the display: flex found in the .MuiGrid-container class. But maybe in the test environment these classes are processed in a different order? Perhaps because of a jsdom issue? Or possibly if in the test environment, for some reason, the style elements are attached differently to the DOM?

I have not dug more into this for now. Just throwing it out there as a potential angle from which to attack this.

[sorinpav]

@gnapse Have you by any chance had any time to look further into this?

Thanks again for the help you've already provided!

[gnapse]

No, I have not dug more into this since my last comment.

[aquelehugo]

It is JSDOM's fault for sure.

I added some console to the first breaking test:

  console.log(document.querySelector('head').innerHTML)
  console.log(getComputedStyle(screen.getByTestId("test2").parentElement.parentElement).display) 
  console.log(screen.getByTestId("test2").parentElement.parentElement.className)

and the output was:

<style data-jss="" data-meta="MuiGrid">
.MuiGrid-container {
  width: 100%;
  display: flex;
  flex-wrap: wrap;
  box-sizing: border-box;
}
... lots of Mui classes
</style><style data-jss="" data-meta="_default">
._default-cssDisplayNone-25 {
  display: none;
}
._default-cssOpacityZero-26 {
  opacity: 0;
}
._default-cssVisibilityHidden-27 {
  visibility: hidden;
}
</style>

---

flex 

---

MuiGrid-root _default-cssDisplayNone-25 MuiGrid-container 

---

So as you can see, the stylesheets are in the correct order but getComputedStyle says the container display property is flex.
I also tried inlining style tags directly into your component and when I did that, it worked as expected.
So I guess that is related to the second stylesheet being attached to the DOM after everything. Probably related to: jsdom/jsdom#2986

Testing Library's logic is ok, the issue is JSDOM gives the wrong value at getComputedStyle.

[gnapse]

Thanks for digging in, @aquelehugo, and for gathering all this extra level of details. I guess we can close this now. Maybe we should also comment explicitly in that other jsdom issue providing reference to this case, which may be helpful for whoever works on fixing it.

[aquelehugo]

@gnapse it's nothing. I don't know if they will ever bother to do something about it though :(
I found out there is an earlier issue open since 2015. It is hard to fix because innerText is dependent on a lot of stuff JSDOM doesn't implement.

Anyway, as a workaround, textContent can be used as a fallback in most cases:

    Object.defineProperty(window.HTMLStyleElement.prototype, "innerText", {
      get() {
        return this.textContent;
      },

      set(content) {
        this.textContent = content;
      }
    });