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

Fixed issue #164, #186 and #173#219

Merged
hebertialmeida merged 15 commits into
FolioReader:masterfrom
PravinNagargoje:master
Apr 5, 2017
Merged

Fixed issue #164, #186 and #173#219
hebertialmeida merged 15 commits into
FolioReader:masterfrom
PravinNagargoje:master

Conversation

@PravinNagargoje
Copy link
Copy Markdown
Contributor

@PravinNagargoje PravinNagargoje commented Mar 11, 2017

This pull request is related to issue -
https://github.com/FolioReader/FolioReaderKit/issues/164
Changes commited on 15 march are related to issue -
https://github.com/FolioReader/FolioReaderKit/issues/186

Changes commited on 17 march solve issue -
https://github.com/FolioReader/FolioReaderKit/issues/173
I have added throw-catch and few custom FolioReader errors for first issue and solved the highlight problem.

@PravinNagargoje PravinNagargoje changed the title Made readEpub failable Fixed issue #164 Mar 11, 2017
@PravinNagargoje PravinNagargoje changed the title Fixed issue #164 Fixed issue #164 and #186 Mar 15, 2017
@PravinNagargoje PravinNagargoje changed the title Fixed issue #164 and #186 Fixed issue #164, #186 and #173 Mar 17, 2017
}

extension FolioReaderContainer {
func alert(message: String) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What do you think creating an extension of UIAlertController and move this method inside it?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@PravinNagargoje I use your fork but still crash when highlight.

Copy link
Copy Markdown
Contributor Author

@PravinNagargoje PravinNagargoje Mar 21, 2017

Choose a reason for hiding this comment

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

@Julyyq this handle errors from readEpub(), readContainer() and readOpf(). In your case error might be from somewhere else. If you are talking about paragraph highlight, it's still not solved.

style: UIAlertActionStyle.cancel
)
{
(result : UIAlertAction) -> Void in
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What do you think of using [weak self] here?

Copy link
Copy Markdown
Contributor Author

@PravinNagargoje PravinNagargoje Mar 21, 2017

Choose a reason for hiding this comment

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

I don't think it will be necessary. But still I have added it.

@@ -150,6 +150,7 @@ extension Highlight {
highlight?.type = type.hashValue

try realm.commitWrite()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe it is a good idea to use guard let to create a Realm

Maybe changing realm.beginWrite() to realm.write { } will be better. Will prevent screen locking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right. It might lead to screen locking.
I have made changes.

Comment thread Source/EPUBCore/FREpubParser.swift Outdated
let coverImageId = book.metadata.findMetaByName("cover")
if let coverResource = book.resources.findById(coverImageId) {
book.coverImage = coverResource
} else if let coverResource = book.resources.findByProperties("cover-image") {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What do you think changing findByProperties to findByProperty?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to findByProperty.

Comment thread Source/FolioReaderWebView.swift Outdated
@@ -108,8 +108,6 @@ open class FolioReaderWebView: UIWebView {
let startOffset = dic["startOffset"]!
let endOffset = dic["endOffset"]!
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here is a good place to use guard let

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

made all the changes as you requested and made PR.

@hebertialmeida hebertialmeida self-assigned this Apr 5, 2017
@hebertialmeida
Copy link
Copy Markdown
Member

@PravinNagargoje Thank you, many improvements here!

@hebertialmeida hebertialmeida merged commit 6bfb055 into FolioReader:master Apr 5, 2017
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.

3 participants