ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.47k stars 13.53k forks source link

fix: check for scrollEl existence #29410

Closed stewones closed 2 weeks ago

stewones commented 2 weeks ago

ion-content is breaking CI environments when the scrollable element is not reachable and the consumer needs to run content.scrollToBottom()


What is the current behavior?

N/A

What is the new behavior?

N/A

Does this introduce a breaking change?

Other information

TypeError: Cannot read properties of undefined (reading 'scrollHeight')

      at Content.<anonymous> (../../node_modules/@ionic/core/components/ion-content.js:242:28)
      at fulfilled (../../node_modules/tslib/tslib.js:166:62)
      at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invoke (../../node_modules/zone.js/bundles/zone.umd.js:411:30)
      at AsyncTestZoneSpec.Object.<anonymous>.AsyncTestZoneSpec.onInvoke (../../node_modules/zone.js/bundles/zone-testing.umd.js:1261:47)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (../../node_modules/zone.js/bundles/zone-testing.umd.js:297:43)
vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 27, 2024 9:19pm
sean-perkins commented 2 weeks ago

Hello @stewones, can you create an issue with a minimal reproduction for me to test against?

The scroll element is set during render. I need more context to understand what issue is being experienced and why this change is the best way to resolve it.

e.g. If this issue impacts scrollToBottom, what about scrollToPoint and scrollToTop?

Thanks!

stewones commented 2 weeks ago

hey @sean-perkins just did it https://github.com/ionic-team/ionic-framework/issues/29419

stewones commented 2 weeks ago

regarding scrollToPoint and scrollToTop, they're prob broken as well. I'm happy to patch them as well if you think this should be baked into the core.