Skip to content

FileManager: Plus dialog button from CoverBrowser#12857

Merged
hius07 merged 3 commits into
koreader:masterfrom
hius07:coverbrowser-plus-button
Dec 7, 2024
Merged

FileManager: Plus dialog button from CoverBrowser#12857
hius07 merged 3 commits into
koreader:masterfrom
hius07:coverbrowser-plus-button

Conversation

@hius07

@hius07 hius07 commented Dec 6, 2024

Copy link
Copy Markdown
Member

The Plus dialog doesn't need an api, just fetch the button from the CoverBrowser.


This change is Reviewable

@NiLuJe

NiLuJe commented Dec 6, 2024

Copy link
Copy Markdown
Member

Nice cleanup!

Friday night, so, take this with a grain of salt, but nothing jumps out ;).

})
end

local extract_button = self.coverbrowser and self.coverbrowser:genExtractBookInfoButton(close_dialog_callback)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(Not fond of seeing named references to a plugin in core - but you have already done that a lot, so ok :))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I do hope one day you will agree it deserves to be moved to the core, it's the basis of the library.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Besides of the view, the database part is important. As we have measured, the sql request is faster than doc_settings opening.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I support someone other than me doing it. 😇

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do hope one day you will agree it deserves to be moved to the core

I have nothing against that (see #8472 (comment)) - as long as the database is still considered just a cache and trashable (and disable'able if possible).

I support someone other than me doing it.

It's huge work, so: me too :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think they are giving you the go ahead... @hius07

@hius07 hius07 merged commit 157c03c into koreader:master Dec 7, 2024
@hius07 hius07 deleted the coverbrowser-plus-button branch December 7, 2024 07:18
@hius07 hius07 added this to the 2025.01 milestone Dec 7, 2024
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants