Skip to content

Commit

Permalink
refactor(item-sliding): remove unused CSS from item options (#28367)
Browse files Browse the repository at this point in the history
Issue number: Internal

---------

## What is the current behavior?
Item sliding has some unused CSS and no tests for safe area padding
based on the direction.

This CSS is not used:
https://github.com/ionic-team/ionic-framework/blob/feda7a0e963048d300eb17d4b9e1056f09088714/core/src/components/item-option/item-option.scss#L20-L30

The rendered markup for a sliding item looks like the following:

```html
<ion-item-sliding>
  <ion-item-options side="start">
    <ion-item-option>
      Archive
    </ion-item-option>
  </ion-item-options>

  <ion-item class="in-list">
    <ion-label>
      Sliding Item
    </ion-label>
  </ion-item>
</ion-item-sliding>
```

Since `ion-item-options` never gets the `in-list` class added to it, and
`ion-item` never contains options, the above CSS is never used.

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

- Removed the CSS that is not used, the correct CSS for safe area
padding has already been added here:
https://github.com/ionic-team/ionic-framework/blob/feda7a0e963048d300eb17d4b9e1056f09088714/core/src/components/item-options/item-options.scss#L57-L67
- Added screenshot tests to verify the safe area padding is applied to
the proper side

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

I could add additional tests that make sure there is not padding added
when opening the opposite side on each direction but since this problem
was happening when changing from `ltr` to `rtl` I did not.

[FW-5174]:
https://ionic-cloud.atlassian.net/browse/FW-5174?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: ionitron <hi@ionicframework.com>
  • Loading branch information
brandyscarney and Ionitron committed Oct 18, 2023
1 parent 0188289 commit 416bb73
Show file tree
Hide file tree
Showing 26 changed files with 91 additions and 12 deletions.
12 changes: 0 additions & 12 deletions core/src/components/item-option/item-option.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@
font-family: $font-family-base;
}

:host(.in-list.item-options-end:last-child) {
@include padding-horizontal(
null, calc(.7em + var(--ion-safe-area-right))
);
}

:host(.in-list.item-options-start:first-child) {
@include padding-horizontal(
calc(.7em + var(--ion-safe-area-left)), null
);
}

:host(.ion-color) {
background: current-color(base);
color: current-color(contrast);
Expand Down
91 changes: 91 additions & 0 deletions core/src/components/item-sliding/test/basic/item-sliding.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,94 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, screenshot, co
});
});
});

/**
* This behavior needs to be tested in both modes and directions to
* make sure the safe area padding is applied only to that side
* regardless of direction
*/
configs().forEach(({ title, screenshot, config }) => {
test.describe(title('item-sliding: basic'), () => {
test.describe('safe area left', () => {
test('should have padding on the left only', async ({ page }) => {
await page.setContent(
`
<style>
:root {
--ion-safe-area-left: 40px;
}
</style>
<ion-item-sliding>
<ion-item-options side="start">
<ion-item-option color="primary">
Archive
</ion-item-option>
<ion-item-option color="danger">
Delete
</ion-item-option>
</ion-item-options>
<ion-item>
<ion-label>
Sliding Item
</ion-label>
</ion-item>
</ion-item-sliding>
`,
config
);

const direction = config.direction;
const item = page.locator('ion-item-sliding');

const dragByX = direction == 'rtl' ? -150 : 150;
await dragElementBy(item, page, dragByX);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot(`item-sliding-safe-area-left`));
});
});

test.describe('safe area right', () => {
test('should have padding on the right only', async ({ page }) => {
await page.setContent(
`
<style>
:root {
--ion-safe-area-right: 40px;
}
</style>
<ion-item-sliding>
<ion-item>
<ion-label>
Sliding Item
</ion-label>
</ion-item>
<ion-item-options side="end">
<ion-item-option color="primary">
Archive
</ion-item-option>
<ion-item-option color="danger">
Delete
</ion-item-option>
</ion-item-options>
</ion-item-sliding>
`,
config
);

const direction = config.direction;
const item = page.locator('ion-item-sliding');

const dragByX = direction == 'rtl' ? 150 : -150;
await dragElementBy(item, page, dragByX);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot(`item-sliding-safe-area-right`));
});
});
});
});
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.
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.

0 comments on commit 416bb73

Please sign in to comment.