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

Add Custom Inset Properties, Option to Disable Image Resizing, Provide Segment Borders #23

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dbburgess
Copy link

Borrowing from the approach @randybyrd322 made in #19, this adds properties to provide custom insets for finger-grained control over positioning. Unfortunately, our two pull requests won't play nicely together. I'd be happy to modify mine as necessary to make it work though.

Also, I haven't had a chance to look at how the vertical hybrid style works, so it doesn't modify the insets for that at all. I think semantically, the way contentInsets are done now also aren't quite correct (in plain hybrid, at least...I think it should probably do some combination of the two provided insets somehow).

Also, I added an option to disable the image resizing, as it was making getting the icons to look good challenging for me.

Feedback and suggestions are welcome, I'm happy to modify / improve the PR as necessary. Thanks for the useful / great project! 👍

I'm not sure how to handle the insets for the vertical style, as I
haven't used that style or really dug into how it's positioning things.
I'm not sure how to handle the insets for the vertical style, as I
haven't used that style or really dug into how it's positioning things.
For some use cases, this may make sizing and positioning images more
challenging and confusing.
@xaviermerino
Copy link
Owner

Looks great. Let me get a working charger so I can use my Mac and then I'll take a look into it. Thanks for your effort.

Sent from my iPhone

On Jun 29, 2016, at 3:14 PM, Daniel Burgess notifications@github.com wrote:

Borrowing from the approach @randybyrd322 made in #19, this adds properties to provide custom insets for finger-grained control over positioning. Unfortunately, our two pull requests won't play nicely together. I'd be happy to modify mine as necessary to make it work though.

Also, I haven't had a chance to look at how the vertical hybrid style works, so it doesn't modify the insets for that at all. I think semantically, the way contentInsets are done now also aren't quite correct (in plain hybrid, at least...I think it should probably do some combination of the two provided insets somehow).

Also, I added an option to disable the image resizing, as it was making getting the icons to look good challenging for me.

Feedback and suggestions are welcome, I'm happy to modify / improve the PR as necessary. Thanks for the useful / great project! 👍

You can view, comment on, or merge this pull request online at:

#23

Commit Summary

Add property for custom title insets
Add property for custom icon insets
Add property to disable image resizing
File Changes

M Pod/Classes/XMSegmentedControl.swift (39)
Patch Links:

https://github.com/xaviermerino/XMSegmentedControl/pull/23.patch
https://github.com/xaviermerino/XMSegmentedControl/pull/23.diff

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@dbburgess
Copy link
Author

Thanks! No problem, I understand...Take your time. I can happily keep using my fork for now. I actually just added another option to provide borders to the segments. I can split stuff up into separate PRs if you'd prefer, and as I said, feedback is welcome if you'd prefer to craft the public interface in a different direction.

@dbburgess dbburgess changed the title Add Custom Inset Properties, Option to Disable Image Resizing Add Custom Inset Properties, Option to Disable Image Resizing, Provide Segment Borders Jun 30, 2016
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

2 participants