[core] [Label] render label element instead of div

[gianmarcotoso]

Fixes #1971

Fixes #1971

Changes proposed in this pull request:

The Label component originally returned a div element, which was not supported by the .pt-label selector. I modified the label.tsx file so that it returns a label element instead of a div.

Reviewers should focus on:

I have written some tests for the Label component (which were previously missing), but they are very naïve. Maybe there's more stuff that could be tested.

[palantirtech]

Thanks for your interest in palantir/blueprint, @gianmarcotoso! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[gianmarcotoso]

It's failing on the DateRangePicker, which I haven't even touched (and it's not part of the core). Is this failing on the master branch as well?

[cmslewis]

Code changes LGTM. Thanks! FYI @gianmarcotoso We ultimately omitted tests after initially writing them because of the trivial nature of the component. I'm open to including tests personally, but let's wait to hear from @adidahiya.

[cmslewis]

@adidahiya FYI these tests

[giladgray]

@gianmarcotoso what's the status of this PR? first step for you is to merge latest develop, which should resolve build issues. please close this PR if you don't have time to finish it and i'll pick up this feature (encountered this issue in my own app development 😄 )

[gianmarcotoso]

Sorry I totally forgot! I'll do it tomorrow so that hopefully we'll get this thing done! :)

…

On Mon, Jan 29, 2018, 22:58 Gilad Gray ***@***.***> wrote: @gianmarcotoso <https://github.com/gianmarcotoso> what's the status of this PR? first step for you is to merge latest develop, which *should* resolve build issues. please close this PR if you don't have time to finish it and i'll pick up this feature (encountered this issue in my own app development 😄 ) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1972 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADIFfzCFgnnNu490M8Snh4jbKamkcMzwks5tPj8VgaJpZM4RP1YO> .

[gianmarcotoso]

So, I did pull the latest develop into my branch and launched yarn verify but it now fails 2 tests on the Overlay component:

✖ does not bring focus to overlay if autoFocus=false
✖ returns focus to overlay after clicking the backdrop if enforceFocus=true

I don't think these are related to my changes, do you get them as well?

[giladgray]

@gianmarcotoso push it up! let's see if it fails on circle. those two tests are flaky, particularly when run in chrome (local) vs phantom (circle).

[gianmarcotoso]

Nope, still borked - even though only one is failing now, still on the Overlay.

[adidahiya]

Not your fault on the broken unit test -- I think it's broken on develop.

Regarding the unit tests -- yes, they are too simple. Please remove them, I don't think it's worth keeping around tautological tests that just verify the basic structure of the component.

[gianmarcotoso]

Ok, I'll remove the tests, merge develop again for good measure and push again later today (hopefully!)

[giladgray]

@gianmarcotoso still waiting on that push...

[gianmarcotoso]

Sorry, pushed right now!

Edit: and the checks have passed! 🎉

[adidahiya]

thanks @gianmarcotoso!

[gianmarcotoso]

You're welcome, thank you for Blueprint! :)