Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(text-field): Added support for character counter. #4244

Merged
merged 36 commits into from
Jan 31, 2019

Conversation

abhiomkar
Copy link
Collaborator

@abhiomkar abhiomkar commented Jan 11, 2019

This change adds support for character counter that displays the ratio of characters used and the total character limit.

Character counter is added as subcomponent of text field (similar to helper-text) which can be rendered inside helper-text subcomponent or inside mdc-text-field root for multi-line text field (textarea).

Single-line text field

New helper class mdc-text-field-helper-line is introduced to render two subcomponents - helper-text and character-counter in a flexbox. These two subcomponents are optional.

char-counter-demo

Multi-line text field

Character counter subcomponent can be independently used and is rendered inside mdc-text-field root as first element. Text field leaves a space at the bottom of textarea for character counter using sibling selector (.mdc-text-field-character-counter + .mdc-text-field__input) only for multi-line text field (textarea) variant.

char-counter-textarea-demo

Fixes #13.

BREAKING CHANGE: Changed the layout of MDCTextField's helper-text to add support for character counter feature.

Copy link
Contributor

@kfranqueiro kfranqueiro left a comment

Choose a reason for hiding this comment

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

I haven't gotten a chance to look at the code yet, but upon quick testing, it looks like the character counter and helper text don't line up with each other vertically.

I also sent some questions to our designer which I've CC'd you on which may have bearing on this PR.

@codecov-io
Copy link

codecov-io commented Jan 18, 2019

Codecov Report

Merging #4244 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4244      +/-   ##
==========================================
+ Coverage    98.4%   98.48%   +0.08%     
==========================================
  Files         127      130       +3     
  Lines        5688     5736      +48     
  Branches      757      766       +9     
==========================================
+ Hits         5597     5649      +52     
+ Misses         91       87       -4
Impacted Files Coverage Δ
packages/mdc-textfield/constants.js 100% <ø> (ø) ⬆️
packages/mdc-textfield/character-counter/index.js 100% <100%> (ø)
packages/mdc-textfield/helper-text/constants.js 100% <100%> (ø) ⬆️
...ages/mdc-textfield/character-counter/foundation.js 100% <100%> (ø)
packages/mdc-textfield/index.js 94.14% <100%> (+0.43%) ⬆️
...kages/mdc-textfield/character-counter/constants.js 100% <100%> (ø)
packages/mdc-textfield/foundation.js 100% <100%> (+1.01%) ⬆️
... and 2 more

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 4bc18d1...0809fed. Read the comment docs.

packages/mdc-textfield/character-counter/README.md Outdated Show resolved Hide resolved
packages/mdc-textfield/helper-text/README.md Outdated Show resolved Hide resolved
packages/mdc-textfield/character-counter/README.md Outdated Show resolved Hide resolved
packages/mdc-textfield/character-counter/README.md Outdated Show resolved Hide resolved
packages/mdc-textfield/character-counter/README.md Outdated Show resolved Hide resolved
packages/mdc-textfield/foundation.js Outdated Show resolved Hide resolved
test/unit/mdc-textfield/mdc-text-field.test.js Outdated Show resolved Hide resolved
packages/mdc-textfield/index.js Show resolved Hide resolved
packages/mdc-textfield/README.md Outdated Show resolved Hide resolved
@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

188 screenshots changed from master on commit f6ac37c:

Details

173 Changed:

15 Added:

Copy link
Contributor

@kfranqueiro kfranqueiro left a comment

Choose a reason for hiding this comment

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

LGTM with a few more doc comments

@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

158 screenshots changed from master on commit 0809fed:

Details

146 Changed:

12 Added:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants