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

Add support for <br /> (line breaks) when matching text #750

Closed
EvgenyOrekhov opened this issue Aug 21, 2020 · 11 comments
Closed

Add support for <br /> (line breaks) when matching text #750

EvgenyOrekhov opened this issue Aug 21, 2020 · 11 comments

Comments

@EvgenyOrekhov
Copy link

Describe the feature you'd like:

Given the following markup:

<div>Hello<br />World</div>

considering that innerText of the above element would be "Hello\nWorld" (see this JSFiddle), I expect that I would be able to use this query:

screen.getByText("Hello\nWorld");

but it throws the following error:

TestingLibraryElementError: Unable to find an element with the text: Hello
World. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

This example on CodeSandbox

Suggested implementation:

Describe alternatives you've considered:

I've considered using a custom function, like the error message suggests, but this is my first experience with Testing Library, and I don't really know how to do it yet.

And since it's a simple case, I would rather like to use a simple query, without providing any custom functions.

Teachability, Documentation, Adoption, Migration Strategy:

@kentcdodds
Copy link
Member

Hi @EvgenyOrekhov,

Thanks for the issue. I can see some of the logic behind this, but I'm not sure. Any thoughts from others?

@nersoh
Copy link

nersoh commented Aug 26, 2020

One thought that I had was to set a space as textContent for children without it (empty tags like <br />) in getNodeText

Maybe not the ideal solution, but it might lead us to find one.

const ELEMENT_NODE = 1;
const TEXT_NODE = 3;

return Array.from(node.childNodes)
    .filter(child => [ELEMENT_NODE, TEXT_NODE].includes(child.nodeType))
    .map(child => {
        if (!Boolean(child.textContent)) {
      	    child.textContent = " ";
        }
        return child;
     })
    .map(c => c.textContent)
    .join('')

@kentcdodds
Copy link
Member

I think I'd like to hear the justification for this. What's the real world use case for wanting to pass a newline in the query to match a <br />?

@EvgenyOrekhov
Copy link
Author

@kentcdodds In our case <br /> usage is significant for user experience (according to our design team).

Our design team wants the following message to be centered on the page and split into two lines:

<p class="centered">
  Look for an email from {{this.systemEmailAddress}},
  <br />
  if you don't see it, check your spam folder.
</p>

So, I would like to be able to check that the text that the users sees includes that newline:

Look for an email from [email protected],
if you don't see it, check your spam folder.

@kentcdodds
Copy link
Member

That makes sense. One thing you could do is this:

expect(screen.getByText(/look for an email/i)).toMatchInlineSnapshot()

That would output everything with the <br />. I'm just not sure how I feel about encoding things like this in the query because you could implement that same thing using spans/divs/etc.

@EvgenyOrekhov
Copy link
Author

@kentcdodds Doesn't snapshot testing contradict with Testing Library's idea of not relying on implementation details?

Hmm, I see now that it is not possible to use screen.getByText("Hello World") or screen.getByText(/hello world/i) for the following markup:

<div>Hello <span>World</span></div>

That makes me realize that implementing support for <br /> would not provide much value, because, like you said, there would still be a lot of cases with spans/divs/etc.

Sorry if this is turning into a Stack Overflow question, it's my first experience with Testing Library, I'm still trying to figure out how to use it properly.

If you have some advice on how to approach the above cases, or if you can point me in the right direction, please do.

@kentcdodds
Copy link
Member

Yes, snapshots are typically not recommended, but this is one case where I think there's merit (especially since the snapshot will be quite small). For more on this: https://kcd.im/snapshots

I think that's as far as we can take this. If you have further questions and/or would like to participate in our community, please join us on https://testing-library.com/discord

@christo8989
Copy link

@EvgenyOrekhov

Might not be the best solution but it's working for me.

screen.getByText(/Look for an email from.*if you don't see it, check your spam folder./);

@EvgenyOrekhov
Copy link
Author

@christo8989 The original goal was to assert that there is a newline, i. e. it should fail if there's a space instead of a newline.

@christo8989
Copy link

@EvgenyOrekhov

Right. I see.

So maybe separating by divs and asserting each divs content is better?

I now understand why snapshot might be good but yeah. Implementation details effect that.

It seems there's no way to get away from implementation details though.

🤔

@meyertee
Copy link

meyertee commented Dec 19, 2024

I implemented a similar requirement this way:

const text = screen.getByText(/Hello/i);
expect(text.innerHTML).toEqual('Hello<br>World');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants