search

koreader / crengine

This is the KOReader CREngine fork. It cross-pollinates with the official CoolReader repository at https://github.com/buggins/coolreader, in case you were looking for that one.
70 stars 45 forks source link

lvdocview: fix and expose functionality for displaying author and book title separately in the status bar #575

Closed trash-pandy closed 1 month ago

trash-pandy commented 2 months ago

I enjoy using KOReader, and I especially like using the alt status bar, but the author and title switching every page was distracting on my Kindle PW2's e-ink display. The functionality seemed to be mostly in place already, but wasn't entirely exposed. I have made an implementation of the feature in KOReader and plan to make a pull request to that repo as well, if this pull request gets merged. I have tested the feature in the emulator and it appears to be working properly, but a second opinion would be much appreciated.

koreader alt status menu

This change is Reviewable

poire-z commented 2 months ago

Looks alright (minus the minor indentation issues we see at the bottom of the Files change pane on GitHub). Looks like setStatusMode is only used internally so there should be no problem with its signature change.

trash-pandy commented 2 months ago

There seems to be a lot of whitespace issues in the project. I felt it was mildly out of scope for this PR. I can make some minor, localized edits to more closely match the code around them, if that's the only blocker to this being merged.

poire-z commented 2 months ago

There seems to be a lot of whitespace issues in the project. I felt it was mildly out of scope for this PR. I can make some minor, localized edits to more closely match the code around them

Yes, there are mixed and ugly whitespace mismatchs. But please don't fix the whitespace on lines you're not touching. Just be sure the lines you touch or add do match the code around - so you don't add to the mess.

trash-pandy commented 1 month ago

Anything else I can do?

poire-z commented 1 month ago

Looks ok, will merge in a few days - we're still waiting for a stable release of koreader to bump crengine, as it includes some possibly risky commits.

poire-z commented 1 month ago

No change in koreader-base I assume? So, you'll be able to PR the frontend part after this gets bumped into koreader (tomorrow or the day after). Will it work ok before you add the frontend part? Or do we need to be sync'ed?

trash-pandy commented 1 month ago

If it isn't synced, then the author will be always displayed in the top left with the alt status bar.