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

Issue 61: Feature request: Shadow support for handles #91

Conversation

redcapua
Copy link
Contributor

@redcapua redcapua commented Dec 4, 2018

Issue #61
Feature request: Shadow support for handles

Feature request: Shadow support for handles
@TomThorpe
Copy link
Owner

Thanks for this!

Just wanted to check, what’s the purpose of removing the left handle from the superview then adding the right handle back (twice) in the opacity and radius methods?

Thanks!
Tom

@redcapua
Copy link
Contributor Author

redcapua commented Dec 4, 2018

That was a typo.
Sorry for that :(

@TomThorpe
Copy link
Owner

No problem at all! :-)

But I just mean the [self.leftHandle removeFromSuperlayer]; , [self.layer addSublayer:self.leftHandle]; and [self.layer addSublayer:self.rightHandle]; lines entirely. I'm just wondering why they need to be there? Or if they can be removed entirely?

I dont think it should be needed to remove the handles from the superview and re-add them after the change? I've just tried checking out your branch and removing those lines completely and it still seemed to work fine - so I was just wondering if I had missed a reason for them being there?

Thanks again for your contribution :-)

@redcapua
Copy link
Contributor Author

redcapua commented Dec 4, 2018

I dont think it should be needed to remove the handles from the superview and re-add them after the change?

It is a habit :)
Remove object from view before changing to avoid conflict in the future changes.

In any case it is your repository, and your rules :)

@TomThorpe
Copy link
Owner

Gotcha. Yeah I think it's best to remove those lines if you dont mind, then we can get this merged! 👍

@redcapua
Copy link
Contributor Author

redcapua commented Dec 5, 2018 via email

    [self.leftHandle removeFromSuperlayer];
@redcapua
Copy link
Contributor Author

redcapua commented Dec 5, 2018

Lines:
[self.leftHandle removeFromSuperlayer];
have been removed.

@TomThorpe
Copy link
Owner

Thanks, but it was also the [self.layer addSublayer:self.leftHandle]; and [self.layer addSublayer:self.rightHandle] lines too! We don't need those either if we're never removing the layer. I maybe should've just annotated the changes :-)

    [self.layer addSublayer:self.leftHandle];
    [self.layer addSublayer:self.rightHandle];
@redcapua
Copy link
Contributor Author

redcapua commented Dec 5, 2018

Removed:

    [self.layer addSublayer:self.leftHandle];
    [self.layer addSublayer:self.rightHandle];

@TomThorpe
Copy link
Owner

LGTM 👍

@TomThorpe TomThorpe merged commit e196786 into TomThorpe:master Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants