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.

Sign up for GitHub

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

Jump to bottom

required() has no effect for SingleSelectionFormField #5265

Closed
TimWolla opened this issue Feb 1, 2023 · 3 comments · Fixed by #5470
Closed

required() has no effect for SingleSelectionFormField #5265

TimWolla opened this issue Feb 1, 2023 · 3 comments · Fixed by #5470
Assignees
@BurntimeX
Labels
Bug

Comments

@TimWolla
Copy link
Member

TimWolla commented Feb 1, 2023

see https://github.com/WoltLab/WCF/pull/5252/files?diff=split&w=0#r1093048802
see #4789
@TimWolla TimWolla added the Bug label Feb 1, 2023
@TimWolla TimWolla mentioned this issue Mar 8, 2023
Closed
@BurntimeX BurntimeX self-assigned this Apr 28, 2023
@BurntimeX BurntimeX mentioned this issue Apr 28, 2023
Closed
@TimWolla
Copy link
Member Author

TimWolla commented May 2, 2023 •
edited
Loading

The linked comment from #5252 said:

Completely deleting the <select> from the HTML will result in a null return even if the field is required(), so this is definitely is a bug there. It appears that support for required() is not implemented at all. I'll file an issue for this.

But this is not actually true, or rather, it's misleading: This is only possible if '' is an option that is offered, because the implicit null will result in an empty string when used as an array key. If the empty string is not a valid value, then the deleted field will be rejected with the expected error message. So the bug is not so much about ->required(), but rather the nullability.

@TimWolla
Copy link
Member Author

TimWolla commented May 2, 2023

It appears there is a bug with GitHub's markdown rendering. I appear to be unable to end the quote.

@TimWolla
Copy link
Member Author

TimWolla commented May 2, 2023

Ah, it's because of the bare <select>.

BurntimeX added a commit that referenced this issue May 3, 2023
@BurntimeX
Add a dedicated form field for plain select fields
0cb2234 
The existing `SingleSelectionFormField` has multiple design flaws that are difficult to solve in a backward compatible way. The main issue with the existing implementation is that it tries to solve too many problems at once, creating an API that is inconsistent and difficult to use / easy to misuse.

This is the first implementation with more to follow that will eventually allow us to phase out the `SingleSelectionFormField`.

Closes #5265
Closes #4789
@BurntimeX BurntimeX mentioned this issue May 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BurntimeX BurntimeX

Labels
Bug
Projects
None yet
Milestone
No milestone
2 participants
@TimWolla @BurntimeX