symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
858 stars 315 forks source link

[LiveComponent] Allow form elements to live outside the form tag by using "form" attribute #2400

Open dsoriano opened 5 days ago

dsoriano commented 5 days ago
Q A
Bug fix? yes
New feature? no
Issues
License MIT

Actually there is an issue if we want to use form elements that live outside the form tag and refer it by using the "form" attribute, i.e:

<input type="text" name="myinput" form="custom-form">
<form data-model="*" id="custom-form"></form>

this should work, especially to be able to handle nested forms (managed by others live components).

This PR fix this issue.

github-actions[bot] commented 5 days ago

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR. Please review the changes and make sure they are expected.

FileBefore (Size / Gzip)After (Size / Gzip)
LiveComponent
live_controller.js 120.36 kB / 23.38 kB 120.45 kB0% / 23.4 kB0%
smnandre commented 5 days ago

I think i see where you want to go from here, but i'd like to see it (working) and not just merge this MR that has not shown its full impact yet.

One thing i'm not sure to understand for instance: in your test you call "parseDirective"... but I'm not sure how/when it would be call in real situation?

LiveComponent only what what's inside it, Stimulus controllers too, and the multiple belongsToComponent methods (or similar) looks to me to always scope things.

Do you think you could describe a precise case and then we'll see if and how it would work ?

smnandre commented 5 days ago

(i removed the bug label because it never was a documented feature in the first place :) )

dsoriano commented 4 days ago

Hello,

the use case is relatively simple, the ability to use nested forms. In HTML this is forbidden:

<form>
    <input type="text" name="username">

    <form>
        <input type="number" placeholder="Add credits">
        <button type="submit">Add credits</button>
    </form>

    <button type="submit">Save</button>
</form>

To do that HTML standard allow to create form field outside the form tag by using the form attribute:

<input type="text" name="username" form="user-form">

<form>
    <input type="number" placeholder="Add credits">
    <button type="submit">Add credits</button>
</form>

<button type="submit" form="user-form">Save</button>
<form id="user-form"></form>

In LiveComponent, this is not possible, because updating a field outside the form tag doesn't enable the data-binding on the form because of const formElement = element.closest('form'); that constraint the element to be a child of the form.

<div {{ attributes }}>
    <input type="text" name="username" form="user-form">

    {# A nested component that contain another form tag #}
    {{ component('user_credits', {user: user}) }}

    <button type="submit" form="user-form">Save</button>
    <form id="user-form" data-model="*"></form>
</div>

There is a lot of cases where this is useful and this is the clean way to do that (I encounter this problem in my two last projects, especially in complex forms in admins for example).

My example is a good one: in the user edition, I want to see the user credits history, and want to be able to add an entry. Why not to use LiveCollection ? For two main reasons:

Now the problem with my update you pointed: I would love to do a component.getElementById(formId) to ensure the element refer a form tag embed into the component and keep the scope, but I don't have access to it in this function, that's why I used document

I hope my explanation was clear, not easy to describe 😂

I hesitated for the "bug" tag, but as LiveComponent does not take into account the entire specification of HTML in this case, I considered it was probably a bug.

I am available for further explanations if needed.

smnandre commented 3 days ago

Will answer more if i can find time this week-end.

I get the use case, and it would be great to make this work, but.

<input type="text" name="username" form="user-form">

<form>
    <input type="number" placeholder="Add credits">
    <button type="submit">Add credits</button>
</form>

<button type="submit" form="user-form">Save</button>
<form id="user-form"></form>

This is not a good example to me, as this could 100% be written

<form>
    <input type="number" placeholder="Add credits">
    <button type="submit">Add credits</button>
</form>

<form id="user-form">
    <input type="text" name="username" form="user-form">
    <button type="submit" form="user-form">Save</button>
</form>

Now, there is one constraint here to acknowledge: form fields inputs must be descendant of the the live component root tag (and must not descendant of any other nested live component root tag)

So in this situation...

<div data-controller="live" id="A">

  <form id="form-a">
      <input type="text" form="form-a" name="input-a" />
  </form>

  <input type="text" form="form-a" name="foo-a />
  <input type="text" form="form-b" name="foo-b />

  <div data-controller="live" id="B">
      <input type="text" form="form-a" name="live-a" />
       <input type="text" form="form-b" name="live-b" />

       <form id="form-b">
          <input type="text" form="form-a" name="input-c" />
      </form>
  </div>

</div>

I call "After" a future where we implement the suggested changes.

Input Form LiveComp Today After
input-a inside same
foo-a outside same
foo-a outside different
live-a outside different
live-b outside same
input-c outside different

This would need some extensive testing.. and clarity in documentation to prevent frustrations or missunderstandings. But if you think "foo-a" and "live-b" are some real improvements, why not.

Again, as long we find time to test it 100% and ensure this change nothing for existing users, nor for nested components 👍

dsoriano commented 3 days ago

Yeah I understand, indeed the goal is to ensure that the form fields are in the same component that the form tag, I agree. Actually I'm not sure this can be done easily with the actual available functions. I will try to investigate more deeply about that.

I'm sure this is a goog improvement (and necessary in some cases). That's why I defend it 😉

I will try to improve this PR as soon as possible