Skip to content
This repository has been archived by the owner on Nov 26, 2020. It is now read-only.

Provide a way to perform java script code in a readers page from external code #155

Merged

Conversation

tschob
Copy link
Member

@tschob tschob commented Sep 21, 2016

This PR:

  • refactors the UIWebView extension to the subclass FolioReaderWebView
  • Provides a way to let extneral code run java script in a FolioReaderPage

@tschob tschob changed the title Feature/exposed webview Provide a way to perform java script code in a readers page from external code Sep 21, 2016
Copy link
Member

@hebertialmeida hebertialmeida left a comment

Choose a reason for hiding this comment

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

Why do you think is better to subclass the UIWebView instead of a extension, as it is not public to override?

@@ -157,7 +157,7 @@ public class FolioReaderPage: UICollectionViewCell, UIWebViewDelegate, UIGesture
}

UIView.animateWithDuration(0.2, animations: {webView.alpha = 1}) { finished in
webView.isColors = false
(webView as? FolioReaderWebView)?.isColors = false
Copy link
Member

Choose a reason for hiding this comment

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

Lets reduce the (webView as? FolioReaderWebView) cast, we can just cast with guard on beginning of the function.

guard let webView = webView as? FolioReaderWebView else { return }

Then just use webView as previously.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this seems to be the better approach, I'll adjust it.

webView.createMenu(options: true)
webView.setMenuVisible(true, andRect: rect)
(webView as? FolioReaderWebView)?.createMenu(options: true)
(webView as? FolioReaderWebView)?.setMenuVisible(true, andRect: rect)
Copy link
Member

Choose a reason for hiding this comment

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

Same for casting here and so on...

@tschob
Copy link
Member Author

tschob commented Sep 22, 2016

I think it's better to use a subclass instead of an extension because:

  • The added logic is only needed in one place and it only contains logic which is needed for the reader and useless for other use cases. With this needs it should normally be a subclass and not an extension.
  • The extension effects all UIWebView instances in an app which uses the Folio Reader. If Folio Readers UIMenuController isn't overridden it's possible to select text in a app owned UIWebView instance and choose eg. "Highlight" (as the UIWebView extension says it can perform this action).
  • Folio Readers UIWebView is less customisable as extension as everything which is added will effect all UIWebView instances in an app. So only things which don't break other UIWebView instances can be added. The old implementation breaks other instances.

@hebertialmeida hebertialmeida merged commit 29d66ce into FolioReader:master Sep 23, 2016
@tschob tschob deleted the feature/exposedWebview branch March 31, 2017 08:17
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