Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toBeVisible not working as expected with visibility property #209

Open
lhrinaldi opened this issue Feb 18, 2020 · 38 comments · May be fixed by #428
Open

toBeVisible not working as expected with visibility property #209

lhrinaldi opened this issue Feb 18, 2020 · 38 comments · May be fixed by #428
Labels
bug Something isn't working

Comments

@lhrinaldi
Copy link

lhrinaldi commented Feb 18, 2020

  • @testing-library/jest-dom version: v5.1.1
  • @testing-library/react version: v9.4.0
  • node version: v12.14.1
  • npm (or yarn) version: v1.21.1

Relevant code or config:

import styled from 'styled-components';

const Container = styled.div`
	visibility: hidden;

	&:hover {
		visibility: visible;
	}

	> input:checked {
		visibility: visible;
	}
`;


const { getByTestId } = render(
	<Container>
		<input data-testid="checkbox" type="checkbox" checked />
	</Container>
);

expect(getByTestId('checkbox')).toBeVisible();

What you did:

Tried to test a checkbox that must be visible when it has been checked with toBeVisible.

What happened:

Even though the checkbox is visible toBeVisible throws an error because it detects that the parent has visibility: hidden.

Reproduction:

https://codesandbox.io/s/kind-rain-62otm?fontsize=14&hidenavigation=1&previewwindow=tests&theme=dark

Problem description:

My code has a checkbox that must be visible when the user checks it. The visibility property is perfect for this because it overrides its parents' values.

So, if the checkbox has visibility: visible property, it will be shown regardless of the parent value.

Suggested solution:

Not sure :( will come up with something if this is really an issue.

@eps1lon
Copy link
Member

eps1lon commented Feb 18, 2020

The codesandbox does not use any of the code you described. Don't forget to save and freeze the codesandbox after submission.

I suspect this is an issue with jsdom not implementing CSS cascade. I need a repro to verify though.

@lhrinaldi
Copy link
Author

@eps1lon thanks for letting me know, just included a test example there.

@eps1lon
Copy link
Member

eps1lon commented Feb 18, 2020

Perfect. This is indeed a bug.

We're currently assuming that visibility acts like display. While display: none hides any child even if they do set display: block, visibility: hidden can be overridden. The difference between display and visibility is that visibility is an inherited CSS property while display is not.

We need to account for that now that JSDOM implements CSS inheritance.

@eps1lon eps1lon added the bug Something isn't working label Feb 18, 2020
@lhrinaldi
Copy link
Author

I've tried to implement but I think JSDOM CSS inheritance doesn't work correctly. By default all elements have visibility: visible, so:

<section style="display: block; visibility: hidden">
    <p>Hello <strong>World</strong></p>
</section>

getComputedStyle brings visible for the strong and p elements even if the property is not set. It should've brought hidden because it inherits.

@eps1lon
Copy link
Member

eps1lon commented Feb 20, 2020

getComputedStyle brings visible for the strong and p elements even if the property is not set. It should've brought hidden because it inherits.

Which jsdom version are you using? It should be working with ^15.2.0. If you have the implementation ready then you could go ahead an open a PR and I'll have a look at it.

@lhrinaldi
Copy link
Author

lhrinaldi commented Feb 20, 2020

@eps1lon are you sure CSS inheritance is working? Check this code.
I've tried to replicate what's being done in the project, see that what's written on the page.

It should be

hidden
hidden
hidden

because of inheritance.

jsdom/jsdom#2160

@eps1lon
Copy link
Member

eps1lon commented Feb 20, 2020

@eps1lon are you sure CSS inheritance is working?

For visibility, yes. Seems to be that inline styles are ignored for inheritance. For declaration it's working: https://runkit.com/eps1lon/5e4ea8c4d54b2400136cd768

@lhrinaldi
Copy link
Author

Oh ok, thanks for explaining @eps1lon . Do you think it's ok if I change this test to use style declaration instead of inline styles?

@eps1lon
Copy link
Member

eps1lon commented Feb 20, 2020

I prefer additional tests over changing existing ones. This way we make sure we don't regress.

@lhrinaldi
Copy link
Author

The problem is that implementing this functionality as if the inheritance works will break these tests because it relies on inline styles instead of declared ones.

@eps1lon
Copy link
Member

eps1lon commented Feb 20, 2020

Write two tests then and comment the wrong one properly so that it is obvious that it will break once jsdom is fixed.

@Smolations
Copy link

I don't currently have time to set up a sandbox repro, but this same assertion is failing for me when opacity: 0 (using stylesheet styles). I'm only commenting because it seems this assertion may need more refining.

Stylesheet (imported directly into component definition file):

.Dialog {
  opacity: 0;
  // ...

  .Dialog--open {
    opacity: 1;
    // ...
  }
}

And test output failure (verified other classes not setting opacity: 1):

    expect(element).not.toBeVisible()

    Received element is visible:
      <div aria-labelledby="h9FY4ou1e5jp_LqptbvKB" aria-modal="true" class="Dialog Dialog--with-close-icon ModalDialog" data-component="ModalDialog" data-focus-lock-disabled="disabled" role="dialog" tabindex="-1" />
  • @testing-library/jest-dom version: v5.7.0
  • @testing-library/react version: v10.0.4
  • node version: v10.18.0
  • npm version: v6.13.4

@simonolubhaing
Copy link

Is there any update on this, or a workaround. I've posted a CodeSandbox with minimal code on StackOverflow reproducing the issue
https://stackoverflow.com/questions/62427793/isvisible-doesnt-work-in-jest-test-using-jest-dom

@eps1lon
Copy link
Member

eps1lon commented Jun 17, 2020

Is there any update on this, or a workaround. I've posted a CodeSandbox with minimal code on StackOverflow reproducing the issue
stackoverflow.com/questions/62427793/isvisible-doesnt-work-in-jest-test-using-jest-dom

That stackoverflow question is about opacity not visibility.

@MarcoLeko
Copy link

MarcoLeko commented Sep 23, 2020

Is there any progress on this? I am also facing the same issue and put up a question with all relevant infos here: https://stackoverflow.com/questions/64026278/react-jest-rtl-check-if-element-is-not-visible

Btw. react testing library only reacts to styling changes when, I am using inline-styling in the react comp.

@gnapse
Copy link
Member

gnapse commented Sep 23, 2020

@MarcoLeko you may be experiencing an additional issue where styles from a stylesheet are not loaded automatically in jest/jsdom test env where you load and test the component in isolation. Check this comment and let me know if that helps. To confirm if the stylesheet is loaded, can you confirm that you can assert toHaveStyles with other properties that should be there no matter what? For instance, based on the example in the SO question, does this assertion passes if you add it to your test right after rendering?

expect(screen.getByTestId('backdrop')).toHaveStyle({ position: 'absolute' })

@tatinacher
Copy link

It's working in styled-component v5.2.1, see this issue

@gnapse
Copy link
Member

gnapse commented Jan 19, 2021

Thanks for letting us know @tatinacher. I'll close this issue now.

@gnapse gnapse closed this as completed Jan 19, 2021
@lhrinaldi
Copy link
Author

lhrinaldi commented Jan 19, 2021

@tatinacher @gnapse

Hey 👋, I think there's been a mistake. This issue has nothing to do with styled-components (only my example).

As you can see in the CodeSandbox, I've updated all the dependencies and it seems that the issue persists.

@gnapse
Copy link
Member

gnapse commented Jan 19, 2021

Oh ok, sorry. The signal here got lost between all the noise of styled components and the concern about stylesheets being loaded or not.

Indeed, it is a mistake that we treat parent lack of visibility as final, when in fact visibility, as opposed to display: none, can be overridden by child elements.

@davidnghk01
Copy link

Not sure i got same bug here or no

CodeSandbox here

But seem due to this issue, Code sandbox not working right now.

Below is my code :

import styled from "styled-components";

const Container = styled.a`
  display: flex;
  width: 50px;
  height: 50px;
  background-color: #0f0;
  &:hover {
    background-color: #f00;
  }
`;

const HoverVisibleElement = styled.button`
  padding: 0px;
  margin: 0px;
  outline: 0px;
  border: 0px;
  width: 10px;
  height: 10px;
  background-color: #00f;
  visibility: hidden;
  ${Container}:hover & {
    visibility: visible;
  }
`;

export default function App() {
  return (
    <Container>
      <HoverVisibleElement />
    </Container>
  );
}

In browser it working well by hover then button become visible and unhover it hidden.

But in test below:

import App from "./App";
import "@testing-library/jest-dom/extend-expect";

import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/userEvent";

it("Show button when hover link", async () => {
  render(<App />);
  expect(screen.queryByRole("button")).toBeNull();
  userEvent.hover(screen.getByRole("link"));
  await expect(screen.findByRole("button")).resolves.toBeVisible();
});

it would fail because button not found

@codewaseem
Copy link

codewaseem commented Jan 13, 2022

Here is a simple test to reproduce this bug

import styled from "@emotion/styled";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";

const Container = styled("div")`
  position: relative;

  &:hover {
    & .child {
      visibility: visible;
    }
  }
`;

const Child = styled("div")`
  visibility: hidden;
`;

function Test() {
  return (
    <Container data-testid="container">
      <p>Test</p>
      <Child className="child" data-testid="child">
        Child
      </Child>
    </Container>
  );
}

it("doesn't detect style changes applied from a parent to child", async () => {
  render(<Test />);

  const container = screen.getByTestId("container");

  expect(container).toBeInTheDocument();

  /* 
     these two lines passes the assertion
     which confirms that styles are loaded
  */
  expect(screen.getByTestId("child")).not.toBeVisible();
  expect(screen.getByTestId("child")).toHaveStyle({
    visibility: "hidden",
  });

  /* 
    when container is hovered, it applies the styles to the child 
    using CSS descendant selector.  
  */
  userEvent.hover(container);

  /*
    JSDOM not picking up styles applied to child through CSS descendant selector, 
    so the child is not visible
  */
  expect(await screen.findByTestId("child")).toBeVisible(); // <--- only this line fails
});

@gnapse gnapse linked a pull request Jan 13, 2022 that will close this issue
3 tasks
@gnapse
Copy link
Member

gnapse commented Jan 13, 2022

I've got a PR ready that fixes this issue: #428

@gnapse
Copy link
Member

gnapse commented Jan 13, 2022

BTW, @codewaseem, your example is not related to this issue. It does not involve the visibility CSS property at all.

I believe your example is suffering from the issue that CSS from styled-components or emotion are not loaded in the DOM in the context of jest tests.

@codewaseem
Copy link

codewaseem commented Jan 14, 2022

@gnapse , thanks for your response!

I have updated my example to use visibility property and also I confirm that styles are being loaded from emotion, because these two lines passes

  expect(screen.getByTestId("child")).not.toBeVisible();
  expect(screen.getByTestId("child")).toHaveStyle({
    visibility: "hidden",
  });

which confirms styles are loaded.

Only the last line fails.

I think the issue is, styles that are applied to a child using css descendent selector are not being detected. This is regardless of specific property.

UPDATE: This is working in pure css setup in codesandbox. There might be some issue in my configuration or emotion.

@gnapse
Copy link
Member

gnapse commented Jan 14, 2022

If it's something that you can reproduce regardless of it being related to the visibility CSS property, then open a new issue, please. If it works in codesandbox but not locally it could be a jsdom issue, though.

@M162
Copy link

M162 commented May 11, 2022

Sorry, I see this issue is still in open. Any update about this issue?

@jrnail23
Copy link

jrnail23 commented Jun 8, 2022

Here's an absolutely minimal example of how .toBeVisible() is failing for me, with inline SVG:

document.body.innerHTML = `
  <svg>
    <g id="my-element" visibility="hidden" />
  </svg>`;

// yep, this assertion fails.
expect(document.getElementById('my-element')).not.toBeVisible();

Also in Code Sandbox.

My best guess is that this is a bug in jsdom itself, because the computed style for visibility of my-element ends up being visible here: https://github.com/testing-library/jest-dom/blob/main/src/to-be-visible.js#L6

@Hai-San
Copy link

Hai-San commented Nov 14, 2023

Same problem here with opacity.

sungik-choi added a commit to sungik-choi/bezier-react that referenced this issue Jan 11, 2024
@yaman3bd
Copy link

I am using tailwindcss and vitest and facing an issue where if I am hiding an element using className it does not work, and I have configured vitest to parse CSS but it still does not work.
however, when I test the component in the browser it works fine as expected and it works only if I add visibility as inline style to the element I want to hide.
Button.tsx:

const Button = forwardRef(({ isLoading, children }, ref) => {
  return (
    <button className={cn("[&.btn-is-loading>[data-slot=children]]:invisible", isLoading && "btn-is-loading")}>
      {isLoading ? (
        <span
          data-slot="children"
          children={children}
        />
      ) : (
        children
      )}
    </button>
  );
});

Button.test.tsx:

  it("should hide children content when loading", () => {
    render(<Button isLoading>children</Button>);

    const content = screen.getByText("children", { selector: '[data-slot="children"]' });

    expect(content).not.toBeVisible();
  });

does anyone have any idea what I might have done wrong?

@gnapse
Copy link
Member

gnapse commented Mar 21, 2024

Are you certain that the CSS generated from tailwind CSS classes is loaded and attached to the document in which the components are mounted during the tests? Otherwise, it will not matter if an element has className="invisible", if there's no CSS for .invisibie { visibility: hidden } attached to the document, jest-dom won't detect that it is invisible, as jest-dom relies on getComputedStyle on the element to know what CSS is applied to it.

@yaman3bd
Copy link

yes, this is the log I get in my Terminal when I run the tests:

Screenshot 2024-03-22 at 3 54 41 PM

I had the same issue even when I tried to directly add invisibie class to the element without any conditions

@gnapse
Copy link
Member

gnapse commented Mar 22, 2024

Can you debug it further, but adding something along these lines to your test?

const element = screen.getByText("children", { selector: '[data-slot="children"]' })

// Get the window associated with the element
const elementWindow = element.ownerDocument.defaultView;

// Now you can use the window's getComputedStyle method
const computedStyles = elementWindow.getComputedStyle(element);

console.log('styles', computedStyles)
console.log('visibility', computedStyles.visibility)

@yaman3bd
Copy link

it does not see the element visibility as invisibie when adding a className, but I inline style the visibility it works,
however, this is the output:

styles CSSStyleDeclaration {
  '0': 'visibility',
  '1': 'pointer-events',
  '2': 'background-color',
  '3': 'border-block-start-color',
  '4': 'border-block-end-color',
  '5': 'border-inline-start-color',
  '6': 'border-inline-end-color',
  '7': 'border-top-color',
  '8': 'border-right-color',
  '9': 'border-bottom-color',
  '10': 'border-left-color',
  '11': 'caret-color',
  '12': 'color',
  _values: {
    visibility: 'visible',
    'pointer-events': 'auto',
    'background-color': 'rgba(0, 0, 0, 0)',
    'border-block-start-color': 'GrayText',
    'border-block-end-color': 'GrayText',
    'border-inline-start-color': 'GrayText',
    'border-inline-end-color': 'GrayText',
    'border-top-color': 'GrayText',
    'border-right-color': 'GrayText',
    'border-bottom-color': 'GrayText',
    'border-left-color': 'GrayText',
    'caret-color': 'auto',
    color: 'GrayText'
  },
  _importants: {
    visibility: undefined,
    'pointer-events': undefined,
    'background-color': undefined,
    'border-block-start-color': undefined,
    'border-block-end-color': undefined,
    'border-inline-start-color': undefined,
    'border-inline-end-color': undefined,
    'border-top-color': undefined,
    'border-right-color': undefined,
    'border-bottom-color': undefined,
    'border-left-color': undefined,
    'caret-color': undefined,
    color: undefined,
    'outline-color': undefined
  },
  _length: 13,
  _onChange: undefined,
  _setInProgress: false
}

visibility visible

is there a way to check the generated class names in the element document?
maybe it does not attach the required class name properly

@gnapse
Copy link
Member

gnapse commented Mar 24, 2024

As suspected, the CSS generated from tailwind CSS classes is not attached to the document in which the components are mounted during the tests. Without that CSS code being attached to the document, jest-dom's toBeVisible (or usages of toHaveStyle) won't work.

This is a known limitation. And unfortunately, due to the various ways in which CSS can be loaded in the document at run time (CSS-in-JS, CSS generated at build time, etc.) this falls out of jest-dom's scope. Not much we can do.

@yaman3bd
Copy link

then, is there any workaround or a recommended way to test the element visibility?
I can apply inline style to the elements I have control of, but there are some elements I can have access to them only via selectors like class name or data-att
for example, when the button is loading I want to hide the text and the icon if exists, the text I handle it:

 {isLoading ? (
        <span
          data-slot="children"
          style={{
             visibility: "hidden"
          }}
          children={children}
        />
      ) : (
        children
      )}

but the icon is a user-provided component and it only has a data-att selector smth like:

<svg
  data-slot="icon"
  xmlns="http://www.w3.org/2000/svg"
  viewBox="0 0 24 24"
>
  <!--icon-->
</svg>

so to toggle icon visibility, I am selecting the element using data-att and class name:

.some_class [data-slot=icon]:invisible {
 visibility: "hidden";
}

what is your workaround to be sure the icon is invisible?

@gnapse
Copy link
Member

gnapse commented Mar 25, 2024

Maybe you can trust that the CSS is ok, and test for the presence or absence of the relevant class names instead?

expect(element).toHaveClass('visible')
expect(element).toHaveClass('invisible')

This would not be a true end-to-end test, as it won't test that the CSS to truly make the visibility change effective is there, but since you're using tailwind (which automatically takes care of loading any CSS for any of its classes that you end up using) then that'd be good enough. Is it?

@MatthewPardini
Copy link

You can also remove unused dom elements and use toBeInTheDocument()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.