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

[Feature] Automatic validation option upon toggling multi-select prompts #121

Closed
wants to merge 1 commit into from
Closed

[Feature] Automatic validation option upon toggling multi-select prompts #121

wants to merge 1 commit into from

Conversation

GoodM4ven
Copy link

@GoodM4ven GoodM4ven commented Mar 7, 2024

  • Added an argument and a property "validateOnToggle" to MultiSelectPrompt that defaults to false
    • If set to true, validated property gets set to true upon toggleHighlight; which triggers validation
  • Wrote a test for it
    • Ensured everything passes

I found this to be cool when it's accompanied with the next PR, which is about introducing re-evaluated and cached options closure; resulting in a reactive experience... Basically, it's what I was a bit confused about in #116 .

If you like it, should I do the docs?

- Added an argument and a property "validateOnToggle" to MultiSelectPrompt that defaults to false
    - If set to true, `validated` property gets set to true upon `toggleHighlight`; which triggers validation
- Wrote a test for it
    - Ensured everything passes

I found this to be valuable and essential for the next PR, which is about introducing re-evaluated and cached options closure... Basically, what I was a bit confused aboutu in #116 .

If you like it, should I do the docs?
@taylorotwell taylorotwell requested a review from jessarcher March 7, 2024 17:55
@taylorotwell taylorotwell marked this pull request as draft March 7, 2024 17:55
@jessarcher
Copy link
Member

Hey @GoodM4ven, thanks for your PR!

I'm not sure it's worth adding an additional function parameter for this. I'd need to see some compelling use cases or multiple people experiencing an issue with the current behaviour.

Just to explain the reasoning behind the current behaviour, the delayed validation is intended to prevent an error message from being displayed before the user has had a chance to finish their input. This is primarily for text-based inputs where a user may be part-way through typing an email address. It also applies with multi-selects where the rule may be that they need to select a minimum number of options, and we don't want to display an error when they've chosen only one if they were already planning on selecting more.

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

Successfully merging this pull request may close these issues.

2 participants