dom-testing-library icon indicating copy to clipboard operation
dom-testing-library copied to clipboard

Allow queries to take optional filter function

Open lazytype opened this issue 4 years ago • 10 comments

Describe the feature you'd like:

A common pattern that I've been using is:

const element = getByText(...);
expect(element).toBeVisible();

However, it may be the case that there are invisible elements that match the first selector, leading to a thrown error since multiple elements were found. It would be much nicer to be able to write something like:

const element = getByText(..., {filter: isVisible}); // It would also be nice for jest-dom to export `isVisible`

Describe alternatives you've considered:

If theisVisible use-case is by far the most common, then it could be sufficient to export queryVisibleBy* methods directly.

lazytype avatar Jul 07 '20 20:07 lazytype

Have you tried using getByRole(role, { hidden: true })? I don't think this needs a new query personally, and I'm not sure what it would consider other than visibility. getByRole seems to be more focused.

nickserv avatar Jul 07 '20 22:07 nickserv

It doesn't quite cover all the same use-cases since hidden only considers the computed visibility CSS property whereas toBeVisible also considers display and opacity (and doesn't require a role attribute).

lazytype avatar Jul 08 '20 00:07 lazytype

It could get difficult to have fully accurate visibility detect, as JSDOM doesn't support all layout features so we may not know if something like positioning or an image covering another at a certain size would cause an element to be invisible. I could see this still being useful for what you mentioned though, but as more of an option than a new query.

nickserv avatar Jul 08 '20 02:07 nickserv

Unless I'm mistaken, *ByRole does this.

kentcdodds avatar Jul 08 '20 18:07 kentcdodds

I believe this was a comment on getByRole https://github.com/testing-library/dom-testing-library/issues/684#issuecomment-655203800

nickserv avatar Jul 08 '20 22:07 nickserv

Would this allow for any custom filter method (not just a matcher)? I have use-cases for asynchronously waiting for certain DOM elements to have their attributes updated.

At the moment, I am writing a lot of custom waitFors and teaching proper usages of waitFors isn't really scalable.

e.g.

await waitFor(() => {
  const button = screen.getByLabelText('some button');
  if (!button.disabled) throw 'Expected `some button` to be disabled.';
})

into

await screen.findByLabelText('some button', { filter: (element) => !!element.disabled });

juanca avatar Feb 10 '21 00:02 juanca

Would this allow for any custom filter method (not just a matcher)? I have use-cases for asynchronously waiting for certain DOM elements to have their attributes updated.

This is an interesting idea and I think it can reduce the custom waitFor you're talking about. I'm adding the needs discussion label for now, let's see what others think about it.

MatanBobi avatar Mar 29 '21 07:03 MatanBobi

I also end up waiting on a button to become enabled a lot like this:

let button;
await waitFor(() => {
  button = screen.getByRole('button', {name: 'foo'});
  expect(button).toBeEnabled()
});
userEvent.click(button);

I would find a lot of value in either a interface like @juanca proposed

await screen.findByLabelText('some button', { filter: (element) => !!element.disabled });

or adding an option to waitFor that only runs the callback on dom mutations

mhess avatar Jun 10 '22 20:06 mhess

I ended up writing my own wrapper. It had an okay success in my work -- since it is yet another way to do things. But I leave this here for your own interest, @mhess et al:

https://github.com/juanca/accessible-query

juanca avatar Jun 10 '22 21:06 juanca

The most intuitive way is to do it the cypress way. i.e. retry queries until the last query/action succeeds. With that model, click() would throw when it tries to click something unclickable such as disabled elements, then it would go back and retry all queries that lead to the click action and then would try to click again.

Because that's a huge change, the closest we can get is to have a click that throws for disabled elements and use click inside a waitFor callback like waitFor(() => userEvent.click(screen.getByLabelText('foo'))).

anilanar avatar Jan 18 '23 20:01 anilanar