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 input and textarea character limit counter #2745

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented May 15, 2019

Closes #1672

This PR adds a character counter for <TextInput> and <TextArea> components in both React and vanilla Carbon. The character counter markup can be customized with the renderCharCount render prop

Changelog

New

  • <TextInput> and <TextArea> character counter

Testing / Reviewing

Ensure the text inputs and textareas function as expected without any regressions (with and without character counters)

@netlify
Copy link

netlify bot commented May 15, 2019

Deploy preview for the-carbon-components ready!

Built with commit f38e21f

https://deploy-preview-2745--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented May 15, 2019

Deploy preview for carbon-components-react ready!

Built with commit f38e21f

https://deploy-preview-2745--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented May 15, 2019

Deploy preview for the-carbon-components ready!

Built with commit 6690dcd

https://deploy-preview-2745--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented May 15, 2019

Deploy preview for carbon-components-react ready!

Built with commit 6690dcd

https://deploy-preview-2745--carbon-components-react.netlify.com

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @emyarod! One question BTW - Can vanilla text area be used without JavaScript if character counter is not there?

@emyarod
Copy link
Member Author

emyarod commented May 15, 2019

@asudoh yes textarea without character counter can be used without JS

also not sure why CI output differs from running vanilla test suite locally

@asudoh
Copy link
Contributor

asudoh commented May 15, 2019

Cool, BTW the unit test issue seems with FF - Would you want to try it locally (--browser Firefox) @emyarod?

@emyarod
Copy link
Member Author

emyarod commented May 15, 2019

no errors when testing locally on Firefox Nightly (Firefox 68) @asudoh. on Firefox stable (Firefox 66) though, I get the same errors and the spec files are not accessible through dev tools sources either

@asudoh
Copy link
Contributor

asudoh commented May 15, 2019

@emyarod I think you can put a debugger statement - Browser debuggers tend to show source as it breaks.

@emyarod emyarod force-pushed the 1672-text-input-character-limit-counter branch 3 times, most recently from a5481ba to 34e290d Compare May 15, 2019 23:31
@emyarod
Copy link
Member Author

emyarod commented May 15, 2019

@asudoh it appears that in FF66 InputEvent object data was not being assigned despite being in the InputEventInitdictionary

34e290d

shixiedesign
shixiedesign previously approved these changes May 16, 2019
Copy link
Contributor

@shixiedesign shixiedesign left a comment

Choose a reason for hiding this comment

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

This got done FAST! 🚤 A few small styling fixes:

  • Label got an extra margin bottom when Char count knob is turned on
    May-16-2019 12-52-39

  • When label text is long, warp to second line before intersecting with Char count. There should be a 16px gap between label and Char count text.
    image

  • When label is hidden, long helper text should push charCount out, but not this far. Should be only a 16px gap between helper text and charCount.
    image

  • CharCount knob visible for Password Input but doesn't do anything. Maybe password input shouldn't have this knob in the first place.
    image

Copy link
Contributor

@shixiedesign shixiedesign left a comment

Choose a reason for hiding this comment

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

For text area, a lot of the similar issues:

  • Extra margin bottom on label when charCount is turned on
    May-16-2019 13-00-50

  • Long label intersecting charCount
    image

  • Large gap between helper text and charCount when label hidden
    image

@emyarod emyarod force-pushed the 1672-text-input-character-limit-counter branch 2 times, most recently from fc3d300 to 5d044bb Compare May 17, 2019 21:26
@emyarod emyarod requested a review from shixiedesign May 17, 2019 21:26
@emyarod
Copy link
Member Author

emyarod commented May 21, 2019

@shixiedesign regarding these points:

Helper text currently occupies 75% of full width. This is not specified in the original design. I think we should allow helper text to take up full width of the text area width, and allow 16px margin right when there is a charCount.

Distance between helper text and charCount looks like a percentage. Should just be a fixed 1 rem padding-right.

the 75% max width was the result of a previous ticket, do we want to modify that? #1997 #2000

@emyarod emyarod force-pushed the 1672-text-input-character-limit-counter branch from 1dbc2c2 to 3aa014c Compare May 21, 2019 13:53
@shixiedesign
Copy link
Contributor

shixiedesign commented May 21, 2019

@emyarod Thanks double checking! I see, that make sense then. I will strike out that item.

Copy link
Contributor

@shixiedesign shixiedesign left a comment

Choose a reason for hiding this comment

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

Love it! 🎉 Great work. Thank you!

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 - Thanks @emyarod!

@asudoh asudoh merged commit 919ba2e into carbon-design-system:master May 22, 2019
@emyarod emyarod deleted the 1672-text-input-character-limit-counter branch May 22, 2019 14:38
emyarod added a commit that referenced this pull request Jun 18, 2019
asudoh pushed a commit that referenced this pull request Jun 20, 2019
…l state" (#3106)

* Revert "fix: pass `defaultValue` in to textarea and text input initial state (#3015)"

This reverts commit d563edf.

* Revert "feat: text input and textarea character limit counter (#2745)"

This reverts commit 919ba2e.
cal-smith pushed a commit to cal-smith/carbon-components that referenced this pull request Jun 21, 2019
…l state" (carbon-design-system#3106)

* Revert "fix: pass `defaultValue` in to textarea and text input initial state (carbon-design-system#3015)"

This reverts commit d563edf.

* Revert "feat: text input and textarea character limit counter (carbon-design-system#2745)"

This reverts commit 919ba2e.
cal-smith pushed a commit to cal-smith/carbon-components that referenced this pull request Jun 21, 2019
…l state" (carbon-design-system#3106)

* Revert "fix: pass `defaultValue` in to textarea and text input initial state (carbon-design-system#3015)"

This reverts commit d563edf.

* Revert "feat: text input and textarea character limit counter (carbon-design-system#2745)"

This reverts commit 919ba2e.
cal-smith pushed a commit to cal-smith/carbon-components that referenced this pull request Jun 21, 2019
…l state" (carbon-design-system#3106)

* Revert "fix: pass `defaultValue` in to textarea and text input initial state (carbon-design-system#3015)"

This reverts commit d563edf.

* Revert "feat: text input and textarea character limit counter (carbon-design-system#2745)"

This reverts commit 919ba2e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Text input] Add character limit/counter
3 participants