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

Fix Pull to Refresh (ion-refresher) #5207

Closed
matiboy opened this issue Jan 26, 2016 · 12 comments
Closed

Fix Pull to Refresh (ion-refresher) #5207

matiboy opened this issue Jan 26, 2016 · 12 comments
Assignees
Milestone

Comments

@matiboy
Copy link

matiboy commented Jan 26, 2016

Type: bug

Ionic Version: 2.x

Platform: ios 9 webview

- SASS file for ion-refresher is not included

  • Swipe events don't seem to trigger (tested on iOS emulator)

Followed the documentation here
http://ionicframework.com/docs/v2/api/components/scroll/Refresher/

Thanks!

@Ionitron Ionitron added the v2 label Jan 26, 2016
brandyscarney added a commit that referenced this issue Jan 26, 2016
…ever working

It’s still finishing the PTR before the complete is called though

References #5207
@brandyscarney
Copy link
Member

@adamdbradley canOverscroll was being set to false by default so PTR was never able to run, but I see some flickering on iOS and the complete function isn't being recognized.

@matiboy
Copy link
Author

matiboy commented Jan 27, 2016

@brandyscarney Thank you for being so responsive!
Related but different issue: in the docs, the refresh handler seems to receive $event and refresher

<ion-refresher (starting)="doStarting()"
                 (refresh)="doRefresh($event, refresher)"
                 (pulling)="doPulling($event, amt)">
  </ion-refresher>

but it seems that: $event actually comes as the Refresher object in doRefresh and refresher is ignored (no second parameter passed to doRefresh)

@brandyscarney brandyscarney self-assigned this Feb 3, 2016
@brandyscarney brandyscarney added this to the 2.0.0-beta.1 milestone Feb 3, 2016
@andrefedalto
Copy link

I was having some unexpected behaviour with the pull to refresh so I tried the exact example of the docs in a new clean ionic2 project (Hello Ionic) running alpha56. I still have the following bugs:

  1. doStarting() method is not being called. By the example from the docs I'd expect to see the "Pull started!" in the console but it never shows up.

  2. the refresher (icon-refreshing) won't stay in the screen. As soon as the pull is released, the arrow icon changes to the ionic icon and disappear. Is this the expected behaviour? Or should the icon-refreshing be there until the .complete() method is called?

  3. also as mentioned before (or in another issue), if you have a long list (bigger than the screen) and scroll down, when scrolling up the refresh is triggered. It should be trigged only on pulling from the top of your content.

Hope this helps :)

@sb8244
Copy link

sb8244 commented Feb 15, 2016

I made a duplicate, oops #5456

Addressing @andrefedalto above. I've done some digging as well, and confirmed 1&2:

  1. The code for activation has the next() commented out. Removing it triggers start:
    Refresher.prototype.activate = function () {
        //this.ele.classList.add('active');
        this.isActive = true;
        //this.starting.next();
    };
  1. setScrollLock is being called which calls deactivate. It looks like it's being called immediately after the tail animation begins

From what I see, It looks like this bit of code is being implemented still, although it comes off from the documentation as it working.

brandyscarney added a commit that referenced this issue Feb 16, 2016
…to emit the refresher

Cleaned up the API docs for scroll and test also.

References #5207
@brandyscarney brandyscarney changed the title bug: <ion-refresher> not working in Alpha51 Fix Pull to Refresh (ion-refresher) Feb 17, 2016
@brandyscarney
Copy link
Member

I added a few fixes for emitting events, including to emit the starting event now. These fixes should get in with the beta.1 release. Going to look into the other issues for the beta.2 release. Let me know if you find any more issues before then! 😄

brandyscarney added a commit that referenced this issue Feb 17, 2016
updated docs to reflect new name and renamed an internal function

BREAKING CHANGE: starting event no longer exists, must use start

References #5207
@brandyscarney
Copy link
Member

The event (starting) has been renamed to (start) as well.

@wangqianbo
Copy link

ptrThreshold is set to 0 in class Refresher, I think this is the reason of the issue 2) which was addressed by @andrefedalto .
if (this.lastOverscroll > this.ptrThreshold) { this.startRefresh(); this.scrollTo(this.ptrThreshold, this.scrollTime);
Should the Refresher Component expose enough attributes so we can control the feature of it?

brandyscarney added a commit that referenced this issue Feb 22, 2016
…until complete is called

Removed the code manually adding classes and used host instead. Fixed
an issue with the tail refresh class not being removed after the first
pull. Added Sass variables to customize the refresher more - will add
even more to this.

References #5207
@brandyscarney
Copy link
Member

Fixed most of the existing bugs:

  1. Refresher will now display until the complete function is called
  2. start event is now being emitted
  3. API docs updated to show use case with all attributes
  4. Sass variables were added to customize

Remaining issues:

  1. If you scroll down in the list and start to drag back up it pops to the top of the list, on release it will go back to your previous position
  2. ion-refresher pulling icon #5485
  3. perf: Swipe to refresh is utterly bad on Android #5550
  4. Implement MD pull to refresh style #5557
  5. Make sure this is not an issue still before release: refresher jump to bottom on iOS 9.2.1 iphone6 #5585

@adamdbradley
Copy link
Contributor

All of these have been addressed in this refactor: 91df5f9

Thanks

@Barathwaja
Copy link

Barathwaja commented Sep 2, 2016

Ref. : #7914 Ion-refresher functionality works but refresher icon and other stuffs are blank in v2.

@jayzyaj
Copy link

jayzyaj commented Jul 18, 2018

Hi there. I am having a problem with ion-slides. This is the link

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 1, 2018

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 Sep 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants