Skip to content

Commit

Permalink
fix(checkbox): align checkbox properly in item using start alignment (#…
Browse files Browse the repository at this point in the history
…29850)

Issue number: resolves #29837

---------

## What is the current behavior?
The checkbox is not aligned properly to the top when using a long label
with `alignment="start"` inside of an `ion-item`:

```html
<ion-item>
  <ion-checkbox justify="start" alignment="start">
    <ion-label class="ion-text-wrap">
      Enable Notifications Enable Notifications Enable Notifications
    </ion-label>
  </ion-checkbox>
</ion-item>
```

## What is the new behavior?
- Applies the same margin to the `.native-wrapper` (checkbox) as the
label.
- Adds a screenshot test to verify the alignment

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

| Before | After |
| --- | --- |
|
![before](https://github.com/user-attachments/assets/53579523-e8b5-4152-ae91-14847cb395e4)
|
![after](https://github.com/user-attachments/assets/0d7315ed-3294-4a27-82fe-6900eb9db1c0)
|
|
![before](https://github.com/user-attachments/assets/ca025a94-4ef7-44b4-85d0-5183e4326814)
|
![after](https://github.com/user-attachments/assets/fab60703-1196-48e8-a485-2f33c4893aba)
|

- [Label
Preview](https://ionic-framework-git-rou-11163-ionic1.vercel.app/src/components/checkbox/test/label)
- [Item
Preview](https://ionic-framework-git-rou-11163-ionic1.vercel.app/src/components/checkbox/test/item)

> [!NOTE]
> The alignment on the Material Design checkbox is still slightly off. I
could add margin directly to its checkbox but then it would change the
margin of the checkbox in all use cases.
  • Loading branch information
brandyscarney authored Sep 10, 2024
1 parent f1d50c0 commit 88b7013
Show file tree
Hide file tree
Showing 16 changed files with 54 additions and 2 deletions.
5 changes: 4 additions & 1 deletion core/src/components/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@
overflow: hidden;
}

:host(.in-item) .label-text-wrapper {
// Checkboxes that are not slotted inside an item and are not used with a
// stacked label should have margins equal to those of the label.
:host(.in-item) .label-text-wrapper,
:host(.in-item:not(.checkbox-label-placement-stacked):not([slot])) .native-wrapper {
@include margin($checkbox-item-label-margin-top, null, $checkbox-item-label-margin-bottom, null);
}

Expand Down
24 changes: 23 additions & 1 deletion core/src/components/checkbox/test/item/checkbox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
});
});

configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, config }) => {
configs({ directions: ['ltr'] }).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(
Expand All @@ -69,6 +69,28 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co
const list = page.locator('ion-list');
await expect(list).toHaveScreenshot(screenshot(`checkbox-long-label-in-item`));
});
test('should render margins correctly when using long label in item with start alignment', async ({
page,
}, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/29837',
});
await page.setContent(
`
<ion-list>
<ion-item>
<ion-checkbox justify="start" alignment="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');
await expect(list).toHaveScreenshot(screenshot(`checkbox-long-label-in-item-align-start`));
});
});

test.describe(title('checkbox: stacked label in item'), () => {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions core/src/components/checkbox/test/item/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,15 @@ <h1>Multiline Label</h1>
</ion-checkbox>
</ion-item>
</div>
<div class="grid-item">
<ion-item>
<ion-checkbox justify="start" alignment="start">
<ion-label class="ion-text-wrap">
Enable Notifications Enable Notifications Enable Notifications
</ion-label>
</ion-checkbox>
</ion-item>
</div>
</div>
</ion-content>
</ion-app>
Expand Down
18 changes: 18 additions & 0 deletions core/src/components/checkbox/test/label/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,24 @@ <h2>Align Center</h2>
<ion-checkbox label-placement="stacked" alignment="center">Enable Notifications</ion-checkbox>
</div>
</div>

<h1>Multiline Label</h1>
<div class="grid">
<div class="grid-item">
<ion-checkbox justify="start">
<ion-label class="ion-text-wrap">
Enable Notifications Enable Notifications Enable Notifications
</ion-label>
</ion-checkbox>
</div>
<div class="grid-item">
<ion-checkbox justify="start" alignment="start">
<ion-label class="ion-text-wrap">
Enable Notifications Enable Notifications Enable Notifications
</ion-label>
</ion-checkbox>
</div>
</div>
</ion-content>
</ion-app>
</body>
Expand Down

0 comments on commit 88b7013

Please sign in to comment.