fix(select): popover uses modern form syntax (#27818)
Issue number: resolves #27071, resolves #27786 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> `ion-select-popover` is using the legacy syntax for `ion-checkbox` and `ion-radio`. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Migrated checkbox and radio to use the modern syntax - Updated the visual rendering tests to also check the checked state. I noticed during development that I accidentally broke the checked state, so I added tests so we can't accidentally break it again. A couple notes on other screenshot diffs: - On iOS, the item border line now extends past the radio circle. This is an intentional behavior and aligns with native iOS. Having the radio in the "start" slot is typically only done when the content in the default slot is another interactive element (like and input) and not a text label. - On MD, the control heights are smaller. When comparing with https://material-components.github.io/material-components-web-catalog/#/component/select, the new behavior seems to be correct. Both the spec and Ionic item heights are 48px. Interestingly, the radios in `ion-select-popover` have always been 48px tall, but the checkboxes were 54px. Now both the radios and checkboxes are 48px. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: amandaejohnston <amandaejohnston@users.noreply.github.com> Co-authored-by: ionitron <hi@ionicframework.com>
@ -1,7 +1,7 @@
|
||||
@import "./select-popover";
|
||||
@import "./select-popover.md.vars";
|
||||
|
||||
ion-list ion-radio {
|
||||
ion-list ion-radio::part(container) {
|
||||
opacity: 0;
|
||||
}
|
||||
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
import type { ComponentInterface } from '@stencil/core';
|
||||
import { Element, Component, Host, Prop, h } from '@stencil/core';
|
||||
import { Element, Component, Host, Prop, h, forceUpdate } from '@stencil/core';
|
||||
import { safeCall } from '@utils/overlays';
|
||||
import { getClassMap } from '@utils/theme';
|
||||
|
||||
@ -116,19 +116,28 @@ export class SelectPopover implements ComponentInterface {
|
||||
|
||||
renderCheckboxOptions(options: SelectPopoverOption[]) {
|
||||
return options.map((option) => (
|
||||
<ion-item class={getClassMap(option.cssClass)}>
|
||||
<ion-item
|
||||
class={{
|
||||
// TODO FW-4784
|
||||
'item-checkbox-checked': option.checked,
|
||||
...getClassMap(option.cssClass),
|
||||
}}
|
||||
>
|
||||
<ion-checkbox
|
||||
slot="start"
|
||||
value={option.value}
|
||||
disabled={option.disabled}
|
||||
checked={option.checked}
|
||||
legacy={true}
|
||||
justify="start"
|
||||
labelPlacement="end"
|
||||
onIonChange={(ev) => {
|
||||
this.setChecked(ev);
|
||||
this.callOptionHandler(ev);
|
||||
// TODO FW-4784
|
||||
forceUpdate(this);
|
||||
}}
|
||||
></ion-checkbox>
|
||||
<ion-label>{option.text}</ion-label>
|
||||
>
|
||||
{option.text}
|
||||
</ion-checkbox>
|
||||
</ion-item>
|
||||
));
|
||||
}
|
||||
@ -139,12 +148,16 @@ export class SelectPopover implements ComponentInterface {
|
||||
return (
|
||||
<ion-radio-group value={checked} onIonChange={(ev) => this.callOptionHandler(ev)}>
|
||||
{options.map((option) => (
|
||||
<ion-item class={getClassMap(option.cssClass)}>
|
||||
<ion-label>{option.text}</ion-label>
|
||||
<ion-item
|
||||
class={{
|
||||
// TODO FW-4784
|
||||
'item-radio-checked': option.value === checked,
|
||||
...getClassMap(option.cssClass),
|
||||
}}
|
||||
>
|
||||
<ion-radio
|
||||
value={option.value}
|
||||
disabled={option.disabled}
|
||||
legacy={true}
|
||||
onClick={() => this.dismissParentPopover()}
|
||||
onKeyUp={(ev) => {
|
||||
if (ev.key === ' ') {
|
||||
@ -156,7 +169,9 @@ export class SelectPopover implements ComponentInterface {
|
||||
this.dismissParentPopover();
|
||||
}
|
||||
}}
|
||||
></ion-radio>
|
||||
>
|
||||
{option.text}
|
||||
</ion-radio>
|
||||
</ion-item>
|
||||
))}
|
||||
</ion-radio-group>
|
||||
|
||||
@ -81,11 +81,11 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
|
||||
selectPopoverPage = new SelectPopoverPage(page);
|
||||
});
|
||||
test('should not have visual regressions with single selection', async () => {
|
||||
await selectPopoverPage.setup(config, options, false);
|
||||
await selectPopoverPage.setup(config, checkedOptions, false);
|
||||
await selectPopoverPage.screenshot(screenshot, 'select-popover-diff');
|
||||
});
|
||||
test('should not have visual regressions with multiple selection', async () => {
|
||||
await selectPopoverPage.setup(config, options, true);
|
||||
await selectPopoverPage.setup(config, checkedOptions, true);
|
||||
await selectPopoverPage.screenshot(screenshot, 'select-popover-multiple-diff');
|
||||
});
|
||||
});
|
||||
|
||||
|
Before Width: | Height: | Size: 2.3 KiB After Width: | Height: | Size: 2.4 KiB |
|
Before Width: | Height: | Size: 3.4 KiB After Width: | Height: | Size: 3.5 KiB |
|
Before Width: | Height: | Size: 2.0 KiB After Width: | Height: | Size: 2.1 KiB |
|
Before Width: | Height: | Size: 3.3 KiB After Width: | Height: | Size: 3.5 KiB |
|
Before Width: | Height: | Size: 4.0 KiB After Width: | Height: | Size: 4.3 KiB |
|
Before Width: | Height: | Size: 2.7 KiB After Width: | Height: | Size: 3.3 KiB |
|
Before Width: | Height: | Size: 2.5 KiB After Width: | Height: | Size: 2.5 KiB |
|
Before Width: | Height: | Size: 3.7 KiB After Width: | Height: | Size: 3.7 KiB |
|
Before Width: | Height: | Size: 2.1 KiB After Width: | Height: | Size: 2.2 KiB |