Skip to content

[ReaderFooter] add status bar presets - #13689

Merged
hius07 merged 19 commits into
koreader:masterfrom
Commodore64user:status-bar-presets
May 2, 2025
Merged

[ReaderFooter] add status bar presets#13689
hius07 merged 19 commits into
koreader:masterfrom
Commodore64user:status-bar-presets

Conversation

@Commodore64user

@Commodore64user Commodore64user commented Apr 26, 2025

Copy link
Copy Markdown
Member

what's new

This PR seeks to enhance the status bar admin, recently I found myself in a situation where my usual status bar was simply inadequate for the content and was therefore forced to make some adjustments, but hey, I now need to remember what settings I had before and.. I ain't got time for that. So PRESETS yay!

Users will be able to save multiple presets and switch to them simply by selecting them, that's it.
This allows for things like: having a status bar specifically for PDFs and another for ePUB, or another for when on Landscape mode, etc.

changes

  • Added a new menu item "Status bar presets" to the status bar menu, enabling users to manage named presets for status bar settings.
  • Implemented getNamedPresetMenuItems to dynamically generate menu items for creating, loading, updating, and deleting named presets.
  • Added helper methods:
  • Updated the progress_style_thin setting to default to false instead of nil when toggled, ensuring its entry is maintained in self.settings
  • Added new dependencies for widgets and utility modules, including InfoMessage, InputDialog, MultiConfirmBox, ffiUtil, and util.

screenshots

presets

related issues


This change is Reviewable

@mergen3107

Copy link
Copy Markdown
Contributor

Woah, that's cool!

Here is an idea: how about cycling through presets when you tap on status bar? Say, I have two presets: all book related (pages, titles, reading speed, etc), all system related (memory, battery, etc). So I can cycle through preset 1, tap - preset 2, another tap - status off.

@Commodore64user

Commodore64user commented Apr 26, 2025

Copy link
Copy Markdown
Member Author

Depending on how radical the different presets are, a simple switch can trigger a re-render. Now i know you personally would keep that in mind and only apply “item” changes (which wouldn’t be problematic) but you are not most people… I also think there is a possibility of adding this to profiles/gestures/hotkeys but baby steps first.

edit:

Woah, that's cool!

thanks!

end,
callback = function()
self.settings.progress_style_thin = nil
self.settings.progress_style_thin = false

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.

this little change here is giving me a bit of a headache. It needs to be false so that there is an actual entry into self.setting (nil removes it altogether) which in turn makes switching between modes (thick and thin) not possible. The problem resides in that anyone whose value at time of update is nil would have issues should they create a preset with our first making it false, so, should I make a migration from nil to false? or is there perhaps another approach that could be taken here?

end
-- Create a copy of current settings, excluding presets themselves.
local current_settings = {}
for k, v in pairs(self.settings) do

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.

There are some clone table util functions too fwiw.

@hius07

hius07 commented Apr 27, 2025

Copy link
Copy Markdown
Member

We have standard UX/architecture approach for such things.

Menu "Presets" with submenu items: "New with current settings" and the saved presets.
No "empty slots".
Tap on preset to apply it, long-press to have a remove/rename dialog.

Presets are not stored in the memory, they are not a part of footer.settings.

Preset are saved to settings.reader.lua as a separate entry footer_presets["preset_name"].

@Commodore64user

Copy link
Copy Markdown
Member Author

We have standard UX/architecture approach for such things.

Menu "Presets" with submenu items: "New with current settings" and the saved presets. No "empty slots". Tap on preset to apply it, long-press to have a remove/rename dialog.

do we have any other examples of this besides the profile plugin?

I actually find, for this particular case at least, the system of one sub_menu to save and another to load a bit easier to grasp, the slots came from reminiscing about the good old PS1 Memory Card days, and also the fact that it requires a lot less intrusion into reader footer. I wonder if @Frenzie or @poire-z have anything to say... others as well of course. We don't discriminate around here, we only gang up on hardback readers ;)

@hius07

hius07 commented Apr 27, 2025

Copy link
Copy Markdown
Member

do we have any other examples of this besides the profile plugin?

Text efitor, Book shortcuts, Patch management.
No "empty slots".

@Frenzie

Frenzie commented Apr 27, 2025

Copy link
Copy Markdown
Member

the good old PS1 Memory Card days

Few things are worse than being limited to a handful of save slots. ;-) I doubt it matters in this context but it does seem like an unnecessary restriction and deviation from the norm.

@Commodore64user

Copy link
Copy Markdown
Member Author

the good old PS1 Memory Card days

Few things are worse than being limited to a handful of save slots. ;-) I doubt it matters in this context but it does seem like an unnecessary restriction and deviation from the norm.

Fine, have it your way. But you are all wrong ;) there’s nothing more delightful than a good old memory slot.

Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
Comment thread frontend/apps/reader/modules/readerfooter.lua Outdated
@hius07

hius07 commented Apr 28, 2025

Copy link
Copy Markdown
Member

You missed reader_footer_mode, reader_footer_custom_text, reader_footer_custom_text_repetitions.

I think that after a preset has been loaded, any changes to the footer configuration will not be saved, because of the broken self.settings table reference:

self.settings = G_reader_settings:readSetting("footer", self.default_settings)

@Commodore64user

Commodore64user commented Apr 28, 2025

Copy link
Copy Markdown
Member Author

I think that after a preset has been loaded, any changes to the footer configuration will not be saved, because of the broken self.settings table reference:

I'm not sure I understand what your concern is, could you clarify it a bit?

@hius07

hius07 commented Apr 28, 2025

Copy link
Copy Markdown
Member

footer.settings are linked to G_reader_settings:

self.settings = G_reader_settings:readSetting("footer", self.default_settings)

self.settings = filtered_preset

breaks the link and the settings are not saved anymore.

@hius07

hius07 commented Apr 30, 2025

Copy link
Copy Markdown
Member
function ReaderFooter:loadFromNamedPreset(preset_name)
    local preset = G_reader_settings:readSetting("footer_presets")[preset_name]
    G_reader_settings:saveSetting("footer", util.tableDeepCopy(preset.footer))
    G_reader_settings:saveSetting("reader_footer_mode", preset.reader_footer_mode)
    G_reader_settings:saveSetting("reader_footer_custom_text", preset.reader_footer_custom_text)
    G_reader_settings:saveSetting("reader_footer_custom_text_repetitions", preset.reader_footer_custom_text_repetitions)
    self.settings = G_reader_settings:readSetting("footer")
    self.mode = preset.reader_footer_mode
    self.custom_text = preset.reader_footer_custom_text
    self.custom_text_repetitions = tonumber(preset.reader_footer_custom_text_repetitions)
    self:updateFooterTextGenerator()
    self:refreshFooter(true, true)
end

@hius07

hius07 commented Apr 30, 2025

Copy link
Copy Markdown
Member

I think the checkboxes are not useful.

local footer_presets = G_reader_settings:readSetting("footer_presets", {})
local items = {
{
text = _("Create new preset from current settings"),

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.

Just to double check, is this congruent with similar usage elsewhere? There's new profile from current document settings or some such for example.

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.

text = _("New with current book settings"),
enabled = self.document ~= nil,
keep_menu_open = true,
callback = function(touchmenu_instance)
local function editCallback(new_name)
self.data[new_name] = self:getProfileFromCurrentBookSettings(new_name)
self.updated = true
touchmenu_instance.item_table = self:getSubMenuItems()
touchmenu_instance.page = 1
touchmenu_instance:updateItems()
end
self:editProfileName(editCallback)
end,
separator = true,
},

@Commodore64user

Copy link
Copy Markdown
Member Author

what's up with that circleCI failure?

@Frenzie

Frenzie commented Apr 30, 2025

Copy link
Copy Markdown
Member

It means it's trying to fetch a commit that doesn't exist. Maybe a GH issue? (I.e., it's presumably distributed around the world, and maybe the node CircleCI tried to pull from didn't have it yet.)

@Commodore64user

Copy link
Copy Markdown
Member Author

I think the checkboxes are not useful.

we tend to disagree sometimes... as people start using it, I have no doubt some will add different options and will want to know what is actually active without having to "remember" anything. Furthermore, that is just a nice visual hint that perhaps you should "update" your preset.

function ReaderFooter:loadFromNamedPreset(preset_name)
local footer_presets = G_reader_settings:readSetting("footer_presets")
local preset = footer_presets[preset_name]
if preset and next(preset) then -- only load if preset exists and is not empty

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.

No need to check, the menu is built of the existing presets.

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.

gestures/hotkeys

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 shall do in the event handler.

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.

are you saying that you are doing the gestures bit? can I get a straight answer please

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.

Yes, I'll merge this PR after you remove checkboxes, then will add Dispatcher actions in a separate PR.

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.

Yes

thank you.

will add Dispatcher actions in a separate PR

I wasn't expecting it to be otherwise.

@hius07

hius07 commented May 1, 2025

Copy link
Copy Markdown
Member

The checked_func here is not cheap and gives doubtful benefits for its cost.

@Commodore64user

Copy link
Copy Markdown
Member Author

The checked_func here is not cheap and gives doubtful benefits for its cost.

is not really problematic for a few ones. I imagine it would be if you had hundreds of them.

I suppose is up to @Frenzie to break to tie then, to checked_func or not to check_func? that is the question.

@hius07

hius07 commented May 1, 2025

Copy link
Copy Markdown
Member

What do you mean "for a few ones"? 55 entries.

@Commodore64user

Commodore64user commented May 1, 2025

Copy link
Copy Markdown
Member Author

few presets, we are also leaving as soon as there is a discrepancy so not really comparing all of them [entries] for every preset.

@Frenzie

Frenzie commented May 1, 2025

Copy link
Copy Markdown
Member

I suppose is up to @Frenzie to break to tie then, to checked_func or not to check_func? that is the question.

I mean, without if it's not really necessary of course. ;-)

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.

FR: Enable saving the Status Bar settings per book FR: Status bar items for landscape mode only FR: Separate status bar settings for PDF

4 participants