New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close filemanger after open last document #3933

Merged
merged 1 commit into from May 6, 2018

Conversation

Projects
None yet
3 participants
@robert00s
Contributor

robert00s commented May 6, 2018

Close: #3310
Similar to that:

FileManager.instance:onClose()
ReaderUI:showReader(file)
after open last document (from menu) we should also close filemanager instance.

This change fix problem: #3310 (Menu with plugin isn't synchronized)

@Frenzie Frenzie merged commit fedb34c into koreader:master May 6, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@poire-z

This comment has been minimized.

Contributor

poire-z commented May 6, 2018

No similar problem if, when being in FileManager, you launch History, and open a document from there?

I remember I did look at something like that around the time I fixed "UIManager to not paint widgets covered by a full screen widget", for situations where we don't close the FileManager while opening reader.
I had a patch (that I never PR, don't remember why I didn't) with something like your fix, plus this:

--- a/frontend/apps/reader/readerui.lua
+++ b/frontend/apps/reader/readerui.lua
@@ -439,6 +439,10 @@ end

 local _running_instance = nil
 function ReaderUI:doShowReader(file, provider)
+    local FileManager = require("apps/filemanager/filemanager")
+    if FileManager.instance then
+        FileManager.instance:onClose()
+    end
     logger.info("opening file", file)
     -- keep only one instance running
     if _running_instance then

(again, there could be reasons - or none - why I didn't push that - I just don't remember them...)

@robert00s

This comment has been minimized.

Contributor

robert00s commented May 6, 2018

@poire-z
Your fix looks much better. We can always close FM before opening document.
I forgot that we can also open book from history, search result, open with, and dropbox, ftp, opds after download.

@poire-z

This comment has been minimized.

Contributor

poire-z commented May 6, 2018

There may be still a case to check for: when opening a book fails (like, rename a mp3 to .epub or .pdf): do we get back to somewhere :) or do koreader exits (because no widget left to show)?

@Frenzie

This comment has been minimized.

Member

Frenzie commented May 6, 2018

I don't think that should be a problem. Quitting for that reason is what happened before I added this to the document opening process about a year ago:

if not document then
UIManager:show(InfoMessage:new{
text = _("No reader engine for this file or invalid file.")
})
self:showFileManager()
return
end

Edit: in #2956

@robert00s robert00s deleted the robert00s:sync_menu branch Aug 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment