Provide a config which disables the readers UIMenuController
#154
Conversation
| @@ -425,7 +425,11 @@ public class FolioReaderPage: UICollectionViewCell, UIWebViewDelegate, UIGesture | |||
| // MARK: UIMenu visibility | |||
|
|
|||
| override public func canPerformAction(action: Selector, withSender sender: AnyObject?) -> Bool { | |||
| if UIMenuController.sharedMenuController().menuItems?.count == 0 { | |||
| guard readerConfig.useReaderMenuController else { | |||
| return false | |||
There was a problem hiding this comment.
I don't think return false is the best option in this case, we will have problem while overriding probably. Also if the custom menu is disable it need to enable the default webView menu.
In my point of view the ideal is to call super here, so the default menu will opens, and when overriding you can chose if whether to show custom or not.
guard readerConfig.useReaderMenuController else {
return super.canPerformAction(action, withSender: sender)
}There was a problem hiding this comment.
Ok, this makes sense. As return super.canPerformAction(action, withSender: sender) is called anyway at the end of the method I have remove the guard lines.
There was a problem hiding this comment.
Don't remove guard here, just return the super, so it will use the default menu.
guard readerConfig.useReaderMenuController else {
return super.canPerformAction(action, withSender: sender)
}| @@ -478,7 +482,10 @@ extension UIWebView { | |||
| } | |||
|
|
|||
| public override func canPerformAction(action: Selector, withSender sender: AnyObject?) -> Bool { | |||
|
|
|||
| guard readerConfig.useReaderMenuController else { | |||
| return false | |||
There was a problem hiding this comment.
Ok. But then there has to be a way to customize the actions which the FolioReaderWebView (aka wich of the default UIWebView actions) should perform. Therefore I could make the FolioReaderWebView class public. I could then make an extension for it in the app I'm working on or we provide a delegate method. What do you think?
There was a problem hiding this comment.
I have made the FolioReaderWebView class public to be able to override its canPerformAction() method.
| @@ -685,7 +696,11 @@ extension UIWebView { | |||
| } | |||
|
|
|||
| func setMenuVisible(menuVisible: Bool, animated: Bool = true, andRect rect: CGRect = CGRectZero) { | |||
| if !menuVisible && isShare || !menuVisible && isColors { | |||
| guard readerConfig.useReaderMenuController else { | |||
There was a problem hiding this comment.
If on canPerformAction call super this check is not necessary.
| @@ -656,7 +663,11 @@ extension UIWebView { | |||
| // MARK: - Create and show menu | |||
|
|
|||
| func createMenu(options options: Bool) { | |||
| isShare = options | |||
| guard readerConfig.useReaderMenuController else { | |||
| return | |||
There was a problem hiding this comment.
If on canPerformAction call super this check is not necessary.
There was a problem hiding this comment.
I don't think so: UIMenuController is a singleton which is owned by the app - not the framework. So this menu items will also effect the rest of the app which inlcudes Folio Reader. createMenu() sets menuItems to a new array and therefore removes all other menu items (also from the app). This should only happen if useReaderMenuController is set to true.
There was a problem hiding this comment.
The createMenu will be only called if you remove the guard from canPerformAction as you did. What I said is to not remove, but instead of return false return the super call, and leave the rest of the function, so it will call super and not create the custom menu, only the default one.
There was a problem hiding this comment.
The thing is that createMenu is called from 6 diferent places but 2 of them only happen after an interaction with Folio menu items. That's why I prefered to put the check in the method itself instead of all 4 places. So should I add the check to these places as well or keep it in the method?
| @@ -425,10 +425,6 @@ public class FolioReaderPage: UICollectionViewCell, UIWebViewDelegate, UIGesture | |||
| // MARK: UIMenu visibility | |||
|
|
|||
| override public func canPerformAction(action: Selector, withSender sender: AnyObject?) -> Bool { | |||
| guard readerConfig.useReaderMenuController else { | |||
There was a problem hiding this comment.
This can I think you should not remove the guard, because it will call the create menu, the idea is just to call super instead inside the guard.
For customization you can override this without problems.
|
Hey @tschob I had to fixed issues on UIMenuItem and seems that I have conficted your PR, I want to merge that, can you merge from master first? |
|
@hebertialmeida Ok, I've merged the master into this branch. You can merge now as well :) |
This PR adds a config which makes it possible to disable the default reader menu controller. By deactivating this it's possible for us to use custom menu entries.