fix(radio, checkbox, toggle): add top and bottom margins when in ion-item (#27788)

Issue number: Resolves #27498

---------

<!-- 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. -->

When `ion-radio`, `ion-checkbox`, or `ion-toggle` is used within
`ion-item`, top and bottom margins are not added to the label, while
they are present when using the legacy syntax. We didn't catch this
because the issue is only visible when using a label that breaks onto
more than one line; otherwise, the height of the item exceeds what the
margins would've added.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

Margins added. Values were taken from `ion-item` styles
[here](096d9cc931/core/src/components/item/item.ios.scss (L203-L206))
and
[here](096d9cc931/core/src/components/item/item.md.scss (L298-L301)),
which no longer get applied because the `ion-label` is nested in an
additional shadow component. Note that left/right margins are already
included in the modern syntax labels.

## 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. -->

This PR is a combination of the following:
- https://github.com/ionic-team/ionic-framework/pull/27771
- https://github.com/ionic-team/ionic-framework/pull/27783
- https://github.com/ionic-team/ionic-framework/pull/27784

---------

Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
This commit is contained in:
Amanda Johnston
2023-07-12 15:10:35 -05:00
committed by GitHub
parent 3bd1d7e1e5
commit 35f0ec581a
21 changed files with 145 additions and 9 deletions

View File

@ -1,4 +1,5 @@
@import "../../themes/ionic.globals"; @import "../../themes/ionic.globals";
@import "./checkbox.vars.scss";
// Checkbox // Checkbox
// -------------------------------------------------- // --------------------------------------------------
@ -103,6 +104,10 @@
overflow: hidden; overflow: hidden;
} }
:host(.in-item:not(.legacy-checkbox)) .label-text-wrapper {
@include margin($checkbox-item-label-margin-top, null, $checkbox-item-label-margin-bottom, null);
}
/** /**
* If no label text is placed into the slot * If no label text is placed into the slot
* then the element should be hidden otherwise * then the element should be hidden otherwise
@ -194,7 +199,7 @@ input {
* the checkbox should be on the end * the checkbox should be on the end
* when the label sits at the start. * when the label sits at the start.
*/ */
@include margin(0, $form-control-label-margin, 0, 0); @include margin(null, $form-control-label-margin, null, 0);
} }
@ -215,7 +220,7 @@ input {
* when the label sits at the end. * when the label sits at the end.
*/ */
:host(.checkbox-label-placement-end) .label-text-wrapper { :host(.checkbox-label-placement-end) .label-text-wrapper {
@include margin(0, 0, 0, $form-control-label-margin); @include margin(null, 0, null, $form-control-label-margin);
} }
@ -228,7 +233,7 @@ input {
* the checkbox should be on the end * the checkbox should be on the end
* when the label sits at the start. * when the label sits at the start.
*/ */
@include margin(0, $form-control-label-margin, 0, 0); @include margin(null, $form-control-label-margin, null, 0);
} }
/** /**

View File

@ -0,0 +1,5 @@
/// @prop - Top margin of checkbox's label when in an item
$checkbox-item-label-margin-top: 10px !default;
/// @prop - Bottom margin of checkbox's label when in an item
$checkbox-item-label-margin-bottom: 10px !default;

View File

@ -50,3 +50,24 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
}); });
}); });
}); });
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('checkbox: long label in item'), () => {
test('should render margins correctly when using long label in item', async ({ page }) => {
await page.setContent(
`
<ion-list>
<ion-item>
<ion-checkbox justify="start">
<ion-label class="ion-text-wrap">Enable Notifications Enable Notifications Enable Notifications</ion-label>
</ion-checkbox>
</ion-item>
</ion-list>
`,
config
);
const list = page.locator('ion-list');
expect(await list.screenshot()).toMatchSnapshot(screenshot(`checkbox-long-label-in-item`));
});
});
});

View File

@ -148,6 +148,19 @@
</ion-list> </ion-list>
</div> </div>
</div> </div>
<h1>Multiline Label</h1>
<div class="grid">
<div class="grid-item">
<ion-item>
<ion-checkbox justify="start">
<ion-label class="ion-text-wrap">
Enable Notifications Enable Notifications Enable Notifications
</ion-label>
</ion-checkbox>
</ion-item>
</div>
</div>
</ion-content> </ion-content>
</ion-app> </ion-app>
</body> </body>

View File

@ -1,4 +1,5 @@
@import "../../themes/ionic.globals"; @import "../../themes/ionic.globals";
@import "./radio.vars.scss";
// Radio // Radio
// -------------------------------------------------- // --------------------------------------------------
@ -122,6 +123,10 @@ input {
overflow: hidden; overflow: hidden;
} }
:host(.in-item:not(.legacy-radio)) .label-text-wrapper {
@include margin($radio-item-label-margin-top, null, $radio-item-label-margin-bottom, null);
}
/** /**
* If no label text is placed into the slot * If no label text is placed into the slot
* then the element should be hidden otherwise * then the element should be hidden otherwise
@ -172,7 +177,7 @@ input {
* the radio should be on the end * the radio should be on the end
* when the label sits at the start. * when the label sits at the start.
*/ */
@include margin(0, $form-control-label-margin, 0, 0); @include margin(null, $form-control-label-margin, null, 0);
} }
// Radio Label Placement - End // Radio Label Placement - End
@ -192,7 +197,7 @@ input {
* when the label sits at the end. * when the label sits at the end.
*/ */
:host(.radio-label-placement-end) .label-text-wrapper { :host(.radio-label-placement-end) .label-text-wrapper {
@include margin(0, 0, 0, $form-control-label-margin); @include margin(null, 0, null, $form-control-label-margin);
} }
// Radio Label Placement - Fixed // Radio Label Placement - Fixed
@ -204,7 +209,7 @@ input {
* the radio should be on the end * the radio should be on the end
* when the label sits at the start. * when the label sits at the start.
*/ */
@include margin(0, $form-control-label-margin, 0, 0); @include margin(null, $form-control-label-margin, null, 0);
} }
/** /**

View File

@ -0,0 +1,5 @@
/// @prop - Top margin of radio's label when in an item
$radio-item-label-margin-top: 10px !default;
/// @prop - Bottom margin of radio's label when in an item
$radio-item-label-margin-bottom: 10px !default;

View File

@ -165,6 +165,21 @@
</ion-radio-group> </ion-radio-group>
</ion-list> </ion-list>
</div> </div>
<h1>Multiline Label</h1>
<div class="grid">
<div class="grid-item">
<ion-radio-group>
<ion-item>
<ion-radio justify="start">
<ion-label class="ion-text-wrap">
Enable Notifications Enable Notifications Enable Notifications
</ion-label>
</ion-radio>
</ion-item>
</ion-radio-group>
</div>
</div>
</ion-content> </ion-content>
</ion-app> </ion-app>
</body> </body>

View File

@ -37,3 +37,26 @@ configs().forEach(({ title, screenshot, config }) => {
}); });
}); });
}); });
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('radio: long label in item'), () => {
test('should render margins correctly when using long label in item', async ({ page }) => {
await page.setContent(
`
<ion-list>
<ion-radio-group>
<ion-item>
<ion-radio justify="start">
<ion-label class="ion-text-wrap">Enable Notifications Enable Notifications Enable Notifications</ion-label>
</ion-radio>
</ion-item>
</ion-radio-group>
</ion-list>
`,
config
);
const list = page.locator('ion-list');
expect(await list.screenshot()).toMatchSnapshot(screenshot(`radio-long-label-in-item`));
});
});
});

View File

@ -134,6 +134,19 @@
</ion-list> </ion-list>
</div> </div>
</div> </div>
<h1>Multiline Label</h1>
<div class="grid">
<div class="grid-item">
<ion-item>
<ion-toggle justify="start">
<ion-label class="ion-text-wrap">
Enable Notifications Enable Notifications Enable Notifications
</ion-label>
</ion-toggle>
</ion-item>
</div>
</div>
</ion-content> </ion-content>
</ion-app> </ion-app>
</body> </body>

View File

@ -50,3 +50,24 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
}); });
}); });
}); });
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('toggle: long label in item'), () => {
test('should render margins correctly when using long label in item', async ({ page }) => {
await page.setContent(
`
<ion-list>
<ion-item>
<ion-toggle justify="start">
<ion-label class="ion-text-wrap">Enable Notifications Enable Notifications Enable Notifications</ion-label>
</ion-toggle>
</ion-item>
</ion-list>
`,
config
);
const list = page.locator('ion-list');
expect(await list.screenshot()).toMatchSnapshot(screenshot(`toggle-long-label-in-item`));
});
});
});

View File

@ -1,4 +1,5 @@
@import "../../themes/ionic.globals"; @import "../../themes/ionic.globals";
@import "./toggle.vars.scss";
// Toggle // Toggle
// -------------------------------------------------- // --------------------------------------------------
@ -120,6 +121,10 @@ input {
overflow: hidden; overflow: hidden;
} }
:host(.in-item:not(.legacy-toggle)) .label-text-wrapper {
@include margin($toggle-item-label-margin-top, null, $toggle-item-label-margin-bottom, null);
}
/** /**
* If no label text is placed into the slot * If no label text is placed into the slot
* then the element should be hidden otherwise * then the element should be hidden otherwise
@ -170,7 +175,7 @@ input {
* the input should be on the end * the input should be on the end
* when the label sits at the start. * when the label sits at the start.
*/ */
@include margin(0, $form-control-label-margin, 0, 0); @include margin(null, $form-control-label-margin, null, 0);
} }
// Toggle Label Placement - End // Toggle Label Placement - End
@ -190,7 +195,7 @@ input {
* when the label sits at the end. * when the label sits at the end.
*/ */
:host(.toggle-label-placement-end) .label-text-wrapper { :host(.toggle-label-placement-end) .label-text-wrapper {
@include margin(0, 0, 0, $form-control-label-margin); @include margin(null, 0, null, $form-control-label-margin);
} }
// Input Label Placement - Fixed // Input Label Placement - Fixed
@ -202,7 +207,7 @@ input {
* the input should be on the end * the input should be on the end
* when the label sits at the start. * when the label sits at the start.
*/ */
@include margin(0, $form-control-label-margin, 0, 0); @include margin(null, $form-control-label-margin, null, 0);
} }
/** /**

View File

@ -0,0 +1,5 @@
/// @prop - Top margin of toggle's label when in an item
$toggle-item-label-margin-top: 10px !default;
/// @prop - Bottom margin of toggle's label when in an item
$toggle-item-label-margin-bottom: 10px !default;