-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
[LiveComponent] Dependent form fields logic broken when submitting the form #2240
Comments
Let's keep the "required" and there is no problem.. you should not be able to submit without selecting a value. (try without |
Seems not a solution to me :) the real problem is submitting the "old" value to the form. And problem is even worst when one of the meal doesn't include any option. |
This is the only "solution" i have .... as I don't see any bug on the demo. Please look at the following video. dependent-form-fields.mov |
So @smnandre we rely on client side validation to make the form "work"? And you are not considering one thing: meal could be optional, so required would not help here. Should I provide a more real example? Like the one explained here: SymfonyCasts/dynamic-forms#35 |
-- friendly tone here 😃 -- i just spent 15 minutes to understand your problem, replicate it, point were you missed something, record and post a video to show you the demo works exactly how it is supposed to. And now you politely suggest me to look at another exemple, with another code. I'm sorry.. i don't have that energy right now ;) I'll check tomorrow or monday the other issue. |
No problem, @smnandre. I really appreciate your work and time to look at this issue. My tone was meant to be "normal", I guess, but not being fully understood is frustrating 🥲 I understand that the example "works" thanks to the required attribute and client side javascript. Tomorrow I'll update my example to show that, in some situation, neither the required attribute will help. |
I know ;) |
@smnandre I’ve updated the question and the repository, trying to explain myself as clearly as possible. I’m not sure if I’m just unlucky 😢 , if no one has ever noticed this issue, or if there’s something wrong with the version of the libraries I have installed. Personally, I believe no one has noticed this behavior before because, as you likely mentioned, they use the required attribute, which in my case should not be specified (since it must be possible to perform the search with empty values). |
For the record: the discussion continued acrosss repositories (😅 ) but point is: there is some case not fully covered / implemented in the DependentFormField here... Let's see if we can suggest something :) |
To summarize what's mentioned in the repository and the excellent research by @smnandre : there is definitely an issue with the library, I quote the source code: // A fake hidden field where we can "store" an error if a dependent form
// field is suddenly invalid because its previous data was invalid
// and a field it depends on just changed (e.g. user selected "Michigan"
// as a state, then the user changed "Country" from "USA" to "Mexico"
// and so now "Michigan" is invalid). In this case, we clear the error
// on the actual field, but store a "fake" error here, which won't be
// rendered, but will prevent the form from being valid. The last statement seems odd since, in fact, the form becomes invalid in any case. But @smnandre , let's pretend we're not using the Dependant Fields library, because in my opinion, the error is more of a logical one and lies in how the data submission is handled (in the context of live components). Here's a 'vanilla' example: public function buildForm(FormBuilderInterface $builder, array $options): void
{
$builder
->add('office', EnumType::class, [
'class' => Office::class,
'choice_label' => fn (Office $office): string => $office->getReadable(),
'placeholder' => 'Any office',
'required' => false,
])
;
$formModifier = function (FormInterface $form, ?Office $office): void {
$form->add('employee', EnumType::class, [
'class' => Employee::class,
'placeholder' => 'Any employee',
'choices' => $office?->getEmployeeChoices(),
'choice_label' => fn (Employee $employee): string => $employee->getReadable(),
'required' => false,
]);
};
$builder->addEventListener(
FormEvents::PRE_SET_DATA,
function (FormEvent $event) use ($formModifier): void {
$formModifier($event->getForm(), $event->getData()?->office);
}
);
$builder->get('office')->addEventListener(
FormEvents::POST_SUBMIT,
function (FormEvent $event) use ($formModifier): void {
$formModifier($event->getForm()->getParent(), $event->getForm()->getData());
}
);
} What happens now? The errors are shown. But look what happens when:
We are now basically stuck. We can't go even back and select "Any office" and "Any employee", without a full page refresh. |
Let's imagine all is easy and possible. So regarding the best user-experience in this scenario.. A) set Employee to "null" I personnaly would opt for A or B... what about you ? |
I do agree this is a weird situation, and we'll try to find a good solution, but you cannot compare to anything "not live" because.... the So more than a submit problem, it's a "invalid state that should not happen" in a way |
Option 1, set to null, without a doubt. If, for some reason, field B (which depends on A) is required, the validation stack will kick in, right? In my case, setting the employee to null is fine, allowing the user to submit the form and search for 'all employees that belong to office 3'.
You mean in a non-live scenario, right? Of course, yes. If we submit the form the traditional way and update the employee field accordingly, this wouldn’t be an issue. But in this case, the form updates as soon as you select 'office 3,' and then everything gets stuck. I'm feeling a bit discouraged, honestly 😢 I don't believe there's a possibility to resolve everything within the ComponentWithFormTrait. I'm afraid I might need to write custom JavaScript for every similar situation (and work on setting field B every time field A changes). |
I know i know ... 😅
This is where i'll personnaly drop entirely formtrait and dependantformfieldd to just manually use 2 props 🤷 Again, DependantFormField is not an UX package currently, and this use case is legit but still very rare... and what you would expect here would prevent another one to get what they expected. Probably stupid question, is it possible to make both fields dependant on ... the other one ?
No JS, just live component, mount, and onUpdate :) I really need to do other things today, but if you're stuck tomorrow i can maybe show you an example :) |
Oh, i'm 100% certain there is none.... but this does not mean LiveComponent is not a good choice (in this very specific case, not saying anything else ;) ) |
Minimal repro: https://github.com/gremo/github-issue-2240
I'm implementing a search form for employee work hours. The form should allow the following options: no filtering at all, filtering by office only, filtering by employee only and iltering by both office and employee.
Therefore, the model has no strict constraints, and the form fields are optional. The form works correctly if no filters are selected, or if either the office or the employee is selected, or if the office is selected first followed by the associated employee.
When do problems occur? They happen when, after selecting the office and employee, the user decides to change the office.
Office 1 -> Employee 1 -> Office 2 -> Submit
In this case (scenario 1) the selected employee is no longer part of the user list for the new office, but the previous value is still sent with the form (because form values prop is a
LiveProp
), so the form is invalid (an unknown value is present).A more critical issue (scenario 2) occurs when the office has no associated employees:
Office 1 -> Employee 1 -> Office 3 -> Submit
In this case, there’s no way to make the form valid and processable, even by selecting "Any employee." While this would understandably result in zero results, that's not the main concern.
In both cases there's no feedback provided to the user, no errors, no one knows what's happening.
I genuinely believe this is an issue that needs to be addressed. Personally, in my case, I don't see any solution, not even a workaround, but I still need to try something with JavaScript. Ideally, if field B depends on A to function, then when A changes, B should always be reset in some way. Otherwise, as described, the old value of B, which makes the form invalid, will continue to be sent to the server.
View old question here
I think there is a very obvious issue with the dependant form fields UX feature, take the meal example.
Let's add the logic for handing the form submission:
Change also the form action and add a submit button:
Now let's use the form:
Until now the form re-renders correcly with HTTP 200 status code. No validation errors are shown.
Now submit the form pressing the "save" button:
Here is a short video of the steps:
I opened a similar issue SymfonyCasts/dynamic-forms#35 but I think that problem is not related to the library itself.
The text was updated successfully, but these errors were encountered: