fix(radio-group): enable unselect with spacebar #23194
Conversation
There was a problem hiding this comment.
Looks good, but we need to add a test for this before we can merge. I would prefer that we do a spec test if possible since they run a bit faster than E2E tests.
You can add a radio-group.spec.ts file in radio-group/test. See https://github.com/ionic-team/ionic-framework/blob/next/core/src/components/accordion/test/accordion.spec.ts for examples of how I write the tests. Also see https://stenciljs.com/docs/unit-testing for the Stencil testing API for newSpecPage.
We should add a test for when the user presses spacebar on a radio group that has allowEmptySelection and a test for when the user presses spacebar on a radio group that does not have allowEmptySelection.
| const radioGroup = await page.find(`ion-radio#${radioButtonId} >>> input`); | ||
| const checked = await radioGroup.getProperty('checked'); | ||
| return checked; |
There was a problem hiding this comment.
Any reason why we can't just look at the checked property on the host ion-radio element instead of diving into the shadow dom?
There was a problem hiding this comment.
Discussed offline to keep test as is. checked is a state not a prop.
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build) was run locally and any changes were pushednpm run lint) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: resolves #22734
What is the new behavior?
allowEmptySelectionis enabled on the radio groupDoes this introduce a breaking change?