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

test: add test where form is descendant of element with stop propagation on click #2309

Merged

Conversation

penspinner
Copy link
Contributor

@penspinner penspinner commented Mar 13, 2022

Bug

Added a failing test where clicking a submit button as a descendant of an element that stops propagation on click does not send the clicked submit button's name and value properties to the action request payload.

I think this bug was introduced from #1781 (released in v1.2.0) because this used to work until I updated dependencies at some point.


How I encountered this

I encountered this bug through my own app. I had a modal with a form in it, and it suddenly stopped sending { _action: add_note } to my action's request payload. My code looks something like this:

import { Dialog } from '@headlessui/react'
import { Form } from 'remix'

const Page = () => {
  return (
    <Dialog>
      <Form method="post">
        <button name="_action" value="add_note">
          Add note
        </button
      </Form
    </Dialog>
  )
}

export default Page

And I noticed Headless UI's Dialog has an event.stopPropagation() on the div's onClick: https://github.com/tailwindlabs/headlessui/blob/c4e35f38794b89ec9692e8bb7027447503623604/packages/%40headlessui-react/src/components/dialog/dialog.tsx#L318

@penspinner penspinner marked this pull request as ready for review March 13, 2022 23:15
@penspinner
Copy link
Contributor Author

integration/bug-report-test.ts Outdated Show resolved Hide resolved
integration/bug-report-test.ts Outdated Show resolved Hide resolved
@penspinner penspinner force-pushed the bug/ancestor-stop-propagation-button branch from 5a74dc8 to 67f771a Compare March 16, 2022 23:08
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Mar 16, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@penspinner penspinner force-pushed the bug/ancestor-stop-propagation-button branch 2 times, most recently from d59968b to 625cb9b Compare March 16, 2022 23:46
@penspinner penspinner changed the title Bug/ancestor stop propagation button bug: add failing test where descendant of element with stop propagation on click messing up submit button payload Mar 16, 2022
@mcansh mcansh changed the title bug: add failing test where descendant of element with stop propagation on click messing up submit button payload test: add test where descendant of element with stop propagation on click May 8, 2022
@mcansh mcansh changed the title test: add test where descendant of element with stop propagation on click test: add test where form is descendant of element with stop propagation on click May 8, 2022
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@mcansh mcansh requested a review from MichaelDeBoey May 8, 2022 14:10
@mcansh mcansh changed the title test: add test where form is descendant of element with stop propagation on click test: add test where form is descendant of element with stop propagation on click May 8, 2022
@mcansh mcansh merged commit e6d5a05 into remix-run:dev May 8, 2022
@penspinner penspinner deleted the bug/ancestor-stop-propagation-button branch May 8, 2022 14:54
@penspinner
Copy link
Contributor Author

@mcansh Thanks! Another duplicated PR for this bug: #2034.

christophertrudel pushed a commit to christophertrudel/remix that referenced this pull request May 16, 2022
…ation on click (remix-run#2309)

Co-authored-by: Logan McAnsh <logan@mcan.sh>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants