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: collapsible header resets scroll position when triggered by iOS overscroll #19839

Closed
DavidStrausz opened this issue Nov 5, 2019 · 7 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@DavidStrausz
Copy link
Contributor

Bug Report

Ionic version:
[x] 4.x

Current behavior:
When using a collapsible header with a large title on a page that's not high enough to scroll only the iOS overscroll functionality takes effect. When the page is scrolled until the point where the opacity of the main and the scroll header is toggled the scroll position is reset which causes a quit ugly and janky behaviour.

ezgif com-crop

Expected behavior:
It works the same way no matter if its normal scrolling or overscrolling.

Steps to reproduce:
Place a collapsible header in an otherwise empty ion-content, run it on an iOS device and try to scroll the page until the point where the scroll header should be hidden and the main header should be visible.

Related code:
If I remove all function calls where the scroll header is activated/deactivated:
https://github.com/ionic-team/ionic/blob/0a7aae28a7eb0270cdcd100933c01850403b66db/core/src/components/header/header.utils.ts#L104
https://github.com/ionic-team/ionic/blob/0a7aae28a7eb0270cdcd100933c01850403b66db/core/src/components/header/header.utils.ts#L119
and also the calls where the opacity of the main toolbar is changed:
https://github.com/ionic-team/ionic/blob/0a7aae28a7eb0270cdcd100933c01850403b66db/core/src/components/header/header.utils.ts#L63

...it works like expected.

ezgif com-crop-2

Let me know if you should need an example repo, but it should be quite easy to reproduce it from the example code from the docs.

Other information:
Related to overscroll but more of a performance thing: you could possible save a few DOM writes here:
https://github.com/ionic-team/ionic/blob/0a7aae28a7eb0270cdcd100933c01850403b66db/core/src/components/header/header.utils.ts#L52
if you only actually scale the title if scrollTop is <= 0, overscroll(-top) is always a positive margin and always results in scale being 1.

Ionic info:

Ionic:
   Ionic CLI                     : 5.4.5 (/usr/local/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.11.3
   @angular-devkit/build-angular : 0.803.12
   @angular-devkit/schematics    : 8.3.12
   @angular/cli                  : 8.3.12
   @ionic/angular-toolkit        : 2.0.0

Cordova:
   Cordova CLI       : 9.0.0 (cordova-lib@9.0.1)
   Cordova Platforms : ios 5.0.1
   Cordova Plugins   : cordova-plugin-ionic-keyboard 2.2.0, cordova-plugin-ionic-webview 4.1.2, (and 26 other plugins)

Utility:
   cordova-res : not installed
   native-run  : not installed

System:
   ios-deploy : 1.9.2
   ios-sim    : 8.0.2
   NodeJS     : v12.4.0 (/usr/local/bin/node)
   npm        : 6.11.3
   OS         : macOS Catalina
   Xcode      : Xcode 11.2 Build version 11B52
@ionitron-bot ionitron-bot bot added the triage label Nov 5, 2019
@liamdebeasi
Copy link
Member

Thanks for the issue. I was able to reproduce this issue using the following markup:

    <ion-app>
      <ion-header>
        <ion-toolbar>
          <ion-title>Settings</ion-title>
        </ion-toolbar>
      </ion-header>
      
      <ion-content>
        <ion-header collapse="condense">
          <ion-toolbar>
            <ion-title size="large">Settings</ion-title>
          </ion-toolbar>
        </ion-header>
      </ion-content>
    </ion-app>

The issue goes away if disable this line: https://github.com/ionic-team/ionic/blob/0a7aae28a7eb0270cdcd100933c01850403b66db/core/src/components/header/header.utils.ts#L119

So far I haven't been able to reproduce this outside of Ionic, so I'm not sure if this is a WebKit bug that we'll need to find a workaround to, or a bug in Ionic Framework. I'll keep digging and update this thread when I have more info. Thanks!

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

I was able to reproduce this issue outside of Ionic: https://codepen.io/liamdebeasi/pen/xxxWNjM

I'm going to see if there's a way to work around this behavior. I will update this thread when I have more. Thanks!

@liamdebeasi
Copy link
Member

liamdebeasi commented Nov 6, 2019

Ok can you give this dev build a try and let me know if it fixes the issue for you?

npm i @ionic/angular@4.12.0-dev.201911062100.c73d095

@DavidStrausz
Copy link
Contributor Author

@liamdebeasi Its fixed in the dev build - great work, thanks for all the effort you put into this!

ezgif com-crop

But I'm seeing the issue described in #19682 again now, the main header title of the entering page is briefly visible when navigating:

ezgif com-crop-2

@liamdebeasi
Copy link
Member

liamdebeasi commented Nov 7, 2019

Glad to hear the issue is resolved! This dev build didn't include the fix in #19682 so that's why it still has that behavior.

@liamdebeasi
Copy link
Member

This issue has been resolved via #19850 and will be available in an upcoming release of Ionic Framework. Thanks!

@ionitron-bot
Copy link

ionitron-bot bot commented Dec 7, 2019

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 Dec 7, 2019
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

No branches or pull requests

2 participants