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

Regression with calcite-button#form where form is not submitted/reset #5164

Closed
nwhittaker opened this issue Aug 16, 2022 · 6 comments
Closed
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 5 A few days of work, definitely requires updates to tests. has workaround Issues have a workaround available in the meantime. p - low Issue is non core or affecting less that 10% of people using the library

Comments

@nwhittaker
Copy link
Contributor

nwhittaker commented Aug 16, 2022

Actual Behavior

Given a submit or reset button associated with a non-ancestor form, clicking the button does not submit/reset the form.

Expected Behavior

Given a submit or reset button associated with a non-ancestor form, clicking the button does submit/reset the form.

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/wvmQjMO

Reproduction Steps

  1. Visit the sample and enter some text into the input.
  2. Click the reset button and not the input does not clear.
  3. Open the console and click the submit button and note "onsubmit" is not printed.

Reproduction Version

beta.91

Relevant Info

Possibly introduced by #3287.

I'd also ask to revisit the decision to deprecate the form attribute. Marking it as deprecated suggests it'll be removed from the API at some point in the future. I understand it's no longer needed in the case where the buttons are descendants of the <form>, but if they're not, the attribute is still needed and perfectly valid.

Regression?

next.290

@nwhittaker nwhittaker added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Aug 16, 2022
@nwhittaker nwhittaker changed the title Regression with calcite-button#form where form is not submitted Regression with calcite-button#form where form is not submitted/reset Aug 16, 2022
@jcfranco
Copy link
Member

I understand it's no longer needed in the case where the buttons are descendants of the <form>, but if they're not, the attribute is still needed and perfectly valid.

Agreed. We can revisit our form-associated pattern to accommodate this. In the meantime, can you use the form-descendant approach as a workaround?

For prioritization, can you share more info about your use case and the impact of this?

@nwhittaker
Copy link
Contributor Author

For prioritization, can you share more info about your use case and the impact of this?

One use-case where I came up against this issue was when I tried to treat the contents of a modal like a form, and the modal's buttons like form buttons:

<calcite-modal>
  <form id="form-id" onsubmit="" slot="content">
    <calcite-input />
  </form>
  <calcite-button form="form-id" slot="primary" type="submit" />
  <calcite-button form="form-id" slot="secondary" type="reset" />
</calcite-modal>

This lets me consolidate the submission logic and retain native submission behavior:

  • Click submit button
  • Hit Enter key when input is focused

Currently achieving this behavior would require separate handlers and added logic:

<calcite-modal>
  <form slot="content">
    <calcite-input onkeyup="" />
  </form>
  <calcite-button onclick="" slot="primary" type="submit" />
  <calcite-button onclick="" slot="secondary" type="reset" />
</calcite-modal>

You might run into this problem with other components that separate actions from content, but is perhaps more easily mitigated by just wrapping the whole thing inside a <form>. The modal doesn't have quite the same affordance because it could be wormholed out of its current spot in the DOM.

@geospatialem geospatialem removed the needs triage Planning workflow - pending design/dev review. label Oct 12, 2022
@geospatialem geospatialem removed this from the Sprint 2022/11/20 - 2022/12/01 milestone Dec 2, 2022
@geospatialem geospatialem added the needs triage Planning workflow - pending design/dev review. label Dec 2, 2022
@geospatialem
Copy link
Member

This issue will be prioritized after the 1.0 release next month.

@benelan benelan added ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. and removed ArcGIS Field Apps labels Jan 30, 2023
@jcfranco jcfranco added the estimate - 5 A few days of work, definitely requires updates to tests. label Feb 6, 2023
@geospatialem geospatialem added p - low Issue is non core or affecting less that 10% of people using the library has workaround Issues have a workaround available in the meantime. labels Feb 27, 2023
@geospatialem geospatialem added this to the Freezer milestone Feb 27, 2023
@alisonailea alisonailea modified the milestone: Freezer Feb 28, 2023
@alisonailea alisonailea changed the title Regression with calcite-button#form where form is not submitted/reset Regression with calcite-button#form where form is not submitted/reset Feb 28, 2023
@brettephillips
Copy link

Our team (Real-Time & Big Data: ArcGIS Velocity) is running into the same problem. We could use the workaround suggested by @nwhittaker, but this would be a large LOE and could cause some regression as there are a lot of moving parts. It is currently blocking a lot of our tests and verification

@geospatialem geospatialem added the research Issues that require more in-depth research or multiple team members to resolve or make decision. label Mar 3, 2023
@geospatialem geospatialem removed the needs triage Planning workflow - pending design/dev review. label Mar 3, 2023
@geospatialem
Copy link
Member

Assigned for researching next steps with the form utility.

@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Mar 3, 2023
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Mar 27, 2023
jcfranco added a commit that referenced this issue Mar 28, 2023
**Related Issue:** #5164 

## Summary

This allows developers to associate form components by providing the
form ID.

### Notes

* the form must be available in the DOM before the component is attached
to the DOM
* components will not look up ancestors for form if the ID is invalid or
if the form is not found
@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 2 - in development Issues that are actively being worked on. labels Mar 28, 2023
@geospatialem
Copy link
Member

Verified on master.

@jcfranco jcfranco removed the research Issues that require more in-depth research or multiple team members to resolve or make decision. label Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 5 A few days of work, definitely requires updates to tests. has workaround Issues have a workaround available in the meantime. p - low Issue is non core or affecting less that 10% of people using the library
Projects
None yet
Development

No branches or pull requests

6 participants