fix(forms): don't prevent default behavior for forms with method="dialog" #47308

crisbeto · 2022-08-31T12:08:08Z

The forms submit event handlers have a return false to prevent form submissions from reloading the page, however this also prevents the browser behavior for forms with method="dialog".

These changes add an exception since the method="dialog" doesn't refresh the page.

Fixes #47150.

packages/forms/src/directives/reactive_directives/form_group_directive.ts

packages/forms/src/directives/ng_form.ts

AndrewKushnir

Reviewed-for: public-api

crisbeto · 2022-09-01T13:47:17Z

I ended up with the event.target workaround that we talked about. These are previous approaches and why they didn't work:

We can't inject the ElementRef using the constructor in order to figure out the method, because there are a bunch of internal tests that construct the NgForm and FormNameDirective classes manually.
We can't use the inject function to inject the ElementRef due to the aforementioned issue where internal targets instantiate the directives manually which means that there won't be an injection context.
We can't use an Input to get the method of the form element, because the method input also has to be forwarded back to the method attribute on the host in order for the form to work correctly. The directive selectors allow for them to be set on an ng-container which will throw an error if we try to bind to an attribute.

AndrewKushnir

👍

dylhunn

reviewed-for: fw-core, fw-forms

pkozlowski-opensource

LGTM

AndrewKushnir · 2022-09-07T05:37:37Z

Global Presubmit.

crisbeto · 2022-09-07T13:16:06Z

It looks like some targets failed internally because onSubmit was being invoked without an event object. I've pushed a change to work around it.

AndrewKushnir · 2022-09-07T18:04:14Z

@crisbeto thanks for the fix 👍
Could you please also add a quick comment around these lines to provide more info on why $event and $event.target might be undefined?

return ($event?.target as HTMLFormElement | null)?.method === 'dialog';

…log" The forms `submit` event handlers have a `return false` to prevent form submissions from reloading the page, however this also prevents the browser behavior for forms with `method="dialog"`. These changes add an exception since the `method="dialog"` doesn't refresh the page. Fixes angular#47150.

crisbeto · 2022-09-08T03:11:55Z

Comment has been added.

AndrewKushnir · 2022-09-08T04:38:37Z

Global Presubmit #2.

dylhunn · 2022-09-09T21:26:51Z

This PR was merged into the repository by commit 4a13210.

…log" (#47308) The forms `submit` event handlers have a `return false` to prevent form submissions from reloading the page, however this also prevents the browser behavior for forms with `method="dialog"`. These changes add an exception since the `method="dialog"` doesn't refresh the page. Fixes #47150. PR Close #47308

…log" (angular#47308) The forms `submit` event handlers have a `return false` to prevent form submissions from reloading the page, however this also prevents the browser behavior for forms with `method="dialog"`. These changes add an exception since the `method="dialog"` doesn't refresh the page. Fixes angular#47150. PR Close angular#47308

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fanimations/14.2.1/14.2.2) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcommon/14.2.1/14.2.2) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/14.2.1/14.2.2) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/14.2.1/14.2.2) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcore/14.2.1/14.2.2) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fforms/14.2.1/14.2.2) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/14.2.1/14.2.2) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`14.2.1` -> `14.2.2`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/14.2.1/14.2.2) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v14.2.2`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1422-2022-09-14) [Compare Source](angular/angular@14.2.1...14.2.2) ##### animations | Commit | Type | Description | | -- | -- | -- | | [937e6c5b3d](angular/angular@937e6c5) | fix | make sure that the animation function delay is applied ([#47285](angular/angular#47285)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [c9bdf9bab1](angular/angular@c9bdf9b) | fix | rename `rawSrc` -> `ngSrc` in NgOptimizedImage directive ([#47362](angular/angular#47362)) ([#47396](angular/angular#47396)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [a3e1303f04](angular/angular@a3e1303) | fix | imply [@Optional](https://github.com/Optional) flag when a default value is provided ([#47242](angular/angular#47242)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [80c66a1e57](angular/angular@80c66a1) | fix | don't prevent default behavior for forms with method="dialog" ([#47308](angular/angular#47308)) | #### Special Thanks Abhishek Rawat, Andrew Kushnir, Benjamin Chanudet, Bob Watson, George Kalpakas, Ikko Ashimine, Kristiyan Kostadinov, Marc Wrobel, Mariia Subkov, Pawel Kozlowski, Sebastian, abergquist, dario-piotrowicz, onrails and vyom1611  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1548 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2022-10-10T16:08:49Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto commented Aug 31, 2022

View reviewed changes

packages/forms/src/directives/reactive_directives/form_group_directive.ts Outdated Show resolved Hide resolved

crisbeto marked this pull request as ready for review August 31, 2022 12:25

pullapprove bot requested a review from AndrewKushnir August 31, 2022 12:25

crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: forms target: patch This PR is targeted for the next patch release labels Aug 31, 2022

ngbot bot modified the milestone: Backlog Aug 31, 2022

AndrewKushnir reviewed Aug 31, 2022

View reviewed changes

packages/forms/src/directives/ng_form.ts Outdated Show resolved Hide resolved

AndrewKushnir approved these changes Aug 31, 2022

View reviewed changes

packages/forms/src/directives/ng_form.ts Outdated Show resolved Hide resolved

pullapprove bot requested review from AndrewKushnir, jessicajaniuk and pkozlowski-opensource August 31, 2022 17:41

AndrewKushnir approved these changes Aug 31, 2022

View reviewed changes

pullapprove bot requested a review from alxhub August 31, 2022 17:41

crisbeto force-pushed the 47150/form-native-dialog branch 4 times, most recently from 5524cfa to 4b00782 Compare August 31, 2022 23:24

AndrewKushnir self-requested a review September 1, 2022 01:16

crisbeto force-pushed the 47150/form-native-dialog branch 3 times, most recently from 05691cf to cdd95f3 Compare September 1, 2022 12:12

AndrewKushnir approved these changes Sep 1, 2022

View reviewed changes

dylhunn self-requested a review September 6, 2022 18:56

dylhunn approved these changes Sep 6, 2022

View reviewed changes

AndrewKushnir added action: global presubmit The PR is in need of a google3 global presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 6, 2022

pkozlowski-opensource approved these changes Sep 6, 2022

View reviewed changes

crisbeto force-pushed the 47150/form-native-dialog branch from cdd95f3 to ef50952 Compare September 7, 2022 12:55

crisbeto force-pushed the 47150/form-native-dialog branch from ef50952 to bde2354 Compare September 8, 2022 03:11

jessicajaniuk removed request for alxhub and jessicajaniuk September 8, 2022 15:44

AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Sep 8, 2022

dylhunn closed this in 4a13210 Sep 9, 2022

angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(forms): don't prevent default behavior for forms with method="dialog" #47308

fix(forms): don't prevent default behavior for forms with method="dialog" #47308

crisbeto commented Aug 31, 2022

AndrewKushnir left a comment

crisbeto commented Sep 1, 2022

AndrewKushnir left a comment

dylhunn left a comment

pkozlowski-opensource left a comment

AndrewKushnir commented Sep 7, 2022

crisbeto commented Sep 7, 2022

AndrewKushnir commented Sep 7, 2022

crisbeto commented Sep 8, 2022 •

edited

AndrewKushnir commented Sep 8, 2022

dylhunn commented Sep 9, 2022

angular-automatic-lock-bot bot commented Oct 10, 2022

fix(forms): don't prevent default behavior for forms with method="dialog" #47308

fix(forms): don't prevent default behavior for forms with method="dialog" #47308

Conversation

crisbeto commented Aug 31, 2022

AndrewKushnir left a comment

Choose a reason for hiding this comment

crisbeto commented Sep 1, 2022

AndrewKushnir left a comment

Choose a reason for hiding this comment

dylhunn left a comment

Choose a reason for hiding this comment

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

AndrewKushnir commented Sep 7, 2022

crisbeto commented Sep 7, 2022

AndrewKushnir commented Sep 7, 2022

crisbeto commented Sep 8, 2022 • edited

AndrewKushnir commented Sep 8, 2022

dylhunn commented Sep 9, 2022

angular-automatic-lock-bot bot commented Oct 10, 2022

crisbeto commented Sep 8, 2022 •

edited