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

Added possibility to turn off adjusting button heights by indicator height #19

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

Conversation

wrutkowski
Copy link

Used for high indicators as in implemented example - thirdScrollPager. Also changed order of subviews so that indicator is behind buttons.

…eight - used for high indicators as in implemented example
@aryaxt
Copy link
Owner

aryaxt commented Feb 15, 2016

Thanks for the PR, this could be a good addition, but It feels weird that indicatorHeightShouldAdjustButtonHeight is still using the indicator height

So the flag is set to op, but it doesn't behave as the name would suggest (adjust button height)

screen shot 2016-02-15 at 9 33 52 am

addSubview(indicatorView)
}
}
insertSubview(indicatorView, belowSubview: buttons.first!)
Copy link
Owner

Choose a reason for hiding this comment

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

let's just insert into index 0 that way we don't force unwrap

@aryaxt
Copy link
Owner

aryaxt commented Feb 15, 2016

Also please update the code to use tabs instead of spaces to be constant with the rest of the code

@wrutkowski
Copy link
Author

Hi, it's still using indicator heights because you can have for example 4pt indicator height, but want the labels to be in the middle of the view instead of shifted a bit top. And setting indicator height to equal with control height we have tabs effect.

I'll fix insertSubview and spaces 👍

@aryaxt
Copy link
Owner

aryaxt commented Feb 16, 2016

aaah, makes sense

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