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.

Sign up for GitHub

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

Jump to bottom

Bugfix: Form Mode opt-in should consider outside submitters #655

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Bugfix: Form Mode opt-in should consider outside submitters #655

merged 1 commit into from
Jul 29, 2022

Conversation

seanpdoyle
Copy link
Contributor

Prior to this commit, the Session.formElementIsNavigatable method
checked for the closest <form> element ancestor to its element
argument. Unfortunately, checking for an ancestor form is an incomplete
solution, since form controls can exist outside of the <form>
elsewhere in the document and reference the form through their
[form] attributes.

This commit incorporates a check for that into the
Session.formElementIsNavigatable method. Since that method is only
ever called on a submission's submitter argument after it's been
verified to be present, this commit changes the signature to expect an
element (by omitting the ? modifier) and renames the method to
submitterIsNavigatable.

@seanpdoyle seanpdoyle changed the title Bugfix: Form Mode opt-in handle outside submitters Bugfix: Form Mode opt-in should consider outside submitters Jul 29, 2022
@seanpdoyle
Bugfix: Form Mode opt-in handle outside submitters
3ae1d46 
Prior to this commit, the `Session.formElementIsNavigatable` method
checked for the closest `<form>` element ancestor to its `element`
argument. Unfortunately, checking for an ancestor form is an incomplete
solution, since form controls can exist outside of the `<form>`
elsewhere in the document and reference the form through their
[`[form]`][form-attr] attributes.

This commit incorporates a check for that into the
`Session.formElementIsNavigatable` method. Since that method is only
ever called on a submission's `submitter` argument after it's been
verified to be present, this commit changes the signature to expect an
`element` (by omitting the `?` modifier) and renames the method to
`submitterIsNavigatable`.

[form-attr]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Input#form
@dhh dhh merged commit 3a18111 into hotwired:main Jul 29, 2022
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 30, 2022
@seanpdoyle
Expand Turbo.setFormMode guards
af184b3 
Follow-up to hotwired#655
Follow-up to hotwired#419

This commit splits out a `form_mode_tests.ts` module separate from
`form_submission_tests.ts`, along with a `form_mode.html` fixture file.

Next, add test fixtures and test coverage for more thorough coverage of
form mode, namely scenarios that submit a form without a submitter (for
example, typing into an `<input type="text">` and pressing
<kbd>enter</kbd>.

Next, the `Turbo.setFormMode()` should be particular about its
argument's value. This commit introduces a `FormMode = "on" | "off" |
"optin"` type.

Finally, rename `submitterIsNavigatable` to `submissionIsNavigatable`,
and pass the `HTMLFormElement` and optional `HTMLElement` as a pair of
arguments.
@seanpdoyle seanpdoyle mentioned this pull request Jul 30, 2022
Merged
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 30, 2022
@seanpdoyle
Expand Turbo.setFormMode guards
6feadc8 
Follow-up to hotwired#655
Follow-up to hotwired#419

This commit splits out a `form_mode_tests.ts` module separate from
`form_submission_tests.ts`, along with a `form_mode.html` fixture file.

Next, add test fixtures and test coverage for more thorough coverage of
form mode, namely scenarios that submit a form without a submitter (for
example, typing into an `<input type="text">` and pressing
<kbd>enter</kbd>.

Next, the `Turbo.setFormMode()` should be particular about its
argument's value. This commit introduces a `FormMode = "on" | "off" |
"optin"` type.

Finally, rename `submitterIsNavigatable` to `submissionIsNavigatable`,
and pass the `HTMLFormElement` and optional `HTMLElement` as a pair of
arguments.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 30, 2022
@seanpdoyle
Expand Turbo.setFormMode guards
01d3300 
Follow-up to hotwired#655
Follow-up to hotwired#419

This commit splits out a `form_mode_tests.ts` module separate from
`form_submission_tests.ts`, along with a `form_mode.html` fixture file.

Next, add test fixtures and test coverage for more thorough coverage of
form mode, namely scenarios that submit a form without a submitter (for
example, typing into an `<input type="text">` and pressing
<kbd>enter</kbd>.

Next, the `Turbo.setFormMode()` should be particular about its
argument's value. This commit introduces a `FormMode = "on" | "off" |
"optin"` type.

Finally, rename `submitterIsNavigatable` to `submissionIsNavigatable`,
and pass the `HTMLFormElement` and optional `HTMLElement` as a pair of
arguments.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 30, 2022
@seanpdoyle
Expand Turbo.setFormMode guards
92996ba 
Follow-up to hotwired#655
Follow-up to hotwired#419

This commit splits out a `form_mode_tests.ts` module separate from
`form_submission_tests.ts`, along with a `form_mode.html` fixture file.

Next, add test fixtures and test coverage for more thorough coverage of
form mode, namely scenarios that submit a form without a submitter (for
example, typing into an `<input type="text">` and pressing
<kbd>enter</kbd>.

Next, the `Turbo.setFormMode()` should be particular about its
argument's value. This commit introduces a `FormMode = "on" | "off" |
"optin"` type.

Finally, rename `submitterIsNavigatable` to `submissionIsNavigatable`,
and pass the `HTMLFormElement` and optional `HTMLElement` as a pair of
arguments.
seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Jul 30, 2022
@seanpdoyle
Expand Turbo.setFormMode guards
db7f233 
Follow-up to hotwired#655
Follow-up to hotwired#419

This commit splits out a `form_mode_tests.ts` module separate from
`form_submission_tests.ts`, along with a `form_mode.html` fixture file.

Next, add test fixtures and test coverage for more thorough coverage of
form mode, namely scenarios that submit a form without a submitter (for
example, typing into an `<input type="text">` and pressing
<kbd>enter</kbd>.

Next, the `Turbo.setFormMode()` should be particular about its
argument's value. This commit introduces a `FormMode = "on" | "off" |
"optin"` type.

Finally, rename `submitterIsNavigatable` to `submissionIsNavigatable`,
and pass the `HTMLFormElement` and optional `HTMLElement` as a pair of
arguments.
dhh pushed a commit that referenced this pull request Jul 31, 2022
@seanpdoyle
Expand Turbo.setFormMode guards (#658)
d2443b6 
Follow-up to #655
Follow-up to #419

This commit splits out a `form_mode_tests.ts` module separate from
`form_submission_tests.ts`, along with a `form_mode.html` fixture file.

Next, add test fixtures and test coverage for more thorough coverage of
form mode, namely scenarios that submit a form without a submitter (for
example, typing into an `<input type="text">` and pressing
<kbd>enter</kbd>.

Next, the `Turbo.setFormMode()` should be particular about its
argument's value. This commit introduces a `FormMode = "on" | "off" |
"optin"` type.

Finally, rename `submitterIsNavigatable` to `submissionIsNavigatable`,
and pass the `HTMLFormElement` and optional `HTMLElement` as a pair of
arguments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@seanpdoyle @dhh