Skip to content

Commit

Permalink
fix(many): do not grow slotted checkboxes, radios, selects and toggles (
Browse files Browse the repository at this point in the history
#29501)

Issue number: resolves #29423

---------

## What is the current behavior?
I fixed a bug where icon was collapsing its width when next to a
checkbox, radio or toggle to match the styles of select in
#29328. This caused a
regression for checkboxes, radios, and toggles when slotted inside of an
item. Our test coverage for this was not great, as the slotted inputs
test in item had so many elements that it was not apparent that this bug
was introduced. In addition, the select itself presented the same issue
before my PR and this is a regression from the v7 behavior. See the
following Codepens to see the regression:

- [Ionic v7](https://codepen.io/brandyscarney/pen/jOoPzoL)
- [Ionic v8](https://codepen.io/brandyscarney/pen/KKLpoLX)

## What is the new behavior?
- Updates the checkbox, radio, select, and toggle to reset the flex
property when slotted.
- Adds test coverage for the previous fix I did in
#29328 where icons
were collapsing their width next to checkboxes, radios and toggles. This
was reproducible with a div and easier to see in a test so I used a div
with a background instead of an icon.
- Adds better test coverage for this fix which separates each component
(checkbox, radio, select, toggle) into their own screenshot test to make
sure the width is shrinking or expanding properly based on where it is
located in an item.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

| Before fix
9b59138
| After |
| ---| ---|
| ![before
fix](https://github.com/ionic-team/ionic-framework/assets/6577830/e27c6e3c-3d3a-4889-a44b-5f4a9a6ba552)
| ![after
fix](https://github.com/ionic-team/ionic-framework/assets/6577830/a05829d6-f776-4f0d-b7eb-cb8177449c90)
|

<table width="100%">
  <tr align="center">
    <td width="50%"><b>Before regression fix</b></td>
    <td width="50%"><b>After</b></td>
  </tr>
  <tr>
<td width="50%"><img alt="before-regression-fix"
src="https://github.com/ionic-team/ionic-framework/assets/6577830/bb1aea84-6c83-4fbe-96ad-855c1c9cca95"></td>
<td width="50%"><img alt="after-regression-fix"
src="https://github.com/ionic-team/ionic-framework/assets/6577830/655dab88-55a9-4961-a7fb-2a3233aa0004"></td>
  </tr>
</table>
  • Loading branch information
brandyscarney committed May 15, 2024
1 parent 44e1977 commit c63d07b
Show file tree
Hide file tree
Showing 48 changed files with 234 additions and 0 deletions.
5 changes: 5 additions & 0 deletions core/src/components/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
*/
:host([slot="start"]),
:host([slot="end"]) {
// Reset the flex property when the checkbox
// is slotted to avoid growing the element larger
// than its content.
flex: initial;

width: auto;
}

Expand Down
54 changes: 54 additions & 0 deletions core/src/components/item/test/inputs/item.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,57 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
});
});
});

/**
* This behavior does not vary across directions
*/
configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('item: inputs'), () => {
test('should not shrink the width of a div next to a checkbox, radio, select or toggle', async ({
page,
}, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/29319',
});
await page.setContent(
`
<style>
.box {
background: lightgreen;
width: 60px;
height: 60px;
}
</style>
<ion-list lines="none">
<ion-item>
<div class="box"></div>
<ion-checkbox>Checkbox</ion-checkbox>
</ion-item>
<ion-item>
<div class="box"></div>
<ion-radio>Radio</ion-radio>
</ion-item>
<ion-item>
<div class="box"></div>
<ion-select label="Select">
<ion-select-option>Option</ion-select-option>
</ion-select>
</ion-item>
<ion-item>
<div class="box"></div>
<ion-toggle>Toggle</ion-toggle>
</ion-item>
</ion-list>
`,
config
);

const list = page.locator('ion-list');

await expect(list).toHaveScreenshot(screenshot(`item-inputs-div-with-inputs`));
});
});
});
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
160 changes: 160 additions & 0 deletions core/src/components/item/test/slotted-inputs/item.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,163 @@ configs().forEach(({ title, screenshot, config }) => {
});
});
});

/**
* This behavior does not vary across directions
*/
configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('item: slotted inputs'), () => {
test.describe('checkbox', () => {
test('should not expand the slotted checkbox width larger than its content', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/29423',
});

await page.setContent(
`
<ion-list>
<ion-item>
<ion-checkbox slot="start"></ion-checkbox>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-checkbox slot="end"></ion-checkbox>
</ion-item>
<ion-item>
<ion-checkbox slot="start">Start</ion-checkbox>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-checkbox slot="end">End</ion-checkbox>
</ion-item>
</ion-list>
`,
config
);

const list = page.locator('ion-list');

await expect(list).toHaveScreenshot(screenshot(`item-slotted-inputs-checkbox`));
});
});
test.describe('radio', () => {
test('should not expand the slotted radio width larger than its content', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/29423',
});

await page.setContent(
`
<ion-list>
<ion-item>
<ion-radio slot="start"></ion-radio>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-radio slot="end"></ion-radio>
</ion-item>
<ion-item>
<ion-radio slot="start">Start</ion-radio>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-radio slot="end">End</ion-radio>
</ion-item>
</ion-list>
`,
config
);

const list = page.locator('ion-list');

await expect(list).toHaveScreenshot(screenshot(`item-slotted-inputs-radio`));
});
});
test.describe('select', () => {
test('should not expand the slotted select width larger than its content', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/29423',
});

await page.setContent(
`
<ion-list>
<ion-item>
<ion-select slot="start">
<ion-select-option>Option</ion-select-option>
</ion-select>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-select slot="end">
<ion-select-option>Option</ion-select-option>
</ion-select>
</ion-item>
<ion-item>
<ion-select slot="start" label="Start">
<ion-select-option>Option</ion-select-option>
</ion-select>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-select slot="end" label="End">
<ion-select-option>Option</ion-select-option>
</ion-select>
</ion-item>
</ion-list>
`,
config
);

const list = page.locator('ion-list');

await expect(list).toHaveScreenshot(screenshot(`item-slotted-inputs-select`));
});
});
test.describe('toggle', () => {
test('should not expand the slotted toggle width larger than its content', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/29423',
});

await page.setContent(
`
<ion-list>
<ion-item>
<ion-toggle slot="start"></ion-toggle>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-toggle slot="end"></ion-toggle>
</ion-item>
<ion-item>
<ion-toggle slot="start">Start</ion-toggle>
<ion-label>Label</ion-label>
</ion-item>
<ion-item>
<ion-label>Label</ion-label>
<ion-toggle slot="end">End</ion-toggle>
</ion-item>
</ion-list>
`,
config
);

const list = page.locator('ion-list');

await expect(list).toHaveScreenshot(screenshot(`item-slotted-inputs-toggle`));
});
});
});
});
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions core/src/components/radio/radio.scss
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ input {
*/
:host([slot="start"]),
:host([slot="end"]) {
// Reset the flex property when the checkbox
// is slotted to avoid growing the element larger
// than its content.
flex: initial;

width: auto;
}

Expand Down
5 changes: 5 additions & 0 deletions core/src/components/select/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@
*/
:host([slot="start"]),
:host([slot="end"]) {
// Reset the flex property when the checkbox
// is slotted to avoid growing the element larger
// than its content.
flex: initial;

width: auto;
}

Expand Down
5 changes: 5 additions & 0 deletions core/src/components/toggle/toggle.scss
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@
*/
:host([slot="start"]),
:host([slot="end"]) {
// Reset the flex property when the checkbox
// is slotted to avoid growing the element larger
// than its content.
flex: initial;

width: auto;
}

Expand Down

0 comments on commit c63d07b

Please sign in to comment.