ionic-framework
ionic-framework copied to clipboard
feat(button): submit from outside of form
Pull request checklist
Please check if your PR fulfills the following requirements:
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been reviewed and added / updated if needed (for bug fixes / features)
- Some docs updates need to be made in the
ionic-docsrepo, in a separate PR. See the contributing guide for details.
- Some docs updates need to be made in the
- [x] Build (
npm run build) was run locally and any changes were pushed - [x] Lint (
npm run lint) has passed locally and any fixes were made for failures
Pull request type
Please check the type of change your PR introduces:
- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):
What is the current behavior?
Buttons with type "submit" will only work from inside forms.
Issue URL: https://github.com/ionic-team/ionic-framework/issues/21194
What is the new behavior?
Now you can add a "form" prop to every
Does this introduce a breaking change?
- [ ] Yes
- [x] No
Other information
Not sure, if the docs have to be updated. If so, I'm happy to do so. Maybe you could guide me to the right place.
@sean-perkins Thanks for the support. But I can't get the tests to run.
I've tried all possible commands for testing npm run test.screenshot or npm run test or npm run run test.e2e but only the plain e2e.ts files will be executed not the button.e2e.ts files.
npm test src/components/button/test/basic/button.e2e.ts will run into the following error message Pattern: src/components/button/test/basic/button.e2e.ts - 0 matches.
@postnerd We are in the middle of a transition between two E2E testing systems. The npm run test.e2e command is for the old system. You should do npx playwright test src/components/button/test/basic to run the new Playwright tests.
The commit message is ok here. We will end up squashing the commits anyways, so only the topmost description (the PR title) will matter.
I approved CI to run to verify the tests. I don't see any other items requiring changes on my end.
If CI passes, I'll add the team and we can discuss the release target for this feature.
@sean-perkins Is there anything to do from my side because of the failed tests?
@postnerd merge the latest change from main into this branch, run npm run build (from the core folder) and commit any changes (if any).
Merged again (after @liamdebeasi already merged) because the branch was again one commit behind.
npm run build didn't bring anything up related to this pull request.
Only one change to angular/src/directives/proxies-list.ts that I've ignored before, because it looks like an unrelated style fix (a strange one, adding a line at the beginning of the file ...).
Yeah, you can ignore that diff. Running npm run prettier from the /angular folder will overwrite that change and reformat the file with the current state.
I re-triggered CI to run.
@postnerd update on our end: The team has drafted and approved a design document around this feature. The only item that is outstanding as a result, is we want to introduce a developer warning when the form cannot be found, when using the form syntax.
I can snag this scope & can commit it directly to your fork.
We will target this feature for 6.3 after I make that change & then the team can do a final review.
Great work here @postnerd 👍 thanks for the contribution!