fix(text-field): add missing prop style

[simsim0709]

TextField component is not passing otherProps, because of that we cannot use style prop.
It seems like it's your intention not passing otherProps to the root element, so I have explicitly added style prop.

[moog16]

Hmmm is the correct thing to do to actually pass ...otherProps? I think it is. It is just the wrapper component, but are there other events that this component should handle as well?

Also if we decide to just keep style on here, we will need tests.

[simsim0709]

I understand your concern, but it seems like a bit weird in that I can add className prop to root div element, but not style prop. Both className and style props are to change style.

[moog16]

Oh no I am agreeing with you. But I'm saying someone should be able to pass whatever they want including style. It could be a tabIndex or aria-foo. If so we should make your change to {...otherProps}. What do you think?

[simsim0709]

Yeah, I like that. Actually, it's better. 👍

[moog16]

Cool! Would you mind doing the changes? Also adding a test case for the style object being passed?

[simsim0709]

Yep. I will try! 😄

[moog16]

Thanks :) We appreciate the help!

[codecov-io]

Codecov Report

Merging #184 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
+ Coverage   98.78%   98.78%   +<.01%     
==========================================
  Files          21       21              
  Lines         821      824       +3     
  Branches       76       76              
==========================================
+ Hits          811      814       +3     
  Misses         10       10

Impacted Files	Coverage Δ
packages/text-field/index.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd7192d...5c5ffd1. Read the comment docs.

[simsim0709]

I just added otherProps support.
Regarding test case you mentioned "the style object being passed", is this test case is needed?
I don't think it is because it just passes rest props to div element.

[moog16]

I understand what you're saying but if we have one for classNames we should also have one for styles. In case someone wants to refactors the implementation, we'll know that styles needs to be included.

[simsim0709]

It makes sense. I will add the test! 😄

[moog16]

Thanks!

[simsim0709]

I just added the test case. I think it's enough to check if style attribute exists at the root element.

Please check this out! 😄

[moog16]

@simsim0709 sorry I dropped the ball on this one. Looking now

[moog16]

Running tests on #184

[moog16]

@simsim0709 can you also sign it for the CLA please

[moog16]

@simsim0709 I thik you might need to add isRtl to the props blacklist. Getting a warning in the screenshot tests. See image:

[simsim0709]

@moog16 . I just added isRtl prop to be ignored.
Regarding CLA, I already did sign up, though. (material-components/material-components-web#796). Should I do once again?

Thanks.

[moog16]

@simsim0709 you have to sign each PR you submit. Thanks!

[moog16]

looks good - we can merge this once the Test's PR #204 test's pass and @simsim0709 signs this PR

[simsim0709]

Hmm, @moog16
I'm not sure, what "CLAs are signed, but unable to verify author consent" means and how to handle this.

Can you give me an instruction?

Thanks.

[moog16]

@simsim0709 I'm not sure what you're referring to, but it looks like that cla/google check has passed. We are good to go.

[moog16]

#204's tests are passing - merging!

[simsim0709]

Then, no problem. I saw CI warning yesterday.

Have a good day! 👍