Skip to content

Status bar presets: action in Dispatcher - #13718

Merged
hius07 merged 3 commits into
koreader:masterfrom
hius07:footer-presets-action
May 5, 2025
Merged

hius07 merged 3 commits into
koreader:masterfrom
hius07:footer-presets-action

Conversation

@hius07

@hius07 hius07 commented May 3, 2025 •

Copy link
Copy Markdown
Member

Status bar settings:

1

Gesture manager:

2

3

Also closes #13710 (dynamic filler to honor progress bar margins).
Also closes #13715 (fix showing porgress bar).


This change is Reviewable

@Commodore64user

Copy link
Copy Markdown
Member

Thank you for adding the actions. 👌

@shuvashish76

shuvashish76 commented May 3, 2025 •

Copy link
Copy Markdown

Can we've the option to next/previous status bar preset options as radio buttons?
I want to assign them to "Bottom edge right/left" for "One-finger swipe" so that I can quickly cycle-through them.
Originally requested in #13689 (comment)

@Commodore64user

Copy link
Copy Markdown
Member

Can we've the option to next/previous status bar preset options as radio buttons?

I think you might be confusing a few things there. Regardless, I still worry that cycling through all possible presets is NOT a good idea (re-renders are a real threat). But you could certainly make work the two-paged status bar like this:

  1. make two preset with similar layout (only items should be different)
  2. assign v1 to bottom edge right
  3. assign v2 to bottom edge left
  4. profit.

@shuvashish76

shuvashish76 commented May 4, 2025 •

Copy link
Copy Markdown

But you could certainly make work the two-paged status bar

Yes possible but for only two presets and two gestures. For more than two I've to assign slightly different category gestures which again I've to memorize them. I want to have one category of gesture + relevant location of the screen for one type of action (in this case status bar presets).

re-renders are a real threat

If we use only two then also it re-renders correct? Anyways if there are technical hurdles for cycling through multiple presets then it's fine for me :)

@Commodore64user

Commodore64user commented May 4, 2025 •

Copy link
Copy Markdown
Member

Just about any change will trigger a re-render, but if you don't mess with the geometry of the status bar it shouldn't be a problem to switch between two presets HOWEVER... the status bar was NOT designed to do what you are asking for.

And as said before, you might be sensible enough to realise the limitations but we can't make that assumption for everyone else. Hot swapping status bars is NOT a feature we can currently* support.

Presets [unofficially] assumes there will be a re-render so don't expect what can't be delivered (at this moment at least).

@hius07
hius07 merged commit 93bffb8 into koreader:master May 5, 2025
@hius07
hius07 deleted the footer-presets-action branch May 5, 2025 14:34
@hius07 hius07 added this to the 2025.05 milestone May 5, 2025
@sebdelsol

Copy link
Copy Markdown

I want to have one category of gesture + relevant location of the screen for one type of action (in this case status bar presets).

I made a User patch for cycling through all the presets when tapping the status bar.

By the way, there's a little bug when loading a preset, it does nothing when the status bar is hidden, which is a bit odd.
I've fixed it in my user patch by adding self:applyFooterMode() at the end of ReaderFooter:loadFromNamedPreset

@sebdelsol

Copy link
Copy Markdown

Now I understand the issue @Commodore64user was referring to: any change in font or progress bar type requires a full refresh. Is there a way to distinguish these cases from those that don’t require a full refresh?

@shuvashish76

shuvashish76 commented May 5, 2025 •

Copy link
Copy Markdown

any change in font or progress bar type requires a full refresh.

Not an issue for me, I don't have any intention of cycling-through regularly but when I do I'll manually full-refresh the page. (Short diagonal one-finger swipe)
If possible add "Status bar presets change" option to "Settings > Screen > E-Ink settings > Full refresh rate"?

@sebdelsol

sebdelsol commented May 5, 2025 •

Copy link
Copy Markdown

If possible add "Status bar presets change" option to "Settings > Screen > E-Ink settings > Full refresh rate"?

It's not working like that. When there are dramatic changes in the status bar, a full refresh of the reader page is needed. It's quite long (~10 sec) and probably very costly battery-wise. I guess that's what @Commodore64user was fearing.

So what I've done in this new user patch version, is to fully refresh only when needed.
Here's the list of setting changes that require it. I'm not completely sure this list is exhaustive:

local need_full_refresh_settings = {
    "container_height",
    "bottom_horizontal_separator",
    "progress_style_thin",
    "progress_style_thick_height",
    "progress_style_thin_height",
    "text_font_bold",
    "text_font_size",
    "progress_bar_position",
    "reclaim_height", 
}

It's probably too contrived to be a proper integrated solution. That's why I wrote a user patch instead.
So if you have presets that keep those settings the same, a full redraw won't be needed. And that's exactly what I needed 😅.

@Commodore64user: I've found another bug when loading a preset: when using the dynamic filler, the text might be cropped when the prefixes have changed (see #13710). You just need to set self.separator_width = nil in ReaderFooter.loadFromNamedPreset to fix it.

@sebdelsol

sebdelsol commented May 5, 2025 •

Copy link
Copy Markdown

@Commodore64user, do I need to open bugs for those:

  • I've found another bug when loading a preset: when using the dynamic filler, the text might be cropped when the prefixes have changed (see Status bar: dynamic filler crops the status bar text #13710). You just need to set self.separator_width = nil in ReaderFooter.loadFromNamedPreset to fix it.
  • By the way, there's a little bug when loading a preset, nothing happens when the status bar is hidden, which is a bit odd.
    I've fixed it in my user patch by adding self:applyFooterMode() at the end of ReaderFooter:loadFromNamedPreset

Or do you want me to try a PR (I'm not too familiar with all the CI stuff) ?

By the way, thanks for the great status bar presets.

@Commodore64user

Copy link
Copy Markdown
Member

Now I understand the issue Commodore64user was referring to: any change in font or progress bar type requires a full refresh. Is there a way to distinguish these cases from those that don’t require a full refresh?

as I said before, the status bar is not designed to hot swap. and as I also said, don't mess with geometry and you might get away with murder (i.e keep changes limited to "items").

is to fully refresh only when needed.

btw, you don't need any of that, the engine will automatically re-render when it needs to. i.e when you mess with the dimension of the bar.

so yes, keep it as a patch because again, we don't support hot swapping status bars. and stop asking for it will you ;)

do I need to open bugs for those:

I shouldn't think so, @hius07 is already aware besides, he only added the dynamic thing recently so I'm sure he's got a fix coming, as for the second I'm not sure I follow, if it's hidden is hidden, what's wrong with that?

@sebdelsol

sebdelsol commented May 5, 2025 •

Copy link
Copy Markdown

btw, you don't need any of that, the engine will automatically re-render when it needs to. i.e when you mess with the dimension of the bar.

Your code forces a full refresh making the screen to flash when it's not needed (same progress bar, same font, etc...). I fixed that, now I can cycle through presets without those annoying screen flashes. I understand why you did it, since loading a preset is a very rare occurrence.

self:refreshFooter(true, true)

so yes, keep it as a patch because again, we don't support hot swapping status bars. and stop asking for it will you ;)

I'm not asking anything, I'm fine with a user patch as I stated before.

I was just wondering if there's a way to determine when a reader page update is actually needed. I know my solution is quite naive.

I shouldn't think so, @hius07 is already aware besides, he only added the dynamic thing recently so I'm sure he's got a fix coming, as for the second I'm not sure I follow, if it's hidden is hidden, what's wrong with that?

If the status bar is hidden and you ask for a preset to load, then you expect something to happen. In this case, I think showing it would resolve the issue.

@Commodore64user

Commodore64user commented May 5, 2025 •

Copy link
Copy Markdown
Member

Your code forces a full refresh making the screen to flash when it's not needed

I see what you mean, in any case it IS needed because we do NOT support hot swapping. you do you in your patch.

I'm not asking anything, I'm fine with a user patch as I stated before.

I don't mean you personally, stating it in general to all who keep coming up with that idea ;)

I was just wondering if there's a way to determine when a real reader page update is actually needed.

whenever crengine feels like it needs it ;) (when you alter the bar's geometry, but maybe in other cases too? ¯\_(ツ)_/¯.)

If your preset is centered and without separators, if one of the preset has compact prefixes, then you'll end up with cropped text in the status bar.

mate, hius07 already has gone back and forth with you about it, he knows.

If the status bar is hidden and you ask for a preset to load, then you expect something to happen. In this case, I think showing it would resolve the issue.

if your computer is off and you start typing, do you also expect it to turn on and accept your key presses? if it's off, it's off as far as I am concerned.

@sebdelsol

sebdelsol commented May 5, 2025 •

Copy link
Copy Markdown

I understand everything except this part:

If your computer is off and you start typing, do you also expect it to turn on and accept your key presses? If it's off, it's off as far as I'm concerned.

If I ask for something to show up, then yes, I expect it to show up (in this case: a preset, whatever its status).
btw, if my computer is sleeping, I do expect a key press to wake it up. That’s a weird analogy (and my cat keeps waking up my computer, that's an issue).

But in this case, it’s about having immediate, perceptible feedback from a tap in a menu (no cat involved). That’s just good UX.
So you could either show the preset or an InfoMessage to explain why you won't do it.

EDIT: btw, showing the preset is a one-liner: add self:applyFooterMode() in ReaderFooter.loadFromNamedPreset

@hius07

hius07 commented May 6, 2025

Copy link
Copy Markdown
Member Author

@sebdelsol, your reports noted and will be fixed in due course, please be patient, do not duplicate.

@sebdelsol

Copy link
Copy Markdown

I've seen your PR, thanks for the fixes

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.

Status bar presets Status bar: dynamic filler crops the status bar text

4 participants