Skip to content

Commit 5cdfac8

Browse files
authored
fix(refresher): show when content is fullscreen (#29608)
Issue number: resolves #18714 --------- <!-- 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 the `ion-content` has the fullscreen attribute, the `ion-refresher` will be hidden while refreshing. This can be seen by dragging far enough to trigger it to snap back and refresh. The refresher ends up being hidden behind the background content element. https://github.com/ionic-team/ionic-framework/assets/13530427/27b5393b-dd31-44a5-b872-97709e3a0980 ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Set the `--offset-top` to `0px` for the background content element. This reflects the same behavior of when the content is not fullscreen. By setting this to `0px`, the refresher is visible while refreshing. - Added a private prop within refresher to keep track of whether `ion-content` is `fullscreen` or not. - Added test. Originally, I was going to update the `pullMin` and `pullMax` as agreed on from the investigation ticket. However, it ended up adding too much space between the refresher and the content. This is the reason why I decided to modify the background background instead. Otherwise, it wouldn't mimic the behavior when content doesn't have the `fullscreen` attribute. Example of what the spacing looked like: https://github.com/ionic-team/ionic-framework/assets/13530427/389cea62-48c1-4464-be47-44bc3b6c0315 ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> [Preview](https://ionic-framework-git-rou-4950-ionic1.vercel.app/src/components/refresher/test/fullscreen) How to test: 1. Navigate to the preview page 2. Use the browser's simulator to chose an iOS device (might need to refresh the page) 3. Drag the screen down 4. Verify that the refreshing text is shown 5. Use the browser's simulator to chose an Android device (might need to refresh the page) 6. Drag the screen down 7. Verify that the refreshing text is shown
1 parent 9cec843 commit 5cdfac8

9 files changed

+215
-0
lines changed

core/src/components/refresher/refresher.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export class Refresher implements ComponentInterface {
4848
private pointerDown = false;
4949
private needsCompletion = false;
5050
private didRefresh = false;
51+
private contentFullscreen = false;
5152
private lastVelocityY = 0;
5253
private elementToTransform?: HTMLElement;
5354
private animations: Animation[] = [];
@@ -476,6 +477,12 @@ export class Refresher implements ComponentInterface {
476477
* Query the background content element from the host ion-content element directly.
477478
*/
478479
this.backgroundContentEl = await contentEl.getBackgroundElement();
480+
/**
481+
* Check if the content element is fullscreen to apply the correct styles
482+
* when the refresher is refreshing. Otherwise, the refresher will be
483+
* hidden because it is positioned behind the background content element.
484+
*/
485+
this.contentFullscreen = contentEl.fullscreen;
479486

480487
if (await shouldUseNativeRefresher(this.el, getIonMode(this))) {
481488
this.setupNativeRefresher(contentEl);
@@ -578,6 +585,15 @@ export class Refresher implements ComponentInterface {
578585
this.progress = 0;
579586
this.state = RefresherState.Inactive;
580587
this.memoizeOverflowStyle();
588+
589+
/**
590+
* If the content is fullscreen, then we need to
591+
* set the offset-top style on the background content
592+
* element to ensure that the refresher is shown.
593+
*/
594+
if (this.contentFullscreen && this.backgroundContentEl) {
595+
this.backgroundContentEl.style.setProperty('--offset-top', '0px');
596+
}
581597
}
582598

583599
private onMove(detail: GestureDetail) {
@@ -735,6 +751,18 @@ export class Refresher implements ComponentInterface {
735751
* user can scroll again.
736752
*/
737753
this.setCss(0, '0ms', false, '', true);
754+
755+
/**
756+
* Reset the offset-top style on the background content
757+
* when the refresher is no longer refreshing and the
758+
* content is fullscreen.
759+
*
760+
* This ensures that the behavior of background content
761+
* does not change when refreshing is complete.
762+
*/
763+
if (this.contentFullscreen && this.backgroundContentEl) {
764+
this.backgroundContentEl?.style.removeProperty('--offset-top');
765+
}
738766
}, 600);
739767

740768
// reset the styles on the scroll element
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
<!DOCTYPE html>
2+
<html lang="en" dir="ltr">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Refresher - Basic</title>
6+
<meta
7+
name="viewport"
8+
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
9+
/>
10+
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
11+
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
12+
<script src="../../../../../scripts/testing/scripts.js"></script>
13+
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
14+
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
15+
16+
<style>
17+
ion-toolbar {
18+
--opacity: 0.5;
19+
}
20+
</style>
21+
</head>
22+
23+
<body>
24+
<ion-app>
25+
<ion-header>
26+
<ion-toolbar>
27+
<ion-title>Pull To Refresh</ion-title>
28+
</ion-toolbar>
29+
</ion-header>
30+
31+
<ion-content fullscreen>
32+
<ion-refresher id="refresher" slot="fixed">
33+
<ion-refresher-content
34+
pulling-icon="arrow-down-outline"
35+
pulling-text="Pull to refresh..."
36+
refreshing-text="Refreshing..."
37+
refreshing-spinner="circles"
38+
>
39+
</ion-refresher-content>
40+
</ion-refresher>
41+
42+
<p>
43+
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean sed tellus nec mauris auctor dignissim
44+
fermentum in risus. Sed nec convallis sapien, id tincidunt enim. Mauris ornare eleifend nunc id mattis. Fusce
45+
augue diam, sagittis nec posuere at, consectetur tempor lectus. Nulla at lectus eget mauris iaculis malesuada
46+
mollis sed neque. Curabitur et risus tristique, malesuada mauris finibus, elementum massa. Proin lacinia
47+
mauris quis ligula blandit ullamcorper. Donec ut posuere lorem. In volutpat magna vitae tellus posuere
48+
pulvinar. Nam varius ligula justo, nec placerat lacus pharetra ac. Aenean massa orci, tristique in nisl ut,
49+
aliquet consectetur libero. Etiam luctus placerat vulputate. Aliquam ipsum massa, porttitor at mollis ut,
50+
pretium sit amet mi. In neque mauris, placerat et neque vel, tempor interdum dolor. Suspendisse gravida
51+
malesuada tellus, vel dapibus nisl dignissim vel. Cras ut nulla sit amet erat malesuada euismod vel a nulla.
52+
</p>
53+
<p>
54+
Phasellus sit amet iaculis odio, eget feugiat erat. Etiam sit amet turpis sit amet massa viverra maximus.
55+
Aenean venenatis porttitor pharetra. Fusce vulputate urna purus, vel efficitur mauris auctor non. Etiam libero
56+
odio, sodales in velit a, faucibus venenatis erat. Ut convallis sit amet urna in ultrices. Cras neque est,
57+
vehicula sed lorem ac, placerat commodo elit. Praesent turpis metus, elementum eget iaculis ac, elementum in
58+
odio. Nunc et elit faucibus, condimentum mauris consequat, ornare dolor. Sed ac lectus a est blandit tempor.
59+
Etiam lobortis tristique maximus.
60+
</p>
61+
<p>
62+
Quisque tempus porttitor massa, vel condimentum risus finibus a. Aliquam viverra maximus odio, id ornare justo
63+
tristique ac. Mauris euismod arcu eget neque sagittis rutrum. Ut vehicula porta lacus nec lobortis. Vestibulum
64+
et elit ultrices, lacinia metus in, lobortis est. Vivamus nisi justo, venenatis sit amet arcu ac, congue
65+
faucibus justo. Duis volutpat posuere enim, vel sagittis elit dictum et. Sed et congue mauris. Nam venenatis
66+
venenatis risus, ac condimentum neque sagittis sed. In eget nulla ultricies urna sollicitudin posuere. Aenean
67+
sagittis congue mauris. Proin nec libero mi. In hac habitasse platea dictumst. Praesent nunc nulla, dictum id
68+
molestie sed, pretium vitae turpis.
69+
</p>
70+
<p>
71+
Pellentesque vitae dapibus lacus. Nullam suscipit ornare risus quis ullamcorper. Nullam feugiat, sapien et
72+
sodales fermentum, risus ligula semper risus, id efficitur ligula augue id diam. Suspendisse lobortis est sit
73+
amet quam facilisis, ut vestibulum nunc dignissim. Donec at vestibulum magna. Maecenas maximus pretium metus.
74+
Phasellus congue sapien vel odio imperdiet, nec mollis odio euismod. Sed vel eros ut sapien accumsan
75+
condimentum vehicula vitae lectus. Donec sed efficitur lorem. Aenean tristique mi libero, eleifend tincidunt
76+
libero finibus at. Mauris condimentum fermentum rutrum.
77+
</p>
78+
<p>
79+
Nulla tristique ultricies suscipit. Donec non ornare elit. Vivamus id pretium mauris, nec sagittis leo. Fusce
80+
mattis eget est id sollicitudin. Suspendisse dictum sem magna, in imperdiet metus suscipit et. Suspendisse
81+
enim enim, venenatis et orci eu, suscipit congue lacus. Praesent vel ligula non eros tempor interdum. Proin
82+
justo orci, ultricies vitae diam sed, semper consectetur ligula. Aenean finibus ante velit, nec efficitur
83+
libero cursus cursus. Duis mi nunc, imperdiet sed condimentum vel, porttitor ut lacus. Quisque dui ipsum,
84+
vehicula sed vestibulum id, semper vel libero. Suspendisse tincidunt mollis condimentum. Nulla facilisi. Etiam
85+
neque nisl, egestas nec iaculis sed, tristique faucibus sem. Sed mollis dui quis ligula cursus rutrum.
86+
</p>
87+
</ion-content>
88+
</ion-app>
89+
<ion-menu-controller></ion-menu-controller>
90+
91+
<script>
92+
const refresher = document.getElementById('refresher');
93+
94+
refresher.addEventListener('ionRefresh', () => {
95+
setTimeout(() => {
96+
refresher.complete();
97+
// Custom event consumed by e2e tests
98+
window.dispatchEvent(new CustomEvent('ionRefreshComplete'));
99+
}, 500);
100+
});
101+
</script>
102+
</body>
103+
</html>
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
import { pullToRefresh } from '../test.utils';
5+
6+
/**
7+
* This behavior does not vary across directions.
8+
*/
9+
configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => {
10+
test.describe(title('refresher: fullscreen content'), () => {
11+
test.beforeEach(async ({ page }) => {
12+
/**
13+
* Do not call `complete()` from `ion-refresher` in this test.
14+
* This will allow the refresher to "pause" while refreshing.
15+
* By pausing, the test can verify that the refresher is
16+
* completely visible when the content is fullscreen.
17+
*/
18+
await page.setContent(
19+
`
20+
<ion-header>
21+
<ion-toolbar>
22+
<ion-title>Pull To Refresh</ion-title>
23+
</ion-toolbar>
24+
</ion-header>
25+
26+
<ion-content fullscreen>
27+
<ion-refresher id="refresher" slot="fixed">
28+
<ion-refresher-content
29+
pulling-icon="arrow-down-outline"
30+
pulling-text="Pull to refresh..."
31+
refreshing-text="Refreshing..."
32+
refreshing-spinner="circles"
33+
>
34+
</ion-refresher-content>
35+
</ion-refresher>
36+
37+
<p>Pull this content down to trigger the refresh.</p>
38+
</ion-content>
39+
40+
<script>
41+
const refresher = document.getElementById('refresher');
42+
43+
refresher.addEventListener('ionRefresh', () => {
44+
setTimeout(() => {
45+
// Custom event consumed by e2e tests
46+
window.dispatchEvent(new CustomEvent('ionRefreshComplete'));
47+
}, 500);
48+
});
49+
</script>
50+
`,
51+
config
52+
);
53+
});
54+
55+
// Bug only occurs with the legacy refresher.
56+
test.describe('legacy refresher', () => {
57+
test('should display when content is fullscreen', async ({ page, browserName }) => {
58+
test.info().annotations.push({
59+
type: 'issue',
60+
description: 'https://github.com/ionic-team/ionic-framework/issues/18714',
61+
});
62+
63+
const refresher = page.locator('ion-refresher');
64+
65+
await pullToRefresh(page);
66+
67+
if (browserName === 'firefox') {
68+
/**
69+
* The drag is highlighting the text in Firefox for
70+
* some reason.
71+
*
72+
* Clicking the mouse will remove the highlight and
73+
* be more consistent with other browsers. This
74+
* does not happen in Firefox, just when running
75+
* in Playwright.
76+
*/
77+
await page.mouse.click(0, 0);
78+
}
79+
80+
await expect(refresher).toHaveScreenshot(screenshot('refresher-legacy-content-fullscreen'));
81+
});
82+
});
83+
});
84+
});

0 commit comments

Comments
 (0)