feat(react): added counter to textinput and storybook updates #12139
Conversation
| it('should not render counter with only enableCounter prop passed in', () => { | ||
| expect( | ||
| counterTestWrapper1.exists(`${prefix}--text-area__counter`) | ||
| ).toEqual(false); | ||
| }); | ||
|
|
||
| it('should not render element without only maxCount prop passed in', () => { | ||
| it('should not render counter with only maxCount prop passed in', () => { |
| <TextInput | ||
| {...args} | ||
| labelText="Text input label" | ||
| helperText="Optional help text" | ||
| id="text-input-1" | ||
| type="text" | ||
| /> |
There was a problem hiding this comment.
Added playground storybook to test
There was a problem hiding this comment.
Nice, thanks for adding that in @TannerS! We'll need to define the args somewhere in the storybook; take a look at FluidTextInput for an example:
Playground.argTypes = {
placeholder: {
control: {
type: 'text',
},
defaultValue: 'Placeholder text',
}
...
...
...
};There are some props that don't apply to FluidTextInput but do to TextInput, so it may not be a complete list. I believe you can copy most of these over to the TextArea story as well
✅ Deploy Preview for carbon-components-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
tw15egan
requested review from
a team and
laurenmrice
and removed request for
a team
|
Thanks for taking the time to contribute this, Tanner! Just had a few comments.
Everything else looks great so far 💪🏻 |
right now stopping at the limit is the current way textarea works. originally it was an idea but bouncing around the design with @abbeyhrt at the time, we thought using the native html input's/textarea's
not sure what you mean but if you want this change ill gladly add it ;D
in response to this comment, #12139 (comment), ill make the change but i dont understand the change lol, playground works as is, what does the thank you |
|
Makes sense, if it's mirroring
Just wondering if the counter should align to the bottom of the container, cc @carbon-design-system/design
I'm seeing them all show up, but some aren't working properly. I am not able to change |
I will do the fixes, although i may wait to hear back from @carbon-design-system/design |
For sake of examples here is both
@tw15egan please see this and the above comment, i went ahead with this change until told otherwise :D |
for the latest commit, everything is done but i didnt do this to the password inout unless told too, the second point was somethign design wanted to discuss eventually but for now mirroring text area until both change |
…n into tanner_addCounterTextInput
There was a problem hiding this comment.
Hi @TannerS sorry for the late review. 👋
As @tw15egan mentioned above, yes, we want to align the counter in the bottom right above the input when the label wraps to multiple lines. Also yes we want to have the same functionality for text area and text input when adding the counter for right now. I also agree we do not want to have a counter on the password variant of text input.
Eventually, the design team may look into both of these counter designs for text area/text input and do some more research and design iterations. Looks good to me!
tw15egan
added
status: ready to merge 🎉
and removed
status: ready for review 👀
status: visual review 🔍
labels
Closes #
#11179
#1672
#5702
but it has been said this code change should be fine
CC: @aledavila
Changelog
New
Changed
Testing / Reviewing