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

Controls: Add max length config to text control #14396

Merged
merged 5 commits into from
Jun 30, 2022

Conversation

stevensacks
Copy link
Contributor

Issue: Added maxLength to text control and fixed two bugs with control story

What I did

  • Added a maxLength to text control and it displays the count to the right of the text area and turns red when it hits the limit.
  • Fixed issue where label was overriding children - now label bolds children
  • Fixed issue where background color was not being set

How to test

  • Is this testable with Jest or Chromatic screenshots?
    Maybe?

  • Does this need a new example in the kitchen sink apps?
    No.

  • Does this need an update to the documentation?
    Yes. The addition of maxLength to the text control should be included in the controls documentation. I'll leave it to @shilman to write it up since he'll need to do some touchup on the design anyway.

@shilman shilman changed the title feat/text control max length Controls: Add max length setting to text control Mar 30, 2021
@shilman shilman changed the title Controls: Add max length setting to text control Controls: Add max length config to text control Mar 30, 2021
@shilman shilman added this to the 6.4 PRs milestone Jul 22, 2021
@nx-cloud
Copy link

nx-cloud bot commented Aug 3, 2021

☁️ Nx Cloud Report

CI is running/has finished running commands for commit fd265db. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@MichaelArestad MichaelArestad self-assigned this Aug 16, 2021
@MichaelArestad
Copy link
Contributor

What are some specific use cases where you would need to add a max-length to an input in Controls?

Copy link
Member

@ndelangen ndelangen left a comment

Choose a reason for hiding this comment

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

This looks fine to me.. I'm sure there are use-cases for a max-length. Doesn't seem to add that much complexity to me.

@stale
Copy link

stale bot commented Jan 9, 2022

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jan 9, 2022
@stevensacks
Copy link
Contributor Author

@MichaelArestad Here’s my use case (and why I added this).

You have a text field in your component that you want to have a max length and be able to test different strings inside of it. More specifically, let’s say the business or marketing team wants to know what a certain string will look like in the component. You don’t necessarily want to ellipses overflow the text, either. You work with the designer to come up with a reasonable limit, and now the biz/mkt team can play around with the component in storybook and know what strings are too long, etc.

It’s unrealistic to expect them to count characters in those strings (which they are in all likelihood copy/pasting from somewhere else). We, as developers, know how to do it, but some of them don’t even know a browser has a console much less the JavaScript to truncate a string.

Currently, I have to add my own character counter and soft-limiter inside my actual stories where I need this because Storybook’s input doesn’t have a max length option. This means that the string in the input is longer than the string that is rendered. It’s a confusing user experience.

As @ndelangen pointed out, this is a simple change, and I think the value it adds is worth it.

@stale stale bot removed the inactive label Jan 9, 2022
@shilman shilman assigned shilman and unassigned ghengeveld and MichaelArestad Mar 28, 2022
@shilman shilman removed this from the 6.4 PRs milestone May 20, 2022
@ndelangen ndelangen merged commit b2c0b40 into storybookjs:next Jun 30, 2022
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.

5 participants