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(form): deprecated and rename empty to notEmpty rule #2949

Merged
merged 4 commits into from
Dec 2, 2023

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Nov 30, 2023

Description

This PR renames the empty rule to notEmpy but keeps the empty rule working for backward compatibility, so people can just upgrade their code for while. A console warning will tell them about.
Reasons for renaming is that the code actually checks if a field is not empty. More explanation inside the related issue

Closes

#2937

@lubber-de lubber-de added type/feat Any feature requests or improvements state/awaiting-reviews Pull requests which are waiting for reviews javascript Pull requests that update Javascript code labels Nov 30, 2023
@lubber-de lubber-de added this to the 2.9.4 milestone Nov 30, 2023
prudho
prudho previously approved these changes Dec 1, 2023
Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

LGTM

@bedengler
Copy link

Does that mean in future versions, if we are looking for en empty entry, we have to say "not[notEmpty]"?
I'd prefer keeping empty, but doing exactly what it means and if we look for not Empty, we just do a "not[empty]"....

@mvorisek
Copy link
Contributor

mvorisek commented Dec 1, 2023

Does that mean in future versions, if we are looking for en empty entry, we have to say "not[notEmpty]"? I'd prefer keeping empty, but doing exactly what it means and if we look for not Empty, we just do a "not[empty]"....

I would be fine with required, but inverting a meaning would be the worst thing possible, as not all devs read all changelogs.

@lubber-de
Copy link
Member Author

Infact the not rule does not even work this way as it only accepts a string for comparison instead of another rulename.

not[empty] would currently check if the input value String isn't equal to the string "empty"

Of course we could implement supporting this, but In case someone really wants to check for a string "empty" and a rule with the same name exists would then break/ not work again. It might als isn't trivial as we would need to support something like not[max[6]] where you could simply do min[7] instead.

So existing code simply needs to be updated from empty to notEmpty.

@bedengler
Copy link

Interesting and weird.

How do you check for an empty entry then? Using "min[1]"?

What about implementing a "notrule" or some characters that signal, we are looking not for a string, but another rule?
Something like "not[empty]" looking for the string "empty" and "not[{notEmpty}]" or "notrule[notEmpty]"?

@lubber-de
Copy link
Member Author

How do you check for an empty entry then? Using "min[1]"?

You can use

maxLength[0] or is[] to accomplish this

What about implementing a "notrule" or some characters that signal, we are looking not for a string, but another rule? Something like "not[empty]" looking for the string "empty" and "not[{notEmpty}]" or "notrule[notEmpty]"?

TBH, this gets complicated and all the existing rules have opposite versions already to accomplish such checks.
In doubt you can always create a custom rule

@lubber-de lubber-de added the state/awaiting-docs Pull requests which need doc changes/additions label Dec 1, 2023
@lubber-de lubber-de requested review from prudho and mvorisek December 1, 2023 23:50
@lubber-de lubber-de merged commit 0224737 into fomantic:develop Dec 2, 2023
9 checks passed
@lubber-de lubber-de deleted the nonEmptyRename branch December 2, 2023 21:49
@lubber-de lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Dec 2, 2023
@lubber-de
Copy link
Member Author

Docs added by fomantic/Fomantic-UI-Docs#483

@lubber-de lubber-de removed the state/awaiting-docs Pull requests which need doc changes/additions label Dec 18, 2023
@lubber-de lubber-de added the state/has-docs A issue/PR which requires documentation changes and has the corresponding PR open in the docs repo label Dec 18, 2023
lubber-de added a commit that referenced this pull request Dec 19, 2023
lubber-de added a commit to lubber-de/Fomantic-UI that referenced this pull request Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code state/has-docs A issue/PR which requires documentation changes and has the corresponding PR open in the docs repo type/feat Any feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants