Make fuzzy matching opt-in instead of default #31
Conversation
alexkrolick
force-pushed
the
no-fuzzy
branch
2 times, most recently
from
fd08b01 to
425083f
Compare
src/__tests__/element-queries.js
Outdated
| @@ -129,6 +129,114 @@ test('can get elements by data-testid attribute', () => { | |||
| expect(queryByTestId('first-name')).not.toBeInTheDOM() | |||
| }) | |||
|
|
|||
| test('queries passed strings match case-sensitive exact strings', () => { | |||
There was a problem hiding this comment.
For these two test cases, I think we could improve things slightly with jest-in-case. Could you look into that please?
src/matches.js
Outdated
| @@ -1,8 +1,10 @@ | |||
| function matches(textToMatch, node, matcher) { | |||
| function fuzzyMatches(textToMatch, node, matcher, collapseWhitespace = true) { | |||
There was a problem hiding this comment.
Perhaps for both of these the forth arg should be an options argument. This would have the benefit of being named so we don't need the constant in the queries file and also if we decide to allow forget configuration we could extend this with more options without trouble.
In addition, I wonder if we should allow users to configure this in their own calls to the queries by forwarding the options they pass to this 🤔 would that complicate things too much? If so, let's do it later if at all.
There was a problem hiding this comment.
Ok, implemented. I also separated trim and collapseWhitespace - everything gets trimmed by default but only content is collapsed (not attributes).
|
Sorry, me merging #34 gave you some conflicts :-( |
- Changes queries to default to exact string matching
- Can opt-in to fuzzy matches by passing { exact: true } as the last arg
- Queries that search inner text collapse whitespace
(queryByText, queryByLabelText)
This is a breaking change!
|
That's ok, rebased. |
|
I think it's ready now 🛫 |
README.md
Outdated
| * In most cases using a regex instead of a string gives you more control over | ||
| fuzzy matching and should be preferred over `{ exact: false }`. | ||
| * `trim`: Defaults to `true`; trim leading and trailing whitespace. | ||
| * `collapseWhitespace`: Defaults to `false` for attribute queries and `true` for content queries (i.e., `queryByLabelText`, `queryByText`). Collapses inner whitespace (newlines, tabs, repeated spaces) into a single space. |
There was a problem hiding this comment.
It just occurred to me. In what situations would not trimming of collapsing white space be desirable? The user never sees the text as anything but collapsed and trimmed right? Should we even have these options?
There was a problem hiding this comment.
Line wrapping depends on CSS white-space and other properties. I'm leery of aggressively normalizing the DOM without a way to turn it off.
The defaults are fine in most cases though.
There was a problem hiding this comment.
Maybe they should default for true for everything...
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What:
Fixes #30
Why:
How:
Checklist: