Skip to content
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

Add percentage read (and other book information) to sleep message #5121

Merged
merged 3 commits into from Jul 18, 2019

Conversation

@sladflob
Copy link
Contributor

commented Jul 18, 2019

This is one feature I miss from the stock Kobo firmware, the ability to see the percentage read on the screen while the device is asleep. I know I can see the book status but it's not quite the same.

I thought a good way to do it would be to create escape sequences that the user can put in their screensaver text. I've modified screensaver.lua to use %p for percentage read. While I was at it I added %c and %t for current and total pages respectively, and %dt for document title.

Attached is a picture of the screen of my Kobo Aura One sleeping...

IMG_20190718_093423

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

And a screenshot of the dialog to configure this sleep message:

Reader_2019-Jul-18_100051

local totalpages = doc:getPageCount()
ret = string.gsub(ret, "%%t", totalpages)

local percent = math.floor(((currentpage * 100) / totalpages) + 0.5)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Jul 18, 2019

Member

We also have our own Math.round() function which, admittedly, does exactly this. But the advantage is it's easier to search the source to see where it's used. :-)

function Math.round(num)
return math.floor(num + 0.5)
end

This comment has been minimized.

Copy link
@sladflob

sladflob Jul 18, 2019

Author Contributor

OK cool, I'll use that then.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Jul 18, 2019

Member

Oops, I forgot about that (am on my phone). Minor follow-up PR if you want? Doesn't matter too much.

This comment has been minimized.

Copy link
@sladflob

sladflob Jul 18, 2019

Author Contributor

Might as well finish it off properly.

frontend/ui/screensaver.lua Show resolved Hide resolved
@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Personally I think using a string is easier for a user to remember than %1, %2 etc. The template function is fine for using in the code because you've got the string and the replacements right next to each other in the function call, but seems a bit obscure to expose to a user? I take the point that %p etc are scarcely better than numbers as they'd both need to be documented somewhere. And %dt is probably too obscure for document title but I couldn't reuse %t. Maybe make them more explicit like %percent, %page, %total, %title?

@Frenzie

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Yes, %percent would be self-explanatory, at least while reading. However, they need to be documented regardless.

For that you should add a description to the InputDialog.

self.input_dialog = InputDialog:new{
title = "Screensaver message",
input = screensaver_message,
buttons = {
{
{
text = _("Cancel"),
callback = function()
UIManager:close(self.input_dialog)
end,
},
{
text = _("Set message"),
is_enter_default = true,
callback = function()
G_reader_settings:saveSetting("screensaver_message", self.input_dialog:getInputText())
UIManager:close(self.input_dialog)
end,
},
},
},
}

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

(I'm fine with this PR as-is - just continuing the discussion for discussion sake :)

And %dt is probably too obscure for document title but I couldn't reuse %t. Maybe make them more explicit like %percent, %page, %total, %title?

Well, if it's one single char, it's easy. And you could have 26 lowercase letters + 26 uppercase letters + 10 numbers to make out something easier than %dt? %T may be? Somehow, uppercase = document metadata, lowercase = others.

Multiple letters like %dt %percent may be harder/ambiguous, and may need some kind of boundaries like %{percent} which is less pretty and fun and more risky :)

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Good idea re "T" for title. Also, I changed %p to not include the percent sign so it's consistent with the others (I only included it because I couldn't see it on the keyboard - d'oh!). I've added a description field to the dialog - how do I see this when testing?

@Frenzie

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

You just do? ;-) (like so)

Screenshot_2019-07-18_11-57-50

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

OK, I must have done something wrong then...

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Oh yeah, I put it in the wrong spot. Here it is... I have written a short essay.

Reader_2019-Jul-18_181107

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

(got rid of the spurious trailing ".")

Update screensaver.lua
Added description, changed document title to %T, got rid of '%' sign in percent text.
frontend/ui/screensaver.lua Outdated Show resolved Hide resolved
Update frontend/ui/screensaver.lua
Co-Authored-By: Frans de Jonge <fransdejonge@gmail.com>

@Frenzie Frenzie merged commit 912e617 into koreader:master Jul 18, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@Frenzie

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Thanks!

@Frenzie Frenzie added the enhancement label Jul 18, 2019

@Frenzie Frenzie added this to the 2019.08 milestone Jul 18, 2019

@sladflob sladflob referenced this pull request Jul 18, 2019
@AlanSP1

This comment has been minimized.

Copy link

commented Jul 18, 2019

@protist

This comment has been minimized.

Copy link

commented Aug 11, 2019

This is pretty nice! Is there any way to have the message overlaid on a different "background", e.g. the cover?

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2019

Thanks! I was thinking this would be a good thing too. No idea how hard it is... maybe I'll investigate when I get some time.

@protist

This comment has been minimized.

Copy link

commented Aug 12, 2019

@sladflob Great! Shall I open an issue so it doesn't get lost?

@KenMaltby

This comment has been minimized.

Copy link

commented Aug 13, 2019

You might include an option for the user to position the box on the page.

@sladflob

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

Sure @protist, and maybe add @KenMaltby's idea about positioning and I'll see how I go.

@protist

This comment has been minimized.

Copy link

commented Aug 13, 2019

Thanks @sladflob!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.