feat: Add setValue method #557

beyer-martin · 2018-04-24T23:25:32Z

#530
First contribution, wanted to know if I am following the correct path.
Its not finished yet.

Thanks

eddyerburgh

Looks great so far :)

Things we need:

setValue added to flow/wrapper.flow.js
A setValue type added to spackages/test-utils/types/index.d.ts and a test in packages/test-utils/types/wrapper.ts

I also think we should throw an error if setValue is called on an element that it won't affect. I think it affects most of the text content input types, like text, email. You can see a full list here—https://html.spec.whatwg.org/multipage/input.html#attr-input-type-keywords.

Could you add tests for each of the text control elements and confirm that setValue does set the v-model value for each type. Then can you write a test and throw an error if setValue is called on an element that isn't a text control.

Thanks 😀

beyer-martin · 2018-04-29T17:24:52Z

I checked that v-model works on every input and confirmed it looking in v-model source code (https://github.com/vuejs/vue/blob/550c3c0d14af5485bb7e507c504664a7136e9bf9/src/platforms/web/compiler/directives/model.js)

I used the same rules that v-model uses to determine the input type, in order to check if the method was valid. Errors and suggestions are thrown if invalid.

Im having difficulties understanding what to test in packages/test-utils/types/wrapper.ts

Will continue with setSelected.

Thanks for reviewing

eddyerburgh · 2018-04-29T18:57:10Z

packages/test-utils/src/wrapper.js

+    const type = this.attributes().type
+    const event = 'input'
+
+    if (this.isVueComponent) {


Why is this check here? I don't think the function needs to behave differently if the wrapper contains a vue instance

Its unnecesary, will remove

eddyerburgh · 2018-04-29T19:02:18Z

packages/test-utils/src/wrapper.js

+    const event = 'change'
+
+    if (this.isVueComponent) {
+      throwError('wrapper.setChecked() cannot be called on component. Use wrapper.setValue() instead')


A component instance still has a root element, which can have its value set, so I think we should let setChecked work for vue instance wrappers

My concern is related to how v-model behaves on components, but Im not sure. Let me know your final decision

The v-model would be set on the root element, so if you have a component like this:

<template> <input type="text" /> </template>

And you called setValue, you would set the value of the input:

const wrapper = mount(TestComponent) wrapper.setValue('some val')

All component instance wrappers have a root element (this.element) so you can keep the current code and it would work correctly on instance wrappers.

Can you remove this check for isVueComponent

eddyerburgh · 2018-04-29T19:03:47Z

Im having difficulties understanding what to test in packages/test-utils/types/wrapper.ts

The tests are to check the typings are correct. You can just add two lines that call wrapper.setChecked(true)
and
wrapper.setValue('some string')

You can see an example for findAll: https://github.com/vuejs/vue-test-utils/blob/dev/packages/test-utils/types/test/wrapper.ts#L60

This file is compiled by typescript, so if the typings are incorrect the file won't compile and the type tests will fail.

beyer-martin · 2018-04-29T21:39:21Z

I believe its done, except for that mysterious failing compatibility test on checkbox and radio v-model @2.1.10. Any idea on that? Thanks

beyer-martin · 2018-05-03T14:35:27Z

@eddyerburgh any idea?

eddyerburgh · 2018-05-06T10:21:31Z

Sorry for the delay, I must have missed the notifications from this PR.

I'm not sure why the tests fail on 2.1.10 without investigating. Have you debugged locally? There might be a different implementation of v-model in 2.1.x

38elements · 2018-05-06T10:51:38Z

If you change vue and vue-template-compiler version to 2.1.10 in package.json and reinstall node modules,
you can execute test in [email protected] .

beyer-martin · 2018-05-07T01:06:39Z

Its solved! Thanks for the help!

There might be a different implementation of v-model in 2.1.x

That was in fact the problem, on that versions v-model on checkboxes and radios is listening for click events. With some changes now it should work on all versions

Anything missing?

eddyerburgh · 2018-05-07T08:09:03Z

packages/test-utils/src/wrapper.js

+    const event = 'input'
+
+    if (tag === 'SELECT') {
+      throwError('wrapper.setValue() cannot be called on select')


You can tell the user to call setSelected(). Also, can you change the errors to this format:

wrapper.setValue() cannot be called on a <select> element. Use wrapper.setSelected() instead

eddyerburgh · 2018-05-07T08:09:47Z

packages/test-utils/src/wrapper.js

+   * Checks radio button or checkbox element
+   */
+  setChecked (checked: boolean) {
+    if (typeof checked !== 'undefined') {


I think we should default to true, so you can call setChecked() and it will set the element to checked. What do you think?

It is set to true if no argument is passed. It isn't set on the parameters as a default but it behaves the way you suggest, there is a test for this specific case.

eddyerburgh · 2018-05-07T08:12:21Z

packages/test-utils/types/index.d.ts

@@ -72,6 +72,10 @@ interface BaseWrapper {
  setData (data: object): void
  setMethods (data: object): void
  setProps (props: object): void
+
+  setValue (value: any): void
+  setChecked (checked: boolean): void


setSelected is missing

Ups, sorry. Done

eddyerburgh · 2018-05-07T08:13:46Z

test/specs/wrapper/setValue.spec.js

+    expect(input.element.value).to.equal('foo')
+  })
+
+  it('calls trigger with input event', () => {


We can remove this test, as long as the dom updates with the v-model, it doesn't matter whether we call trigger or dispatch an event directly.

eddyerburgh · 2018-05-07T08:15:50Z

Good job finding that 2.1.x listens to the click event 👏

I've just realized there aren't any docs pages. Can you add them in this PR?

beyer-martin · 2018-05-07T22:35:12Z

I've just realized there aren't any docs pages. Can you add them in this PR?

Yeah of course, will do!

eddyerburgh · 2018-05-15T06:13:00Z

Hey @beyersito do you have time to add docs this week? It would be great to get this merged 😄

38elements · 2018-05-28T13:51:25Z

Since there is no reply, I think it is better to close this pull request and someone send new one, or merge this one and someone add document.

beyer-martin · 2018-05-28T13:53:08Z

Sorry for the delay, will add docs this week or if someone prefers to add docs great

lmiller1990 · 2018-06-08T16:01:42Z

Let's get this merged, I can add docs tomorrow unless you were about to do so @beyersito ?

eddyerburgh · 2018-06-08T16:30:30Z

Great, I was thinking about this PR earlier 👍

beyer-martin · 2018-06-08T18:53:12Z

Please do @lmiller1990
Been very busy.
Please accept my apologies 😢

eddyerburgh

Thanks @beyersito and @lmiller1990

The setSelected() method was added in vuejs/vue-test-utils#557, but apparently never documented. It seems stable and intended for end-user use nonetheless, so it is probably just an oversight documentation wise.

added setValue method and few tests

b600bbc

eddyerburgh requested changes Apr 25, 2018

View reviewed changes

eddyerburgh mentioned this pull request Apr 25, 2018

[help-wanted] How to change a select element #260

Closed

Added setChecked

a955dec

Fixed error wrapper and wrapper array missing methods

241e772

eddyerburgh reviewed Apr 29, 2018

View reviewed changes

beyer-martin added 2 commits April 29, 2018 16:21

Fixed Flow errors on element Object

5e243ca

Added setSelected method

37150d5

Fixed v-model not updating on 2.1.10

4fddcd1

eddyerburgh reviewed May 7, 2018

View reviewed changes

Minor changes

2e91724

lmiller1990 mentioned this pull request Jun 9, 2018

docs: setValue, setChecked, setSelected #694

Merged

eddyerburgh approved these changes Jun 9, 2018

View reviewed changes

eddyerburgh merged commit b4331ff into vuejs:dev Jun 9, 2018

jaxx2104 mentioned this pull request Jun 15, 2018

docs: ja update #722

Closed

micgro42 mentioned this pull request Nov 24, 2020

Use specialized method to select an <option> in test wmde/query-builder#76

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add setValue method #557

feat: Add setValue method #557

beyer-martin commented Apr 24, 2018

eddyerburgh left a comment

beyer-martin commented Apr 29, 2018

eddyerburgh Apr 29, 2018

beyer-martin Apr 29, 2018

eddyerburgh Apr 29, 2018

beyer-martin Apr 29, 2018

eddyerburgh May 6, 2018

eddyerburgh May 7, 2018

eddyerburgh commented Apr 29, 2018

beyer-martin commented Apr 29, 2018

beyer-martin commented May 3, 2018

eddyerburgh commented May 6, 2018

38elements commented May 6, 2018

beyer-martin commented May 7, 2018

eddyerburgh May 7, 2018

eddyerburgh May 7, 2018

beyer-martin May 7, 2018

eddyerburgh May 7, 2018

beyer-martin May 7, 2018

eddyerburgh May 7, 2018

eddyerburgh commented May 7, 2018

beyer-martin commented May 7, 2018

eddyerburgh commented May 15, 2018

38elements commented May 28, 2018

beyer-martin commented May 28, 2018

lmiller1990 commented Jun 8, 2018

eddyerburgh commented Jun 8, 2018

beyer-martin commented Jun 8, 2018

eddyerburgh left a comment

feat: Add setValue method #557

feat: Add setValue method #557

Conversation

beyer-martin commented Apr 24, 2018

eddyerburgh left a comment

Choose a reason for hiding this comment

beyer-martin commented Apr 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyerburgh commented Apr 29, 2018

beyer-martin commented Apr 29, 2018

beyer-martin commented May 3, 2018

eddyerburgh commented May 6, 2018

38elements commented May 6, 2018

beyer-martin commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyerburgh commented May 7, 2018

beyer-martin commented May 7, 2018

eddyerburgh commented May 15, 2018

38elements commented May 28, 2018

beyer-martin commented May 28, 2018

lmiller1990 commented Jun 8, 2018

eddyerburgh commented Jun 8, 2018

beyer-martin commented Jun 8, 2018

eddyerburgh left a comment

Choose a reason for hiding this comment