Skip to content

PermissionButton improvements - #94

Open
Jeehut wants to merge 5 commits into
delba:masterfrom
Jeehut:master
Open

PermissionButton improvements#94
Jeehut wants to merge 5 commits into
delba:masterfrom
Jeehut:master

Conversation

@Jeehut

@Jeehut Jeehut commented Feb 8, 2017

Copy link
Copy Markdown

When using the PermissionButton in an app I recognised two issues. I opened an issue for each:

This PR solves both #92 and #93 and probably also #116.

NOTE: Since @delba is not answering, I've integrated #90 to this PR. If you want to use a iOS 10, Xcode 9 and Swift 4 compatible version of this framework, feel free to integrate my fork instead:
https://github.com/Dschee/Permission

@delba delba left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR, thanks! Just a little change and we're good to go :)

Tells the view that its superview changed.
*/
open override func didMoveToSuperview() {
render(.normal)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use render() here please? I prefer to keep update() solely as a public interface.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just updated this.

@Jeehut

Jeehut commented Mar 28, 2017

Copy link
Copy Markdown
Author

Anything new here? I did the requested change two weeks ago, still not merged though.
Would be great to link against the original project in my Cartfile! :)
(Just had to rebase due to Xcode 8.3 changes ...)

@Jeehut

Jeehut commented Jul 15, 2017

Copy link
Copy Markdown
Author

@delba What about merging this?

@Jeehut

Jeehut commented Sep 14, 2017

Copy link
Copy Markdown
Author

@delba Ping.

@jeffaburt

Copy link
Copy Markdown

@delba do you mind taking a second pass at this when you get a free second?

@Jeehut

Jeehut commented Oct 17, 2017

Copy link
Copy Markdown
Author

@jeffaburt @CraigSiemens See my comment here.

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.

3 participants