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

[form] Fix nested form support natively #2792

Open
mvorisek opened this issue May 21, 2023 · 3 comments
Open

[form] Fix nested form support natively #2792

mvorisek opened this issue May 21, 2023 · 3 comments
Labels
type/feat Any feature requests or improvements

Comments

@mvorisek
Copy link
Contributor

mvorisek commented May 21, 2023

Feature Request

(originally posted in #2791, that issue was based on mixed (on owning div / form tag directly) .form() usage, this issue is feature request to fix the support natively and allow .form() to be used on the owning div with (self closed/empty) form tag inside)

Nested forms theory and current Fomantic-UI impl.

please see atk4/ui#1275

Nesting html

tags is not possible, hovewer they can be closed immediatelly and linked with the input controls/tags, see https://stackoverflow.com/questions/3430214/form-inside-a-form-is-that-alright#21900324

forms spec: https://www.w3.org/TR/html52/sec-forms.html

In atk4/ui all html forms code look like <div id="fff"><form id="fff_form"></form><input form="fff_form"> to overcome the form tag limitation and allow form nesting. This is the only possible solution per html forms spec.

In atk4/ui I tried to fix all JS .form() calls to be done on form tag ($('fff_form').form( instead of $('fff').form() but this is not working with Fomantic-UI as well, as:

a) success class is not added to the owning form div (with id fff) causing all success messages to be hidden
b) $('fff_form').form('add prompt', ...) errors with Form: Field identifier email1 not found

So calling .form() on a form tag is not a solution either.

Looking into https://github.com/fomantic/Fomantic-UI/blob/develop/src/definitions/behaviors/form.js code Fomantic-UI simply expects all form components to be a contained in the module element.

Current atk4/ui impl./usage

In atk4/ui#1275 I found some sweet spot to do some .form() calls on the owning div and some on the form tag directly:

This is obviously a hack. As Fomantic-UI module expects the module to be a parent of all form subviews, I propose to fix the nested forms (via official/W3 immediately self closing form tag) support by:

a) allow to specify the form element selector explicitly
b) if the selector is not specified, locate&use the first form tag relative/within the module
c) if the explicitly passed element is not a form tag or no form tag was found, emit an error

@mvorisek mvorisek added state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/feat Any feature requests or improvements labels May 21, 2023
@mvorisek
Copy link
Contributor Author

mvorisek commented May 21, 2023

Here is example of not working submit - https://jsfiddle.net/ebnvam9r/

Here are all atk4/ui usages (old changes in the commit) that require explicit form tag currently - atk4/ui@b239458.

And failing example with nested form with duplicate names: https://jsfiddle.net/kauq125z/

@mvorisek
Copy link
Contributor Author

mvorisek commented May 21, 2023

api behaviour module

https://github.com/atk4/ui/blob/97515e9719/src/Form.php#L551 (form api processing on the form tag directly)

-$form = $module.closest(selector.form)
+$form = $module.find('> form').addBack().closest(selector.form)

Would fix it, but api behaviour module works on form/inputs per html spec, ie. not on ui selectors, thus api behaviour module does probably not need any change as data are retrived by formData = new FormData($form[0]) which impl. W3C standard which support linked inputs with immediately self closing form tag natively.

form behaviour module

This module does work primary on *.ui.form and *.ui.field DOM nodes vs. native form/input as api behaviour module does.

Currently the module does not resolve native form tag in any way.

If I understand the current impl. correctly - https://github.com/fomantic/Fomantic-UI/blob/afea456335/src/definitions/behaviors/form.js#L160, the submit event is currently working together with the native form events by coincidence when the module element is also a form tag.

⏩⏩ In order to work with nested forms, it must ignore any fields of inner form, ie, any DOM subtrees starting with .ui.form class must be ignored. Interaction with sub forms is probably not desired by default.


And if integration of submit/reset with the native form is desired, the module must:

Resolve the native form tag, at least by the most strict selector: module.find('> form').addBack()[0].

And listen on native form tag events, per https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#events there are 3 native events - formData, submit, reset. I would expect corresponding FUI event fired and formData can be probably ignored or validate fired.

@mvorisek
Copy link
Contributor Author

mvorisek commented May 22, 2023

The events can be fixed by:

$form = $('#div.ui.form');
$form.on('submit', function (event) {
    if (event.target === this) {
        event.preventDefault();
        event.stopImmediatePropagation();
        $form.find('> form').first().trigger('submit');
};
});

The JS for nested forms can be fixed by:

in https://github.com/fomantic/Fomantic-UI/blob/afea456335/src/definitions/behaviors/form.js#L119

-$field = $module.find(selector.field);
+$field = $module.find(selector.field).filter((i, elem) => $(elem).parentsUntil($module, '.ui.form').length === 0);

of course this needs to be fixed for every method that search the form/$module subtree.

CSS must be fixed to reset some classes once inner .ui.form is found:

demo: https://jsfiddle.net/kauq125z/1/ (with form.js copied with the diff above applied)

@lubber-de lubber-de removed the state/awaiting-triage Any issues or pull requests which haven't yet been triaged label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feat Any feature requests or improvements
Projects
None yet
Development

No branches or pull requests

2 participants