Validation api

[manolo]

Connects to #18

This change is 

[coveralls]

Pull Request Test Coverage Report for Build 719

• 20 of 20 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+4.7%) to 94.313%

---

Totals
Change from base Build 713:	4.7%
Covered Lines:	121
Relevant Lines:	124

---

💛 - Coveralls

[yuriy-fix]

Reviewed 12 of 12 files at r1, 2 of 3 files at r2.
Review status: all files reviewed, 4 unresolved discussions (waiting on @manolo)

---

src/vaadin-radio-group.html, line 188 at r1 (raw file):

              // it's not possible to set readonly to radio buttons, but we can
              // unchecked ones instead.
              button.disabled = button !== this._checkedButton && this.readonly;

This functionality of readonly state seems unusual to me. Do we really need to implement it? There is no such functionality in paper-radio-group or on MDN specification for radiogroup: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/radiogroup .

---

src/vaadin-radio-group.html, line 202 at r1 (raw file):

          this.addEventListener('keydown', e => {

            if (this.disabled || this.readonly) {

Nit: untested code.

---

src/vaadin-radio-group.html, line 281 at r1 (raw file):

          this._radioButtons.forEach(button => button.checked = button === checkedButton);
          this.value = checkedButton.value;
          this.validate();

Nit: untested code.

---

src/vaadin-radio-group.html, line 295 at r1 (raw file):

          }

          if (newV !== '' && newV != null) {

Nit: untested code.

---

Comments from Reviewable

[manolo]

Review status: all files reviewed, 4 unresolved discussions (waiting on @manolo)

---

src/vaadin-radio-group.html, line 188 at r1 (raw file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

This functionality of readonly state seems unusual to me. Do we really need to implement it? There is no such functionality in paper-radio-group or on MDN specification for radiogroup: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/radiogroup .

we have readonly in all our components, and flow expects that all form elements that hasValue implements that, otherwise they offer an API that wouldn't work

https://github.com/vaadin/flow/blob/master/flow-server/src/main/java/com/vaadin/flow/component/HasValue.java#L193

They already expects that their VaadinRadioGroup follows this API because they extend the AbstractField which eventually implements HasValue

https://github.com/vaadin/vaadin-radio-button-flow/blob/master/src/main/java/com/vaadin/flow/component/radiobutton/GeneratedVaadinRadioGroup.java#L73

---

Comments from Reviewable

[manolo]

Review status: 10 of 12 files reviewed, 2 unresolved discussions (waiting on @YuriyVaadin and @manolo)

---

src/vaadin-radio-group.html, line 202 at r1 (raw file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

Nit: untested code.

Actually unneeded code since radiobutton disabled already takes care of it.
Added tests to demonstrate the feature
Done.

---

src/vaadin-radio-group.html, line 281 at r1 (raw file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

Nit: untested code.

Done

---

src/vaadin-radio-group.html, line 295 at r1 (raw file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

Nit: untested code.

Done

---

Comments from Reviewable

[yuriy-fix]

Reviewed 3 of 3 files at r3.
Review status: complete! all files reviewed, all discussions resolved

---

Comments from Reviewable

[tomivirkki]

demo/radio-group-demos.html, line 32 at r4 (raw file):

    </vaadin-demo-snippet>

    <h3>Validation</h3>

I don't see how this demonstrates the feature as there's no way to select an invalid option. Maybe we should just add "required error-message='Something....'" to the "Radio Group with Iron Form" where the group is validated on Submit

---

Comments from Reviewable

[manolo]

Review status: 0 of 12 files reviewed, 1 unresolved discussion (waiting on @YuriyVaadin and @manolo)

---

demo/radio-group-demos.html, line 32 at r4 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

I don't see how this demonstrates the feature as there's no way to select an invalid option. Maybe we should just add "required error-message='Something....'" to the "Radio Group with Iron Form" where the group is validated on Submit

If you visit wit keyboard tab it shows on focus out.
Will try to make a better demo, as you indicate iron form might be the most appropriate to reuse

---

Comments from Reviewable

[manolo]

Review status: 0 of 12 files reviewed, 1 unresolved discussion (waiting on @YuriyVaadin and @manolo)

---

demo/radio-group-demos.html, line 32 at r4 (raw file):

Previously, manolo (Manuel Carrasco Moñino) wrote…

If you visit wit keyboard tab it shows on focus out.
Will try to make a better demo, as you indicate iron form might be the most appropriate to reuse

Moved validation to iron-form demos. At the same time adding real case label, names and values to other demos.

---

Comments from Reviewable

[tomivirkki]

Reviewed 1 of 12 files at r4, 11 of 11 files at r5.
Review status: complete! all files reviewed, all discussions resolved

---

Comments from Reviewable

[platosha]

Reviewed 1 of 12 files at r4, 11 of 11 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manolo)

---

test/vaadin-radio-group.html, line 319 at r5 (raw file):

      });

      it('should remove aria-hiden when error is shown', () => {

Nit: typo aria-hidden

[manolo]

Reviewed 1 of 12 files at r4, 4 of 11 files at r5, 4 of 7 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved