Skip to content

Conversation

@malayladu
Copy link
Contributor

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/2867998139/78938

Summary

Min Max character limit validation make sense when the field has a value. If we want to make sure field has a value, that can be handled through field Required rules.

So, this we can do min max character limit validation only for the filed that has a value.

@malayladu malayladu self-assigned this Mar 7, 2025
@github-actions
Copy link

github-actions bot commented Mar 7, 2025

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against 5c27332

Copy link
Contributor

@saifsultanc saifsultanc left a comment

Choose a reason for hiding this comment

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

Working well!

This does the job as David emphasized, "I think we can ignore the required status and only apply our validation if the value is not empty."

However, I do wonder if we can make it more versatile by adding a boolean flag, say:

'ignore_if_empty' => true

Maybe a future improvement scenario for the snippet.

For now, LGTM!

@spivurno
Copy link
Contributor

spivurno commented Mar 8, 2025

@saifsultanc Love where your head is at!

But I think this is probably best without an additional parameter since this is arguably how it should have been working all along. If this were a plugin, we'd need to be more considerate of backwards compatibility for folks who may have been using this min/max validation (without having marked the field as required) but fortunately, it's a snippet. 😄

@saifsultanc
Copy link
Contributor

@malayladu S&M this. Thanks! 🙌

@malayladu malayladu merged commit c5aa23d into master Mar 12, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants