Added label property to the vaadin-radio-group

[gatanaso]

Connected to #19

This PR introduces a labelproperty to the <vaadin-radio-group> element.

The label is styled according to the same rules used for other vaadin component labels.

This PR does not introduce the styles for required or error states of the component, since those are not yet available as properties.

---

This change is 

[CLAassistant]

All committers have signed the CLA.

[jouni]

Not really a fault of this PR, but it just highlights the problem that we have with styling, that there’s no good way to share styles between components (and we end up copy-pasting the label styles here).

As a future enhancement, to keep it DRY, I suppose the best we could do is extract the label styles into a new style module in vaadin-lumo-styles, using the [part="label"] selector.

@platosha, thoughts?

[tomivirkki]

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

      :host([has-label])::before {
        /* Label height + margin */
        margin-top: calc(var(--lumo-font-size-s) * 1.5);

Lumo-specific stuff should be moved under themes/lumo/

---

Comments from Reviewable

[manolo]

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

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

    <vaadin-demo-snippet id="radio-group-demos-radio-group-label">
      <template preserve-content>
        <vaadin-radio-group label="Radio Group">

Do you mind to use a real use case here, lately we try not to use meaningless examples

---

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

        _labelChanged(label) {
          if (label !== '' && label != null) {

I think here you can safely use if (label)

---

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

        vaadinRadioButtonGroup.label = 'foo';

        expect(vaadinRadioButtonGroup.hasAttribute('has-label')).to.be.true;

It's better assertion check that label has the actual value of 'foo'

---

theme/lumo/vaadin-radio-group.html, line 12 at r2 (raw file):

      :host([has-label])::before {
        /* Label height + margin */

You copied this from text-field, but I think the comment is not needed

---

Comments from Reviewable

[manolo]

---

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[web-padawan]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

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

            /**
            * String used for the label element.

BTW misaligned JSDoc comment, one more whitespace in this line and below.

---

Comments from Reviewable

[web-padawan]

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[web-padawan]

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[web-padawan]

Reviewed 3 of 4 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[coveralls]

Pull Request Test Coverage Report for Build 693

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

---

Totals
Change from base Build 631:	0.3%
Covered Lines:	101
Relevant Lines:	107

---

💛 - Coveralls

[manolo]

---

Reviewed 1 of 1 files at r4, 2 of 3 files at r5, 7 of 7 files at r6.
Review status: all files reviewed, 1 unresolved discussion (waiting on @gatanaso)

---

Comments from Reviewable

[tomivirkki]

theme/material/vaadin-radio-group-styles.html, line 26 at r7 (raw file):

        transform-origin: 0 75%;
        transform: scale(0.75);
        transition: transform 0.175s, color 0.175s, width 0.175s;

These transition properties are not used for anything since the label doesn't float

---

Comments from Reviewable

[tomivirkki]

a discussion (no related file):
Baseline seems off with both of the themes:
https://www.dropbox.com/s/6uzq24y8sdi8bem/Screenshot%202018-06-26%2017.33.09.png?dl=0
https://www.dropbox.com/s/mpg93y2t96v7wvw/Screenshot%202018-06-26%2017.40.38.png?dl=0

---

Comments from Reviewable

[manolo]

@tomivirkki foobar demo

---

Reviewed 1 of 1 files at r7, 1 of 1 files at r9.
Review status: all files reviewed, 3 unresolved discussions (waiting on @gatanaso)

---

Comments from Reviewable

[tomivirkki]

@manolo What about it? My commit removed the foobar demo.

---

Comments from Reviewable

[manolo]

Reviewed 2 of 2 files at r10.
Review status: all files reviewed, 3 unresolved discussions (waiting on @gatanaso)

---

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

Previously, tomivirkki (Tomi Virkki) wrote…

Lumo-specific stuff should be moved under themes/lumo/

Done

---

Comments from Reviewable

[manolo]

We did the same thing. Restored in your way and squashed my commits.

---

Reviewed 2 of 2 files at r11.
Review status: all files reviewed, 3 unresolved discussions (waiting on @tomivirkki and @gatanaso)

---

Comments from Reviewable

[manolo]

Review status: 10 of 11 files reviewed, 3 unresolved discussions (waiting on @manolo, @tomivirkki, and @gatanaso)

---

a discussion (no related file):

Previously, tomivirkki (Tomi Virkki) wrote…

Baseline seems off with both of the themes:
https://www.dropbox.com/s/6uzq24y8sdi8bem/Screenshot%202018-06-26%2017.33.09.png?dl=0
https://www.dropbox.com/s/mpg93y2t96v7wvw/Screenshot%202018-06-26%2017.40.38.png?dl=0

I see them aligned:

http://g.recordit.co/wIp3kpQTvs.gif
http://g.recordit.co/ppVzbDreWv.gif

---

Comments from Reviewable

[yuriy-fix]

Reviewed 1 of 3 files at r5, 1 of 7 files at r6, 1 of 2 files at r10, 2 of 2 files at r11, 1 of 1 files at r12, 5 of 5 files at r13.
Review status: all files reviewed, 3 unresolved discussions (waiting on @tomivirkki)

---

a discussion (no related file):

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

I see them aligned:

http://g.recordit.co/wIp3kpQTvs.gif
http://g.recordit.co/ppVzbDreWv.gif

Seems to be ok.
https://www.dropbox.com/s/g3yugv07v2m1uw8/Screen%20Shot%202018-06-27%20at%2010.18.41.png?dl=0
https://www.dropbox.com/s/irs14nwtrol4je2/Screen%20Shot%202018-06-27%20at%2010.19.02.png?dl=0

---

Comments from Reviewable

[tomivirkki]

a discussion (no related file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

Seems to be ok.
https://www.dropbox.com/s/g3yugv07v2m1uw8/Screen%20Shot%202018-06-27%20at%2010.18.41.png?dl=0
https://www.dropbox.com/s/irs14nwtrol4je2/Screen%20Shot%202018-06-27%20at%2010.19.02.png?dl=0

@yuriyvaadin your screenshots don't have the components wrapped in a flexbox

---

Comments from Reviewable

[yuriy-fix]

Review status: all files reviewed, 3 unresolved discussions (waiting on @tomivirkki and @manolo)

---

a discussion (no related file):

Previously, tomivirkki (Tomi Virkki) wrote…

@yuriyvaadin your screenshots don't have the components wrapped in a flexbox

Yeah, sorry. Just checked. The baseline is misaligned.

---

Comments from Reviewable

[manolo]

Review status: all files reviewed, 3 unresolved discussions (waiting on @tomivirkki)

---

a discussion (no related file):

Previously, YuriyVaadin (Yuriy Yevstihnyeyev) wrote…

Yeah, sorry. Just checked. The baseline is misaligned.

I took a look to text-field and seems that covering baseline in a flex context needs much more work, basically some wrappers and styles for those.

I'll try to figure out all the changes needed for that.

---

Comments from Reviewable

[tomivirkki]

a discussion (no related file):
New API needs an update to analysis.json

---

Comments from Reviewable

[manolo]

Reviewed 8 of 8 files at r14.
Review status: all files reviewed, 4 unresolved discussions (waiting on @tomivirkki and @gatanaso)

---

Comments from Reviewable

[manolo]

Reviewed 1 of 2 files at r15.
Review status: 11 of 12 files reviewed, 4 unresolved discussions (waiting on @manolo, @tomivirkki, and @gatanaso)

---

a discussion (no related file):

Previously, tomivirkki (Tomi Virkki) wrote…

New API needs an update to analysis.json

Done

---

Comments from Reviewable

[manolo]

Reviewed 1 of 2 files at r15.
Review status: all files reviewed, 4 unresolved discussions (waiting on @tomivirkki)

---

Comments from Reviewable

[yuriy-fix]

---

Reviewed 7 of 8 files at r14, 2 of 2 files at r16.
Review status: all files reviewed, 4 unresolved discussions (waiting on @tomivirkki)

---

Comments from Reviewable

[tomivirkki]

---

Reviewed 1 of 3 files at r5, 1 of 7 files at r6, 1 of 2 files at r11, 7 of 8 files at r14, 2 of 2 files at r16.
Review status: complete! all files reviewed, all discussions resolved

---

Comments from Reviewable

[tomivirkki]

theme/lumo/vaadin-radio-group-styles.html, line 5 at r16 (raw file):

    <style>
      :host {
        --lumo-text-field-size: var(--lumo-size-m);

Shouldn't say text-field

---

Comments from Reviewable

[jouni]

Review status: all files reviewed, 1 unresolved discussion (waiting on @gatanaso)

---

theme/lumo/vaadin-radio-group-styles.html, line 5 at r16 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

Shouldn't say text-field

Where is this even used? The whole concept of component-specific​ size properties in Lumo is not fully​ thought out, and I would avoid adding any more of these (IIRC text-field and button have them).

---

Comments from Reviewable

[manolo]

Review status: all files reviewed, 1 unresolved discussion (waiting on @tomivirkki)

---

theme/lumo/vaadin-radio-group-styles.html, line 5 at r16 (raw file):

Previously, jouni (Jouni Koivuviita) wrote…

Where is this even used? The whole concept of component-specific​ size properties in Lumo is not fully​ thought out, and I would avoid adding any more of these (IIRC text-field and button have them).

@tomivirkki It's used in :host::before {height: var(--lumo-text-field-size);} not sure if it's really needed and does not break anything.
I mean if someone changes that value in global scope, and we have in :host::before { height: var(--lumo-size-m));} are we going to have alignment problems @jouni ?

---

Comments from Reviewable

[manolo]

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

---

theme/lumo/vaadin-radio-group-styles.html, line 5 at r16 (raw file):

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

@tomivirkki It's used in :host::before {height: var(--lumo-text-field-size);} not sure if it's really needed and does not break anything.
I mean if someone changes that value in global scope, and we have in :host::before { height: var(--lumo-size-m));} are we going to have alignment problems @jouni ?

Removed, setting value to --lumo-size.m

---

Comments from Reviewable

[tomivirkki]

theme/lumo/vaadin-radio-group-styles.html, line 5 at r16 (raw file):

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

Removed, setting value to --lumo-size.m

What I meant was that the name of the property should have been "--lumo-radio-group-size" instead of "--lumo-text-field-size". But if it's not needed, even better.

---

Comments from Reviewable

[tomivirkki]

Reviewed 1 of 1 files at r17.
Review status: complete! all files reviewed, all discussions resolved

---

Comments from Reviewable