fix(queryAllByAttribute): query input elements with dynamically assigned value

[eunjae-lee]

What: The implementation of queryAllByAttribute has been changed so that it can successfully query input elements with dynamically assigned value.

Why: When value is dynamically assigned to an input, not by an attribute on HTML, queryAllByAttribute won't query that element at the moment.

How: queryAllByAttribute behaves differently when its attribute is equal to "value". I'm not sure if this is the best way to implement this. Feedbacks are welcomed.

Checklist:

• Documentation
• Tests
• Ready to be merged
• Added myself to contributors table

[eunjae-lee]

closes #158

[eunjae-lee]

Thoughts? @alexkrolick

[kentcdodds]

I don't like this. I might be inclined to add a new query called queryByProperty which compares DOM node properties directly rather than getAttribute. I honestly don't really like getting by attributes or properties because those are not typically visible to the end user. Getting by the value property of an input is acceptable though because the value of an input is visible to the user.... Hmmm...

What do you think @alexkrolick?

[sompylasar]

My two cents: if the goal of this library is to operate like a user viewing the page with eyes or screen reader, the inputs (except of type button and submit) and select should be special cases because their default UI is showing certain state that the test may want to assert on. The gray area is when the default UI is hidden, and most likely there are some other elements that represent the actual UI that the user should look at.

[kentcdodds]

True. We do have getBySelectText perhaps we should do getByInputText and getByTextAreaText as well?

[sompylasar]

The only reason for querying by attribute is because the attributes are proxies to what the browser displays to the user (there's no good way to query the browser render tree itself and get annotated positioned semantic items out of it).

[alexkrolick]

Maybe the queries for select value and textarea could be combined with the input query - getByCurrentValue? This would smooth over the select value in the same way React does it.

I think this would work for most use cases because the value property and the DOM attribute are initially in sync when an element is created, and when they are different, the property is what is visually displayed.

https://codepen.io/alexkrolick/pen/NELEKE?editors=1011

[kentcdodds]

Ok, I'm in favor of that. Let's go ahead and add it and we can add a deprecation message to the README for the getBySelectText. Then we can officially remove it in a future breaking change (when we have sufficient justification to break stuff).

Thanks for the PR anyway @eunjae-lee! Would you be interested in making a PR to add a getByCurrentValue query?

[eunjae-lee]

Thanks all for the feedbacks. I really love how this conclusion has been made!

@kentcdodds I'm still interested in contributing on this. To make sure, getByCurrentValue will refer to value property or (if not exists) DOM attribute, right?

[kentcdodds]

I think the property should be sufficient.

[eunjae-lee]

Hmm. Although value property hasn't been assigned to the element, if it has DOM attribute, end-user will see it on screen. Shouldn't we get elements like this?

[alexkrolick]

The value property is initially set to the same thing as the attribute, unless I'm interpreting this wrong (check console) https://codepen.io/alexkrolick/pen/NELEKE?editors=1011

Note that getByCurrentValue should also do what getBySelectText does now, so the same interface can be used for user input.

[eunjae-lee]

@alexkrolick oh you're right. My bad. I'll bring a draft for getByCurrentValue today.