react icon indicating copy to clipboard operation
react copied to clipboard

experimental/SelectPanel: Don't render form if nested inside form

Open siddharthkp opened this issue 1 month ago • 2 comments

[!NOTE]
If we want, we can wait until we figure out the semantics for https://github.com/github/primer/issues/3183 to find out if we want to render a <form> element at all

Bug:

SelectPanel renders a <form> element inside a <dialog>. We also do allow the use of SelectPanel inside with FormControl (which is expected to be inside a <form> already)

This would create invalid DOM nesting of <form> nested inside <form>. react-dom throws a validation warning on the console

Fix:

If SelectPanel is nested inside FormControl, it will now render a <div> instead of <form>.

Note: If someone is using SelectPanel in a custom form without FormControl, this PR would not be able to catch that scenario.

Rollout strategy

  • [x] Patch release
  • [ ] Minor release
  • [ ] Major release; if selected, include a written rollout or migration plan
  • [ ] None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

  • [ ] Added/updated tests
  • [ ] Added/updated documentation
  • [x] Added/updated previews (Storybook)
  • [x] Changes are SSR compatible
  • [x] Tested in Chrome
  • [ ] Tested in Firefox
  • [x] Tested in Safari
  • [ ] Tested in Edge
  • [ ] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

siddharthkp avatar May 02 '24 14:05 siddharthkp