Skip to content
This repository has been archived by the owner on Jun 17, 2018. It is now read-only.

Fix weird behavior on moving through pages #291 #322

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

Conversation

CoderPug
Copy link

Fixes # .

Changes proposed in this pull request:

@@ -8,6 +8,8 @@

import UIKit

internal let MenuViewItemsMarginRigth: CGFloat = 20
Copy link
Owner

Choose a reason for hiding this comment

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

Can you explain why we have static right margin?

Copy link
Author

Choose a reason for hiding this comment

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

Hi Kitasuke. First of all thanks for your framework.

So I declared it that way so it could be easy to change later if the right margin was not the desired. Because of some constants like MinimumSupportedViewCount, in that moment I thought it will make sense. Honestly looking at it right now probably will be better to just have a constant inside the same function contentOffsetXForCurrentPage.
Let me know what you think.

Thanks !

Copy link
Owner

Choose a reason for hiding this comment

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

I understand that it makes easier, but I actually prefer not to have static value. Anyway I'll wait for this answer to understand what the case is at first.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for you answer. Then this static margin wouldn't be appropriate way, because this change is supposed to be for dynamic change of the number of items. In addition, I'm not sure this approach would be suitable for other cases as well.

Ideally there should be method which can change it's option, but it's still in progress...
Well, let me think different approach.

Copy link
Author

Choose a reason for hiding this comment

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

Awesome, let me know if I can help you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants