Skip to content
This repository was archived by the owner on Jan 14, 2025. It is now read-only.

feat(text-field): character counter #861

Merged
merged 46 commits into from Jun 3, 2019
Merged

feat(text-field): character counter #861

merged 46 commits into from Jun 3, 2019

Conversation

ghost
Copy link

@ghost ghost commented May 12, 2019

related to #817

TODO

  • Create Component
  • Create Unit Test
  • Create Screenshot Test
  • Make Test Coverage 100%
  • Write README.md

@codecov-io
Copy link

codecov-io commented May 12, 2019

Codecov Report

Merging #861 into rc0.14.0 will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           rc0.14.0     #861      +/-   ##
============================================
+ Coverage     93.99%   94.09%   +0.09%     
============================================
  Files            85       86       +1     
  Lines          3599     3643      +44     
  Branches        568      574       +6     
============================================
+ Hits           3383     3428      +45     
  Misses           90       90              
+ Partials        126      125       -1
Impacted Files Coverage Δ
packages/menu/MenuListItem.tsx 100% <ø> (ø) ⬆️
packages/list/ListItem.tsx 99.01% <ø> (ø) ⬆️
packages/text-field/character-counter/index.tsx 100% <100%> (ø)
packages/text-field/index.tsx 98.26% <100%> (+0.78%) ⬆️

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 34a7f41...963d359. Read the comment docs.

Copy link

@moog16 moog16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TroyTae Thanks for taking a stab at the char counter. Could you provide a screenshot test of some sort? Its difficult to follow what is going on without a working example.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@moog16
Copy link

moog16 commented May 16, 2019

@TroyTae is this ready for review? I checked out code and it failed to compile. Let me know when you're ready. Thanks! :)

@ghost
Copy link
Author

ghost commented May 17, 2019

@moog16
Is there compile error?
Can you show me?
I have no error and unit test is passed.

I need one help.
I don't understand about screenshot test well...
How can I fix screenshot test error?

@moog16
Copy link

moog16 commented May 17, 2019

@TroyTae looks like it is working now!

@ghost
Copy link
Author

ghost commented May 30, 2019

I think there was an error in the merge process.
Now I fix it!

@moog16 moog16 changed the base branch from rc0.13.0 to rc0.14.0 May 31, 2019 13:20
@moog16
Copy link

moog16 commented May 31, 2019

@TroyTae I updated branch to rc0.14.0. Please fix conflicts.

@ghost
Copy link
Author

ghost commented May 31, 2019

@moog16
I fixed it!

@moog16
Copy link

moog16 commented Jun 1, 2019

@TroyTae can you try creating a new branch from your rc0.13.0 branch and opening a PR? I'm not sure why your branch isn't picking up the changes in the screenshot tests. Possibly some kind of name collision could be happening? I can confirm there is a text-field/characterCounter image, so it shouldn't be be 404'ing.

@ghost
Copy link
Author

ghost commented Jun 1, 2019

@moog16
First I combined two screenshot test file.
If this error's cause is name conflict, I guess this change can solve them.

@moog16
Copy link

moog16 commented Jun 3, 2019

signed in #861 (comment)

@moog16 moog16 added cla: yes and removed cla: no labels Jun 3, 2019
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@moog16
Copy link

moog16 commented Jun 3, 2019

@TroyTae please run npm run fix and fix all prettier/lint warnings. Then we're good to go!

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Jun 3, 2019
@ghost
Copy link
Author

ghost commented Jun 3, 2019

@moog16
I didn’t know about npm run fix.
So I found each one and fixed it 😂
Anyway I fixed it!

@moog16
Copy link

moog16 commented Jun 3, 2019

Thanks!

@moog16 moog16 added cla: yes and removed cla: no labels Jun 3, 2019
@codecov-io
Copy link

codecov-io commented Jun 3, 2019

Codecov Report

Merging #861 into rc0.14.0 will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           rc0.14.0     #861      +/-   ##
============================================
+ Coverage     93.99%   94.09%   +0.09%     
============================================
  Files            85       86       +1     
  Lines          3599     3643      +44     
  Branches        568      574       +6     
============================================
+ Hits           3383     3428      +45     
  Misses           90       90              
+ Partials        126      125       -1
Impacted Files Coverage Δ
packages/menu/MenuListItem.tsx 100% <ø> (ø) ⬆️
packages/list/ListItem.tsx 99.01% <ø> (ø) ⬆️
packages/text-field/character-counter/index.tsx 100% <100%> (ø)
packages/text-field/index.tsx 98.26% <100%> (+0.78%) ⬆️

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 34a7f41...963d359. Read the comment docs.

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@moog16 moog16 merged commit 862e04e into material-components:rc0.14.0 Jun 3, 2019
@moog16 moog16 mentioned this pull request Jun 3, 2019
moog16 pushed a commit that referenced this pull request Jun 11, 2019
moog16 pushed a commit that referenced this pull request Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants