Skip to content

feat: add warning message returned from @testing-library/dom#270

Merged
smeijer merged 3 commits intotesting-library:developfrom
marcosvega91:pr/add_warning_message
Oct 9, 2020
Merged

feat: add warning message returned from @testing-library/dom#270
smeijer merged 3 commits intotesting-library:developfrom
marcosvega91:pr/add_warning_message

Conversation

@marcosvega91
Copy link
Member

What: I have update @testing-library/dom and @testing-library/user-event to the latest versions and add a new warning message to the ResultSuggestion component (look here)

Why: Because on @testing-library/dom we return from getSuggestedQuery a new field warning used to give some advice to the user.
Now this field is only used to show to the user the following message

Element is inaccessible. This means that the element and all its children are invisible to screen readers. If you are using the aria-hidden prop, make sure this is the right choice for your case.

when the selected element is hidden

How: I have added a new div like the image below

Checklist:

image

  • Tests
  • Ready to be merged

@marcosvega91 marcosvega91 force-pushed the pr/add_warning_message branch from d2f147f to 82af9d6 Compare August 23, 2020 07:15
@smeijer
Copy link
Member

smeijer commented Aug 23, 2020

Thanks! This is awesome. I'm just wondering, based on the screenshot, should we still show that "this is awesome..." message? Or should the warning be shown instead of it?

@marcosvega91
Copy link
Member Author

I was thinking about this too. The query is the best one that you can use but the message will only warning you for that use case. So I think that we can leave as is for now.

@smeijer smeijer merged commit 0962018 into testing-library:develop Oct 9, 2020
@smeijer
Copy link
Member

smeijer commented Oct 9, 2020

Thanks again. Sorry I've waited so long with the merge 😔

@marcosvega91
Copy link
Member Author

no problem 😄

@marcosvega91 marcosvega91 deleted the pr/add_warning_message branch October 9, 2020 10:50
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

Successfully merging this pull request may close these issues.

2 participants