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

get anchor elements or even other elements #204

Closed
KevinHerklotz opened this issue Feb 5, 2019 · 16 comments
Closed

get anchor elements or even other elements #204

KevinHerklotz opened this issue Feb 5, 2019 · 16 comments
Labels
needs discussion We need to discuss this to come up with a good solution

Comments

@KevinHerklotz
Copy link

Describe the feature you'd like:

When I have an anchor with some nested elements like this:

<a
    href="https://www.example.com"
    role="button"
>
    <span>
		<span class="some-icon-class" />
        <span>Click here!</span>
    </span>
    <span class="MuiTouchRipple-root" />
</a>

I would love to have an easy way to get this anchor with a function like

getAnchorByText()
queryAnchorByText()

that returns the anchor when I say getAnchorByText('Click here!').

Suggested implementation:

I did not look into that yet as I want to make sure others consider this feature as useful as well. If yes I might find the time to create a PR.

Describe alternatives you've considered:

2 possible (not so beautiful) workarounds would be:

  1. Adding a test id to an additional wrapper and look for the anchor like that (The problem is that I use an external library to render the link, so I cannot give the link it's own test id):
getByTestId('button-wrapper').querySelector('a')
  1. Search for the text and go up the DOM tree manually (and pray that the external component will never change its nesting)
getByText('Click here!').parentNode.parentNode

Alternative

There are other HTML elements than anchors that people might want to query for. So if it is preferred we could make it an universal function:

getElementByText(<ElementName>, <Text>)

This would always return the closest element (of the type ElementName) of the text found when you go up the DOM tree.

In my example above I would use it like this:

getElementByText('a', 'Click here!')
@alexkrolick
Copy link
Collaborator

alexkrolick commented Feb 5, 2019

Thanks for the suggestion!

There are other HTML elements than anchors that people might want to query for.

This goes against the goals of the library, which is to provide queries that resemble how users find elements. Tag names are invisible to a user.

There are some some cases that have special treatment by assistive technology, but I'm not sure a tag-based query is the right way to do it, or if this sufficiently resembles the way a mouse (sighted) user would interact. For this I'm thinking a "getFocusableElement" query would resemble how screen readers and tab navigation allow you to cycle between links, buttons, and aria roles that mimic buttons.

@kentcdodds
Copy link
Member

I kinda feel like this is a dupe of #201

Thoughts?

@alexkrolick
Copy link
Collaborator

Yes, #201 would allow selecting the link by text at least (but not specifying that it must be a link unless you add the selector option).

@kentcdodds
Copy link
Member

Oh, that's true. This is slightly different. Hmmm...

Well, normally you interact with the elements and events bubble up, but if you were wanting to make an assertion on the element (like the HREF attribute is set correctly, which would be a totally legit assertion) then being able to access the nearest parent anchor would be useful.

What if we added an option to the query:

const anchor = getByText('Click here!', {getParent: '<selector>'})

By making it an option it de-emphasizes it as something that we don't really want people to typically use, but it gives the escape hatch we want while reducing the impact of implementation details leaking in your tests (by that I mean, this would be better than .parentElement.parentElement).

Thoughts?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Feb 5, 2019

If the innertext query was implemented, this would work:

getByText('nested text', {selector: 'a'})

I'd worry that a parent selector would be abused because in most cases an aria-labelledby association clarifies the relationship correctly.

@kentcdodds
Copy link
Member

Let's stick with that Alex 👍 I like that.

@KevinHerklotz
Copy link
Author

Sounds good to me, Alex. I try to find time during the weekend to implement it.

@ValentinH
Copy link

Any update on this @KevinHerklotz ?

@ValentinH
Copy link

I just did a quick and dirty workaround for now:

const getAnchorByText = (getByText, text) => {
  let el = getByText(text)
  while (el.parentNode) {
    if (el.tagName === 'A') {
      return el
    }
    el = el.parentNode
  }
  throw new Error(`Couldn't find a anchor with the "${text}" text`)
}

I think I could have used a custom query for this but I didn't manage to do it with https://testing-library.com/docs/api-helpers#custom-queries

@kentcdodds
Copy link
Member

You could probably use closest

@ValentinH
Copy link

Indeed, it's much cleaner, thanks for the suggestion! 🙂

@kentcdodds
Copy link
Member

I wonder if the best way to solve this would be to say: "find me the closest interactive element" via a config option like this:

getByText(container, 'submit', {closestInteractiveElement: true})

And the implementation would be basically to add something like this to the end of the query:

return foundNode.closest('a,button') // or anything else?

Just an idea... Not sure I like it.

@kentcdodds kentcdodds added the needs discussion We need to discuss this to come up with a good solution label Mar 19, 2019
@ValentinH
Copy link

I like this very much!

@viniciusavieira
Copy link

I found it can be interesting.
Are you willing a PR to test if it works as expect?
I can try, since it looks like a good first issue to me and a good learning opportunity.

@kentcdodds
Copy link
Member

Go ahead and make the PR. No promises that I'll merge it though. I'm not sure I like the option.

@kentcdodds
Copy link
Member

I really don't think this is the right place for this. PR would still be interesting to look at (and would give this particular feature the best chance of success), but I'd need to be convinced it'd be worthwhile.

I'm going to close this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion We need to discuss this to come up with a good solution
Projects
None yet
Development

No branches or pull requests

5 participants