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

[Improvement] The view should complete the fade in and growth animations before fading out. #9

Closed
NSExceptional opened this issue Jan 21, 2015 · 16 comments

Comments

@NSExceptional
Copy link

I've implemented a fix myself, and it looks a lot better! It can be easily accomplished by adding a void block property to the class and queuing the removeBackground and removeCircle animations to be called in animationDidStop::

@property (nonatomic, copy) void (^removalAnimationsQueue)();
...
if (self.removalAnimationsQueue) {
    self.removalAnimationsQueue()
    self.removalAnimationsQueue = nil;
}

For example, in setSelected:animated:, the if (!selected) statement can be modified like so:

if (!selected) {
    if (!self.growthFinished) {
          // Not done yet, queue removeBackground call

          void (^oldBlock)() = self.removalAnimationsQueue;
          __weak typeof(self) weakSelf = self;

          self.removalAnimationsQueue = ^void() {
               if (oldBlock)
                    oldBlock();
               [weakSelf removeBackground];
        };
    }
    else {
        [self removeBackground];
    }
}

And in touchesEnded:withEvent:...

if (self.growthFinished) {
    [self removeCircle];
}
else {
     // Not done yet, queue removeCircle call

     __weak typeof(self) weakSelf = self;
     void (^oldBlock)() = self.removalAnimationsQueue;

     self.removalAnimationsQueue = ^void() {
          if (oldBlock)
               oldBlock();
          [weakSelf removeBackground];
    };
}

And that should do it. :)

@bfeher
Copy link
Owner

bfeher commented Jan 21, 2015

Hi @ThePantsThief !
I'm open to this idea if it looks better, even though the Google Polymer library does not complete the fade in and growth before fading out. If you try their controls here you can see what I'm talking about. However they are much smoother than the current implementation...I'm working on that in my spare time.

I must have done something wrong when implementing your changes because the circles are not being removed and destroyed. They linger and cause a memory leak:
thepantstheif-fix

Would you mind posting your full code (image, text, or link to your repo) or email me (ben.feher@gmail.com) the file so I can do it properly? I'd like to see it in action :)

Thanks!

@NSExceptional
Copy link
Author

Hey! Thanks for the speedy reply :)

Where are you calling (and then nil-ing out) the block I suggested you add? I had that problem at first, but it was because I forgot to actually call the block, haha. Here, take a look at my subclass. Specifically, animationDidStop:finished:, removeBackground, and removeCircle (the finally_ methods are a workaround for calling a super method from within a block; replace them with the original implementations).

I made a few changes to your .h and .m files to modify the behavior of the animations (some of which are unrelated to the issue I filed). By the way, it would be awesome if you could make some of those constants properties of the class so they can be modified without changing your code, haha.

One other thing: in growTapCircle:, it looks like the desired result (if no diameter is specified) is to draw the circle so that it covers the entire cell, right? Choosing the larger of the width and height of the cell won't actually cover the entire cell, even though it may look like it if the cell is a thin rectangle. To cover the entire cell, you have to adjust the diameter so that it reaches all corners of the cell from it's origin, using the Pythagorean theorem. If that is the intended result, take a look at growTapCircle: in the attached zip, I whipped up a fix. Here's a screenshot:

Tanner Bennett

On Jan 20, 2015, at 8:31 PM, Bence Feher notifications@github.com wrote:

Hi @ThePantsThief https://github.com/ThePantsThief !
I'm open to this idea if it looks better, even though the Google Polymer library does not complete the fade in and growth before fading out. If you try their controls here http://www.polymer-project.org/components/paper-elements/demo.html#paper-item you can see what I'm talking about. However they are much smoother than the current implementation...I'm working on that in my spare time.

I must have done something wrong when implementing your changes because the circles are not being removed and destroyed. They linger and cause a memory leak:
https://cloud.githubusercontent.com/assets/3466708/5830188/81aed1fc-a160-11e4-863f-80fd72937955.gif
Would you mind posting your full code (image, text, link to your repo) or email me (ben.feher@gmail.com mailto:ben.feher@gmail.com) the file so I can do it properly? I'd like to see it in action :)

Thanks!


Reply to this email directly or view it on GitHub #9 (comment).

@bfeher
Copy link
Owner

bfeher commented Jan 21, 2015

Thank you!
I might be doing github wrong, but I don't see any of your files or screenshots.

Yeah I really should and will change a lot of those constants to properties. When I first started the project I was thinking to myself that I wanted to mimic Google's Polymer controls as closely as possible, so I didn't have much customization in mind. Then it evolved.

About the circle diameter: again, when I first started it seemed that the Polymer controls which were in beta or alpha(?) at the time didn't have a clear set of defined rules. I noticed that every control's circle stopped just a few pixels short of the full frame. I had worked out some rough estimates by zooming in on frames that I recorded as an animated gif. But now it seems that Polymer has changed yet again and the circles are covering it all.

Anyways, I'd love to take a peek at your solutions and implement them into the library (writing your name into the source of course). You are right, I wasn't calling the block hahaha.

@NSExceptional
Copy link
Author

Oh, this is a github email. That explains why nothing showed up. I just sent a few attachments from Mail.app.

Tanner Bennett

On Jan 20, 2015, at 11:20 PM, Bence Feher notifications@github.com wrote:

Thank you!
I might be doing github wrong, but I don't see any of your files or screenshots.

Yeah I really should and will change a lot of those constants to properties. When I first started the project I was thinking to myself that I wanted to mimic Google's Polymer controls as closely as possible, so I didn't have much customization in mind. Then it evolved.

About the circle diameter: again, when I first started it seemed that the Polymer controls which were in beta or alpha(?) at the time didn't have a clear set of defined rules. I noticed that every control's circle stopped just a few pixels short of the full frame. I had worked out some rough estimates by zooming in on frames that I recorded as an animated gif. But now it seems that Polymer has changed yet again and the circles are covering it all.

Anyways, I'd love to take a peek at your solutions and implement them into the library (writing your name into the source of course). You are right, I wasn't calling the block hahaha.


Reply to this email directly or view it on GitHub.

@NSExceptional
Copy link
Author

I'm away from my computer, I'll resent them through github when I get a chance. Or I can forward the original message to another address if you'd prefer

Tanner Bennett

On Jan 20, 2015, at 11:20 PM, Bence Feher notifications@github.com wrote:

Thank you!
I might be doing github wrong, but I don't see any of your files or screenshots.

Yeah I really should and will change a lot of those constants to properties. When I first started the project I was thinking to myself that I wanted to mimic Google's Polymer controls as closely as possible, so I didn't have much customization in mind. Then it evolved.

About the circle diameter: again, when I first started it seemed that the Polymer controls which were in beta or alpha(?) at the time didn't have a clear set of defined rules. I noticed that every control's circle stopped just a few pixels short of the full frame. I had worked out some rough estimates by zooming in on frames that I recorded as an animated gif. But now it seems that Polymer has changed yet again and the circles are covering it all.

Anyways, I'd love to take a peek at your solutions and implement them into the library (writing your name into the source of course). You are right, I wasn't calling the block hahaha.


Reply to this email directly or view it on GitHub.

@bfeher
Copy link
Owner

bfeher commented Jan 21, 2015

If you could forward them ben.feher@gmail.com that would be nice. I can wait though. Whichever is easiest for you.

Thanks!

@bfeher
Copy link
Owner

bfeher commented Jan 30, 2015

Whew its been a while. Updated to version 2.1.16
Ok, I added the property BOOL alwaysCompleteFullAnimation which is YES by default. This runs your code with the blocks to always complete the animation. It can be disabled by setting the property to NO. It includes bursting the circle just incase your circle isn't taking up the full view.

I also added your code to calculate a tap-circle diameter which will cover the view perfectly. You can set the tapCircleDiameter = bfPaperTableViewCell_tapCircleDiameterFull to automatically get this new effect. Thanks a lot!

Along with those additions, the circle and background drawing and animations have been revamped with the same code as my other BFPaper components. The animations now pick up where they left off and run much smoother, using nothing but CALayers and CAShapeLayers.

Thanks a lot for your help. Take a look and let me know if you find any more bugs or have any more feature requests. By the way, I didn't make the tapGestureRecognizer public yet ;p You will have to do that yourself again sorry.

@bfeher
Copy link
Owner

bfeher commented Jan 30, 2015

Oh also you are written in the source code in two places, as well as in the git commit logs for the commit which added your features. Thanks!

@NSExceptional
Copy link
Author

Thanks so much :D Nice work!

Tanner Bennett

On Jan 30, 2015, at 1:13 AM, Bence Feher notifications@github.com wrote:

Whew its been a while. Ok, I added the property BOOL alwaysCompleteFullAnimation which is YES by default. This runs your code with the blocks to always complete the animation. It can be disabled by setting the property to NO. It includes bursting the circle just incase your circle isn't taking up the full view.

I also added your code to calculate a tap-circle diameter which will cover the view perfectly. You can set the tapCircleDiameter = bfPaperTableViewCell_tapCircleDiameterFull to automatically get this new effect. Thanks a lot!

Along with those additions, the circle and background drawing and animations have been revamped with the same code as my other BFPaper components. The animations now pick up where they left off and run much smoother, using nothing but CALayers and CAShapeLayers.

Thanks a lot for your help. Take a look and let me know if you find any more bugs or have any more feature requests. By the way, I didn't make the tapGestureRecognizer public yet ;p You will have to do that yourself again sorry.


Reply to this email directly or view it on GitHub.

@bfeher bfeher closed this as completed Feb 2, 2015
@NSExceptional
Copy link
Author

Is there a way to prevent the cells from animating when you're scrolling? I've been fiddling with this for a few hours and I can't find a way to make it not animate if the table is scrolling when your finger touches down.

Tanner Bennett

On Jan 30, 2015, at 7:35 AM, Tanner Bennett tannerbennett@me.com wrote:

Thanks so much :D Nice work!

Tanner Bennett

On Jan 30, 2015, at 1:13 AM, Bence Feher notifications@github.com wrote:

Whew its been a while. Ok, I added the property BOOL alwaysCompleteFullAnimation which is YES by default. This runs your code with the blocks to always complete the animation. It can be disabled by setting the property to NO. It includes bursting the circle just incase your circle isn't taking up the full view.

I also added your code to calculate a tap-circle diameter which will cover the view perfectly. You can set the tapCircleDiameter = bfPaperTableViewCell_tapCircleDiameterFull to automatically get this new effect. Thanks a lot!

Along with those additions, the circle and background drawing and animations have been revamped with the same code as my other BFPaper components. The animations now pick up where they left off and run much smoother, using nothing but CALayers and CAShapeLayers.

Thanks a lot for your help. Take a look and let me know if you find any more bugs or have any more feature requests. By the way, I didn't make the tapGestureRecognizer public yet ;p You will have to do that yourself again sorry.


Reply to this email directly or view it on GitHub.

@bfeher
Copy link
Owner

bfeher commented Feb 15, 2015

Hmm I'm not sure. I know that if you move the table then
- (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event
Gets called. You could maybe kill all animations there?

Sorry ill take a better look on Monday.

@NSExceptional
Copy link
Author

I got off my computer after sending that but just before I did, I had an idea of maybe delaying the animation until the touches weren't canceled, like how selection works when you have tableView.delaysContentTouches enabled.

Tanner Bennett

On Feb 15, 2015, at 1:33 AM, Bence Feher notifications@github.com wrote:

Hmm I'm not sure. I know that if you move the table then

  • (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event
    Gets called. You could maybe kill all animations there?

Sorry ill take a better look on Monday.


Reply to this email directly or view it on GitHub.

@NSExceptional
Copy link
Author

A few other things I noticed, while I'm here!

  1. beganHighlight and beganSelection are unused (private) properties
  2. Possible bug I encountered while testing some modifications to the class to delay the animation:

tapCircle will be nil if self.rippleAnimationQueue is empty, so rearranging the code to this should make sure you never try to add a nil object to self.deathRowForCircleLayers:

Tanner Bennett

On Feb 15, 2015, at 1:33 AM, Bence Feher notifications@github.com wrote:

Hmm I'm not sure. I know that if you move the table then

  • (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event
    Gets called. You could maybe kill all animations there?

Sorry ill take a better look on Monday.


Reply to this email directly or view it on GitHub #9 (comment).

@bfeher
Copy link
Owner

bfeher commented Feb 17, 2015

Thanks for the heads up! I'll fix those next.

As for the tap circles not appearing when you just scroll through:
Have you made any progress? All I can think of would be to just use a LongPressGestureRecognizer instead to capture the touches. This way we can delay it. The delay can even be a public property with a default value.

@bfeher
Copy link
Owner

bfeher commented Feb 19, 2015

I pushed version 2.2.1 which removed some vestigial code as well as added a new public property: CGFloat tapDelay. The default value is 0.1f and this should prevent cells from animating while just scrolling through.

Thanks again and as always, I appreciate any help or bugs you find :)

@NSExceptional
Copy link
Author

Huh, cool. Thanks again yourself! :) Sorry I never got back to you, it's exam week

Tanner Bennett

On Feb 18, 2015, at 9:58 PM, Bence Feher notifications@github.com wrote:

I pushed version 2.2.1 which removed some vestigial code as well as added a new public property: CGFloat tapDelay. The default value is 0.1f and this should prevent cells from animating while just scrolling through.

Thanks again and as always, I appreciate any help or bugs you find :)


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants