Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: ion-infinite-scroll ionInfinite event does not fire in ion-popovers #28455

Closed
3 tasks done
HighQualityWaschbaer opened this issue Nov 2, 2023 · 9 comments · Fixed by #28861
Closed
3 tasks done
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@HighQualityWaschbaer
Copy link

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

When using ion-infinite-scroll inside a ion-popover with an ion-list, the ionInfinite event does not fire once one has scrolled down to the bottom of the list.

Expected Behavior

The ionInfinite event should fire when scrolling down to the ion-infinite-scroll element in a ion-popover.

Steps to Reproduce

  1. Create a ion-popover and properly configure it
  2. Add a ion-list and fill it with content programmatically
  3. Add an ion-infinite-scroll after the list as per example in the ionic docs
  4. Add a function for the fired event that adds new content and completes the event
  5. Optionally have a console.log in the function to check that the event has been fired
  6. Run ionic serve and view the result

Code Reproduction URL

https://github.com/HighQualityWaschbaer/infinite-scroll-bug

Ionic Info

Ionic:

Ionic CLI : 6.20.8 (C:\Users\SRach\AppData\Roaming\npm\node_modules@ionic\cli)
Ionic Framework : not installed
@angular-devkit/build-angular : 16.2.9
@angular-devkit/schematics : 16.2.9
@angular/cli : 16.2.9
@ionic/angular-toolkit : 9.0.0

Capacitor:

Capacitor CLI : 5.5.1
@capacitor/android : not installed
@capacitor/core : 5.5.1
@capacitor/ios : not installed

Utility:

cordova-res : 0.15.4
native-run : 1.7.4

System:

NodeJS : v16.20.0 (C:\Program Files\nodejs\node.exe)
npm : 8.19.4
OS : Windows 10

Additional Information

No response

@liamdebeasi
Copy link
Contributor

Thanks for the report. The problem here is the .popover-viewport element has a height larger than the visible area of the popover. This means that when you scroll, you are scrolling that element and not the child ion-content. The infinite scroll component is listening for scroll events on ion-content which is why the infinite scroll event does not fire.

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Nov 6, 2023
@ionitron-bot ionitron-bot bot removed the triage label Nov 6, 2023
@liamdebeasi
Copy link
Contributor

This should be preventing this but .popover-viewport sits outside of the popover's Shadow DOM, so it never matches:

This likely broke when popover was updated to use the Shadow DOM. Fixing this should involve moving this style to a global stylesheet (i.e. ion-popover .popover-viewport).

@HighQualityWaschbaer
Copy link
Author

Is there an estimate for a fix, yet? (I don't know how long it usually takes for a fix to get into production)

@liamdebeasi
Copy link
Contributor

We don't provide time estimates for fixes. Someone from the team will post here when the issue has been resolved.

@HighQualityWaschbaer
Copy link
Author

We don't provide time estimates for fixes. Someone from the team will post here when the issue has been resolved.

Alright

@liamdebeasi
Copy link
Contributor

Here's a dev build with a proposed fix if you are interested in testing: 7.6.6-dev.11705696534.177666f8. Note that in your demo you'll want to make sure you initially load enough data such that your content can scroll.

@HighQualityWaschbaer
Copy link
Author

Here's a dev build with a proposed fix if you are interested in testing: 7.6.6-dev.11705696534.177666f8. Note that in your demo you'll want to make sure you initially load enough data such that your content can scroll.

I tested the new version and it works flawlessly :)

github-merge-queue bot pushed a commit that referenced this issue Jan 23, 2024
Issue number: resolves #28455

---------

<!-- 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 main `ion-content` inside of a popover is not scrollable. Instead,
the `.popover-viewport` container is scrolled because certain styles no
longer applied once we moved popover to the Shadow DOM. This bug
impacted the linked issue because it meant that the infinite scroll
inside of the content never fires (since the content never scrolls).

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

- The `.popover-viewport` rule now targets slotted content with
the`.popover-viewport` class. This class is added to the root node that
is slotted. This enables us to target the user's content in the light
dom.

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

Dev build: `7.6.6-dev.11705696534.177666f8`

---------

Co-authored-by: ionitron <hi@ionicframework.com>
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #28861, and a fix will be available in an upcoming release of Ionic Framework. Feel free to keep testing the dev build, and let me know if you run into any issues.

Copy link

ionitron-bot bot commented Feb 22, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants