fix: use an option's label attribute for toHaveDisplayValue - #734
Open
wahidrizka wants to merge 1 commit into
Open
fix: use an option's label attribute for toHaveDisplayValue#734wahidrizka wants to merge 1 commit into
wahidrizka wants to merge 1 commit into
Conversation
The matcher promises the value the end user sees, but it read only option.textContent. An option that carries a label attribute renders that label instead, so <option value="apple" label="Apple"></option> reported an empty display value. Falls back to the option text when the label attribute is missing or empty, which is how the HTML spec defines an option's label. Closes testing-library#507
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
toHaveDisplayValuereadsoption.textContentfor<select>elements. When an option carries alabelattribute, that attribute is what the browser renders, so the matcher reports the wrong value:The README already promises "the displayed value (the one the end user will see)", so the docs were right and the implementation was not.
Closes #507.
Fix
Per the HTML spec, an option's label is the
labelcontent attribute if there is one and its value is not the empty string, otherwise the option's text.||covers all three cases: attribute absent (getAttributereturnsnull), attribute present but empty (''), and attribute present with a value.That empty-string case is why I did not use
hasAttribute('label'), which the issue suggests:<option label="">Apple</option>renders asApple, not as an empty string. There is a test covering it.Tests
Two added to
src/__tests__/to-have-display-value.js, one per branch of that rule. The first fails onmainand passes with the change; the second passes either way and exists to keep a naive fix from regressing it.I ran the whole suite before and after: 94 tests fail on
mainunchanged (pre-existing snapshot colour mismatches in unrelated files) and exactly the same 94 fail afterwards, with 4 more passing.eslintandprettier --checkare clean on both touched files.