Skip to content

Close html tag immediatelly - add nested Form support#1275

Merged
mvorisek merged 19 commits intodevelopfrom
close_form_immediatelly
Jul 14, 2020
Merged

Close html tag immediatelly - add nested Form support#1275
mvorisek merged 19 commits intodevelopfrom
close_form_immediatelly

Conversation

@mvorisek
Copy link
Copy Markdown
Member

@mvorisek mvorisek commented Jun 13, 2020

Nesting html <form> 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

Atk should support AST as much as possible, this is also how some React components cope with this html limitation.

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

@mvorisek mvorisek force-pushed the close_form_immediatelly branch 2 times, most recently from b48e264 to 4ff041f Compare June 15, 2020 17:10
@mvorisek mvorisek force-pushed the close_form_immediatelly branch 2 times, most recently from a1027b8 to d6934dc Compare June 22, 2020 21:54
@mvorisek mvorisek requested a review from ibelar June 22, 2020 21:55
@mvorisek
Copy link
Copy Markdown
Member Author

mvorisek commented Jun 22, 2020

@ibelar, @georgehristov, can you please help with this?

What I have done:

  • replaced the owner <form> tag with div

  • added hidden/immediatelly closed

    tag and linked the controls to it (with form attribute)

  • now load /demos/form/form.php and compare the rendered source from this PR and develop -> this seems to be good

  • I tried to replace the JS selectors by setting the 3rd param to be $this->formElement

but it seems to not work, the the content is not reloaded properly (compare vs. expected result on the demo above on develop)

it seems like some atk issue. JQuery serialization seems to be working well with the linked (ie. not in subtree of <form>) controls...

@ibelar
Copy link
Copy Markdown
Contributor

ibelar commented Jun 23, 2020

@mvorisek - I have taken a quick look at this.

First: The HTML element set to fire the Fomantic api request is usually set to the form id. So form submit correctly but it is replacing content of the form, which in this PR has a display: none css property.

You would need to adjust this line Form::class:

 $this->js(true)
            ->api(array_merge(['url' => $cb->getJSURL(), 'method' => 'POST', 'serializeForm' => true], $this->apiConfig))
            ->form(array_merge(['inline' => true, 'on' => 'blur'], $this->formConfig));

Probably replacing with the id selector where you want to replace content.

new jQuery('#my_id')->js(true)->api()...

The other problem and I think this would be a major one, is the way Fomantic-UI is handling its form module. Especially when you need to display an error on a specific field. They have a namespace/selector mapping that you can set, but I never try and do not know if it will work. See https://fomantic-ui.com/behaviors/form.html#dom-settings

I am afraid that even if you manage to adjust field selector, their search for input is limited to the enclosed form tag, i.e. search is done inside form, not outside.

I have test manually in browser console and Fomantic throws error:

$('#atk_layout_maestro_tabs_tabssubview_form_2').form('add prompt', 'name', 'error');

@mvorisek
Copy link
Copy Markdown
Member Author

mvorisek commented Jun 24, 2020

$this->js(true)
        ->api(array_merge(['url' => $cb->getJSURL(), 'method' => 'POST', 'serializeForm' => true], $this->apiConfig))
        ->form(array_merge(['inline' => true, 'on' => 'blur'], $this->formConfig));

This was the issue - this means two things that were written together (not horibly bad, but I did not realized it, thanks)

The other problem and I think this would be a major one, is the way Fomantic-UI is handling its form module. Especially when you need to display an error on a specific field. They have a namespace/selector mapping that you can set, but I never try and do not know if it will work. See https://fomantic-ui.com/behaviors/form.html#dom-settings

I am afraid that even if you manage to adjust field selector, their search for input is limited to the enclosed form tag, i.e. search is done inside form, not outside.

It seems .form() on owning div is working great. It seems they already count with input controls not wrapped in <form>.

So current situation is:

@ibelar wdyt?

@mvorisek mvorisek force-pushed the close_form_immediatelly branch 4 times, most recently from d4fa73e to 4d58f5d Compare June 24, 2020 13:57
@mvorisek mvorisek changed the title WIP Close html tag immediatelly - add nested Form support Close html tag immediatelly - add nested Form support Jun 24, 2020
@mvorisek mvorisek force-pushed the close_form_immediatelly branch from 22fff86 to 23b9c37 Compare July 8, 2020 11:36
@mvorisek mvorisek force-pushed the close_form_immediatelly branch 2 times, most recently from 949da9f to 51c7833 Compare July 8, 2020 23:27
@mvorisek mvorisek marked this pull request as ready for review July 14, 2020 07:06
@mvorisek mvorisek added the RTM label Jul 14, 2020
@mvorisek mvorisek requested a review from ibelar July 14, 2020 07:21
Copy link
Copy Markdown
Contributor

@ibelar ibelar left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants