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

Exclude form elements within disabled fieldsets #2012

Merged
merged 3 commits into from
Nov 30, 2023

Conversation

flixcor
Copy link
Contributor

@flixcor flixcor commented Nov 19, 2023

Description

The default behavior for browsers is to exclude form inputs from submissions when they're inside a fieldset that is disabled (tested on Chrome, Firefox and IE11).

However, when using the hx-boost attribute, these inputs are included in submissions.

<!-- submits to ? -->
<form>
  <fieldset disabled>
    <input name="my-input" value="oops">
  </fieldset>
  <button>Submit</button>
</form>

<!-- submits to ?my-input=oops -->
<form hx-boost="true">
  <fieldset disabled>
    <input name="my-input" value="oops">
  </fieldset>
  <button>Submit</button>
</form>

Corresponding issue:

Testing

I added a test for an input within a disabled fieldset

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@alexpetros alexpetros added the under discussion Issues that are being considered but require further alignment label Nov 22, 2023
@alexpetros
Copy link
Collaborator

I like it but this might have to go behind a config; I could imagine a situation where people are using disabled inputs to pre-populate forms and show them to the user, disable.

@alexpetros
Copy link
Collaborator

Does hx-put on a form respect disable attributes? If it does, then this is something I think we could just fix without a config. If it doesn't, then the behavior is kind of baked and I think we do need one.

@flixcor
Copy link
Contributor Author

flixcor commented Nov 22, 2023

It respects disable attributes directly on inputs as far as I can tell (as in, it excludes them from the form post).

@alexpetros
Copy link
Collaborator

Okay let's send it, I like it

@alexpetros alexpetros added ready for review Issues that are ready to be considered for merging and removed under discussion Issues that are being considered but require further alignment labels Nov 23, 2023
@1cg 1cg merged commit 4dc97a4 into bigskysoftware:master Nov 30, 2023
1 check passed
@1cg
Copy link
Contributor

1cg commented Nov 30, 2023

great change, thank you!

@marcus-at-localhost
Copy link
Contributor

I like it but this might have to go behind a config; I could imagine a situation where people are using disabled inputs to pre-populate forms and show them to the user, disable.

I totally tripped over that change.

I disable a fieldset while htmx is in the process of sending the form.
Also, I use bootstrap, and they style [disabled] fieldsets so it was my first choice without thinking about the side effect of fields not being sent, especially since it just worked with HTMX, until it didn't after the update.

[readonly] is not available on fieldsets, so I'm left with adding a css class that's removing the pointer-event and maybe add some other visual styles to dim the fields.

fieldset.disabled {
	pointer-events: none;
	opacity: .75;
}

At the end of the day, this is the correct implementation and I guess it's debatable if disabling form elements while in the process of sending, is great UX.

Just leaving this here as context if someone searches for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants