Skip to content

Commit cafafcc

Browse files
liamdebeasiIonitronbrandyscarney
authored
fix(list): remove border from last item with item-sliding (#28439)
Issue number: resolves #28435 --------- <!-- 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. --> The item in the last item-sliding still has a border in an inset list. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Item in last item-sliding no longer has a border I originally only added `ion-item-sliding:last-of-type ion-item` but I discovered that the original `ion-item:last-child` causes items in item-sliding where the item is the last element in the item-sliding container to not have a border, so the original fix was incomplete. I added comments as to what each line does and why we didn't just do `ion-item:last-child`. ## 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: Brandy Carney <[email protected]> --------- Co-authored-by: ionitron <[email protected]> Co-authored-by: Brandy Carney <[email protected]>
1 parent 00c3a44 commit cafafcc

21 files changed

+215
-5
lines changed

‎core/src/components/list/list.ios.scss‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,25 @@
1919
@include border-radius($list-inset-ios-border-radius);
2020
}
2121

22-
.list-ios.list-inset ion-item:last-child {
22+
/**
23+
* These selectors ensure the last item in the list
24+
* has the correct border.
25+
* We need to consider the following scenarios:
26+
1. The only item in a list.
27+
2. The last item in a list as long as it is not the only item.
28+
3. The item in the last item-sliding in a list.
29+
* Note that we do not select ion-item:last-of-type
30+
* because that will cause the borders to disappear on
31+
* items in an item-sliding when the item is the last
32+
* element in the item-sliding container.
33+
*/
34+
.list-ios.list-inset ion-item:only-child,
35+
.list-ios.list-inset ion-item:not(:only-of-type):last-of-type,
36+
.list-ios.list-inset ion-item-sliding:last-of-type ion-item {
2337
--border-width: 0;
2438
--inner-border-width: 0;
2539
}
2640

27-
2841
.list-ios.list-inset + ion-list.list-inset {
2942
@include margin(0, null, null, null);
3043
}

‎core/src/components/list/list.md.scss‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,50 @@
2323
@include border-radius($list-inset-md-border-radius);
2424
}
2525

26-
.list-md.list-inset ion-item:first-child {
26+
/**
27+
* These selectors ensure the first item in the list
28+
* has the correct radius.
29+
* We need to consider the following scenarios:
30+
1. The first item in a list as long as it is not the only item.
31+
2. The item in the first item-sliding in a list.
32+
* Note that we do not select "ion-item-sliding ion-item:first-of-type"
33+
* because that will cause the borders to disappear on
34+
* items in an item-sliding when the item is the first
35+
* element in the item-sliding container.
36+
*/
37+
.list-md.list-inset ion-item:not(:only-of-type):first-of-type,
38+
.list-md.list-inset ion-item-sliding:first-of-type ion-item {
2739
--border-radius: #{$list-inset-md-border-radius $list-inset-md-border-radius 0 0};
2840
}
2941

30-
.list-md.list-inset ion-item:last-child {
31-
--border-radius: #{0 0 $list-inset-md-border-radius, $list-inset-md-border-radius};
42+
/**
43+
* These selectors ensure the last item in the list
44+
* has the correct radius and border.
45+
* We need to consider the following scenarios:
46+
1. The last item in a list as long as it is not the only item.
47+
2. The item in the last item-sliding in a list.
48+
* Note that we do not select "ion-item-sliding ion-item:last-of-type"
49+
* because that will cause the borders to disappear on
50+
* items in an item-sliding when the item is the last
51+
* element in the item-sliding container.
52+
*/
53+
.list-md.list-inset ion-item:not(:only-of-type):last-of-type,
54+
.list-md.list-inset ion-item-sliding:last-of-type ion-item {
55+
--border-radius: #{0 0 $list-inset-md-border-radius $list-inset-md-border-radius};
56+
--border-width: 0;
57+
--inner-border-width: 0;
58+
}
59+
60+
/**
61+
* The only item in a list should have a border radius
62+
* on all corners.
63+
* We target :only-child instead of :only-of-type
64+
* otherwise borders will disappear on items inside of
65+
* ion-item-sliding because the item will be the only
66+
* one of its type inside of the ion-item-sliding group.
67+
*/
68+
.list-md.list-inset ion-item:only-child {
69+
--border-radius: #{$list-inset-md-border-radius};
3270
--border-width: 0;
3371
--inner-border-width: 0;
3472
}

‎core/src/components/list/test/lines/list.e2e.ts‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,162 @@ configs().forEach(({ title, screenshot, config }) => {
2626
});
2727
});
2828
});
29+
30+
/**
31+
* Padding and border color ensures the bottom border can be easily seen if it regresses.
32+
* The background color ensures that any border radius values can be seen.
33+
*/
34+
configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
35+
test.describe(title('list: lines with children'), () => {
36+
test('only item in inset list should not have line', async ({ page }) => {
37+
test.info().annotations.push({
38+
type: 'issue',
39+
description: 'https://github.com/ionic-team/ionic-framework/issues/28435',
40+
});
41+
await page.setContent(
42+
`
43+
<style>
44+
#container {
45+
padding: 10px;
46+
background: #0088cc;
47+
}
48+
49+
ion-item {
50+
--border-color: red;
51+
}
52+
</style>
53+
<div id="container">
54+
<ion-list inset="true">
55+
<ion-item>
56+
<ion-label>Item 0</ion-label>
57+
</ion-item>
58+
</ion-list>
59+
</div>
60+
`,
61+
config
62+
);
63+
64+
const container = page.locator('#container');
65+
66+
await expect(container).toHaveScreenshot(screenshot('inset-list-only-item-no-lines'));
67+
});
68+
test('last item in inset list with end options should not have line', async ({ page }) => {
69+
test.info().annotations.push({
70+
type: 'issue',
71+
description: 'https://github.com/ionic-team/ionic-framework/issues/28435',
72+
});
73+
await page.setContent(
74+
`
75+
<style>
76+
#container {
77+
padding: 10px;
78+
background: #0088cc;
79+
}
80+
81+
ion-item {
82+
--border-color: red;
83+
}
84+
</style>
85+
<div id="container">
86+
<ion-list inset="true">
87+
<ion-item-sliding>
88+
<ion-item>
89+
<ion-label>Item 0</ion-label>
90+
</ion-item>
91+
<ion-item-options slot="end">
92+
<ion-item-option color="warning">
93+
<ion-icon slot="icon-only" name="pin"></ion-icon>
94+
</ion-item-option>
95+
</ion-item-options>
96+
</ion-item-sliding>
97+
98+
<ion-item-sliding>
99+
<ion-item>
100+
<ion-label>Item 1</ion-label>
101+
</ion-item>
102+
<ion-item-options slot="end">
103+
<ion-item-option color="warning">
104+
<ion-icon slot="icon-only" name="pin"></ion-icon>
105+
</ion-item-option>
106+
</ion-item-options>
107+
</ion-item-sliding>
108+
109+
<ion-item-sliding>
110+
<ion-item>
111+
<ion-label>Item 2</ion-label>
112+
</ion-item>
113+
<ion-item-options slot="end">
114+
<ion-item-option color="warning">
115+
<ion-icon slot="icon-only" name="pin"></ion-icon>
116+
</ion-item-option>
117+
</ion-item-options>
118+
</ion-item-sliding>
119+
</ion-list>
120+
</div>
121+
`,
122+
config
123+
);
124+
125+
const container = page.locator('#container');
126+
127+
await expect(container).toHaveScreenshot(screenshot('inset-list-end-options-no-lines'));
128+
});
129+
test('last item in inset list with start options should not have line', async ({ page }) => {
130+
await page.setContent(
131+
`
132+
<style>
133+
#container {
134+
padding: 10px;
135+
background: #0088cc;
136+
}
137+
138+
ion-item {
139+
--border-color: red;
140+
}
141+
</style>
142+
<div id="container">
143+
<ion-list inset="true">
144+
<ion-item-sliding>
145+
<ion-item-options slot="start">
146+
<ion-item-option color="warning">
147+
<ion-icon slot="icon-only" name="pin"></ion-icon>
148+
</ion-item-option>
149+
</ion-item-options>
150+
<ion-item>
151+
<ion-label>Item 0</ion-label>
152+
</ion-item>
153+
</ion-item-sliding>
154+
155+
<ion-item-sliding>
156+
<ion-item-options slot="start">
157+
<ion-item-option color="warning">
158+
<ion-icon slot="icon-only" name="pin"></ion-icon>
159+
</ion-item-option>
160+
</ion-item-options>
161+
<ion-item>
162+
<ion-label>Item 1</ion-label>
163+
</ion-item>
164+
</ion-item-sliding>
165+
166+
<ion-item-sliding>
167+
<ion-item-options slot="start">
168+
<ion-item-option color="warning">
169+
<ion-icon slot="icon-only" name="pin"></ion-icon>
170+
</ion-item-option>
171+
</ion-item-options>
172+
<ion-item>
173+
<ion-label>Item 2</ion-label>
174+
</ion-item>
175+
</ion-item-sliding>
176+
</ion-list>
177+
</div>
178+
`,
179+
config
180+
);
181+
182+
const container = page.locator('#container');
183+
184+
await expect(container).toHaveScreenshot(screenshot('inset-list-start-options-no-lines'));
185+
});
186+
});
187+
});

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-end-options-no-lines-ios-ltr-Mobile-Chrome-linux.png‎

4 KB
Loading

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-end-options-no-lines-ios-ltr-Mobile-Firefox-linux.png‎

5.15 KB
Loading

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-end-options-no-lines-ios-ltr-Mobile-Safari-linux.png‎

3.63 KB
Loading

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-end-options-no-lines-md-ltr-Mobile-Chrome-linux.png‎

3.65 KB
Loading

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-end-options-no-lines-md-ltr-Mobile-Firefox-linux.png‎

4.88 KB
Loading

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-end-options-no-lines-md-ltr-Mobile-Safari-linux.png‎

3.01 KB
Loading

‎core/src/components/list/test/lines/list.e2e.ts-snapshots/inset-list-only-item-no-lines-ios-ltr-Mobile-Chrome-linux.png‎

1.94 KB
Loading

0 commit comments

Comments
 (0)