PagingMenuController icon indicating copy to clipboard operation
PagingMenuController copied to clipboard

Weird behavior on moving through pages

Open CoderPug opened this issue 9 years ago • 10 comments

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.

  1. 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
        }
  1. 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

CoderPug avatar Oct 16 '16 01:10 CoderPug

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 ! 😄

CoderPug avatar Oct 16 '16 04:10 CoderPug

A good improvement. It will help a lot if this is moved to master.

mihaigabriel1992 avatar Oct 24 '16 15:10 mihaigabriel1992

@CoderPug Create a pull request with your change in order for @kitasuke to review it and maybe accept it.

mihaigabriel1992 avatar Oct 26 '16 06:10 mihaigabriel1992

Thanks @mihaigabriel1992 I will start working on that and will update once it's done

CoderPug avatar Oct 26 '16 14:10 CoderPug

@CoderPug Thanks for ur fix. Really awesome!

iad24 avatar Dec 02 '16 06:12 iad24

Not merged to master?

spsammy avatar Jan 01 '17 18:01 spsammy

I just did some cleanup and created a pull request for this fix. Sorry for taking some time.

CoderPug avatar Jan 14 '17 04:01 CoderPug

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?

kitasuke avatar Jan 22 '17 23:01 kitasuke

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.

CoderPug avatar Jan 23 '17 01:01 CoderPug

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.

t avatar Feb 08 '17 21:02 t