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 view's borders when using it with corner radius allSides #530

Merged
merged 4 commits into from
Jan 22, 2018

Conversation

tbaranes
Copy link
Member

@tbaranes tbaranes commented Jan 17, 2018

The border doesn't respect the allSides radius when both are set. Using a custom layer for every radius fix this.

@tbaranes tbaranes force-pushed the hotfix/allsides_border_corner branch from d59cc7c to e2f57e9 Compare January 17, 2018 16:15
@tbaranes tbaranes force-pushed the hotfix/allsides_border_corner branch from e2f57e9 to 4923ea4 Compare January 17, 2018 16:15

// if a layer mask is specified, remove it
layer.mask?.removeFromSuperlayer()
layer.mask = cornerSidesLayer()
Copy link
Member

Choose a reason for hiding this comment

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

can we encapsulate the code since they are very similar?

Copy link
Member Author

@tbaranes tbaranes Jan 18, 2018

Choose a reason for hiding this comment

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

If that's ok with you, I will do the refactor in #529 since I'm already refactoring a bit the class there

Copy link
Member Author

Choose a reason for hiding this comment

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

Done there

Copy link
Member

@JakeLin JakeLin left a comment

Choose a reason for hiding this comment

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

👍 thanks for fixing it. Please have a look at my small comment.

@tbaranes
Copy link
Member Author

Travis was failing because of the tests, I fixed most of them, but there's still one failing that I don't understand why. I welcome any idea 🤔

@SD10
Copy link
Member

SD10 commented Jan 18, 2018

@tbaranes I'm sorry I missed this. If there's a problem with something I wrote, please tag me in the future

@tbaranes
Copy link
Member Author

You didn’t really miss it, my comment has been posted less than 2h ago! The tests are failing due to my change, your code is really clear and understandable but path are a bit tricky sometimes 😀

@SD10
Copy link
Member

SD10 commented Jan 19, 2018

It's still going after 6 hours @_@ I hate travis

@tbaranes
Copy link
Member Author

tbaranes commented Jan 19, 2018

@SD10 Yeah, travis is completely down these days, but it will fail since the test is failing locally.

@tbaranes tbaranes force-pushed the hotfix/allsides_border_corner branch from 9050206 to d932fc9 Compare January 19, 2018 08:10
@tbaranes
Copy link
Member Author

Finally understood why that last test didn't pass, travis should be green now!

@IBAnimatableBot
Copy link

IBAnimatableBot commented Jan 20, 2018

2 Warnings
⚠️ Consider adding supporting documentation to this change. Documentation can be found in the docs directory.
⚠️ Consider adding / updating the demo ap.

Generated by 🚫 Danger

@SD10
Copy link
Member

SD10 commented Jan 20, 2018

@tbaranes I'm not sure if you know, I just learned this a couple months ago to remove those annoying SwiftLint warnings:

Xcode -> Preferences -> Text Editing

screen shot 2018-01-20 at 1 40 12 pm

@tbaranes tbaranes merged commit 464b463 into master Jan 22, 2018
@tbaranes tbaranes deleted the hotfix/allsides_border_corner branch January 22, 2018 07:56
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.

None yet

4 participants