Weird behavior on moving through pages
Thanks for your work, I really like PagingMenuController!
- Expected behavior and actual behavior.
Expected behavior : When moving through pages the position of MenuItemViews updates the position of Menu Item's views accordingly so that 1) there is no 'jumps' on the position of views just to force them use the whole Menu View's width ( when there exists less Menu Items than are needed to fill the whole Menu View's width ) and 2) that the current Menu Item View is always visible ( when there exists more Menu Items than are possible to show in the current Menu View's width )
Current behavior : When moving through pages the position of MenuItemViews updates in a weird way:
Scenario 1 : If the Menu Item's total width is less than the current Menu View's width then the calculation of the offset that each Menu Item should have is based on a ratio of the view's width. Making the Menu Items move across the whole width. Example
Scenario 2 : If the Menu Item's total width is greater than the current Menu View's width then the calculation follows the same rule as case 1. But as the number increases the ratio factor does not work and Menu Items stop being visible. Example
- Steps to reproduce the problem.
- For reproduce both behaviors consider the following properties on
MenuViewCustomizable
var displayMode: MenuDisplayMode {
return .standard(widthMode: .flexible, centerItem: false, scrollingMode: .pagingEnabled)
}
var focusMode: MenuFocusMode {
return .none
}
- For Scenario 1 init the PageMenuController with 3 controllers on iPad. For Scenario 2 init the PageMenuController with 8 controllers on iPhone.
- Specifications like the version of the project, library, or Swift.
Using the demo provided project, with "PagingMenuController" (2.0.0), Swift 3.0 and Xcode 8.0
I have done a reimplementation of contentOffsetXForCurrentPage: for dealing with these scenarios. Link. In the case that the current behavior is as intended, then I could move this implementation to a new type of MenuDisplayMode. I Would love to receive any input and perform the changes. Thanks ! 😄
A good improvement. It will help a lot if this is moved to master.
@CoderPug Create a pull request with your change in order for @kitasuke to review it and maybe accept it.
Thanks @mihaigabriel1992 I will start working on that and will update once it's done
@CoderPug Thanks for ur fix. Really awesome!
Not merged to master?
I just did some cleanup and created a pull request for this fix. Sorry for taking some time.
Honestly I don't see any point using standard mode when menu item's total width is less than device's width even it's rotated. Could you tell me what the case is?
Sure. Basically I'm dealing with an scenario where the number of items are dynamic so when there exists enough information and multiple menu items the total width is grater than device's width and everything works fine but when there isn't enough number of items then I have that weird behavior described in the issue and visible in the example.
I am also feeling it is a bit strange behaviour for moving menu items' positions when total menu width is less than device width on standard mode(with centerItem is false) (Scenario 1 ).
For forcing to fix position, I am patching code by below simple change.
MenuView.swift
fileprivate var contentOffsetX: CGFloat {
switch menuOptions.displayMode {
case .standard(_, let centerItem, _) where centerItem:
return centerOfScreenWidth
case .segmentedControl:
return contentOffset.x
case .infinite:
return centerOfScreenWidth
default:
return contentOffsetXForCurrentPage
}
}
to
fileprivate var contentOffsetX: CGFloat {
switch menuOptions.displayMode {
case .standard(_, let centerItem, _) where centerItem:
return centerOfScreenWidth
// added below case
case .standard(_, let centerItem, _) where !centerItem:
if self.contentView.frame.width < self.frame.width {
return contentOffset.x
} else {
return contentOffsetXForCurrentPage
}
case .segmentedControl:
return contentOffset.x
case .infinite:
return centerOfScreenWidth
default:
return contentOffsetXForCurrentPage
}
}
I don't know it is best, but simple.