Reduce sleep screen message's font size to fit one line by Commodore64user · Pull Request #13026 · koreader/koreader

Commodore64user · 2025-01-08T16:25:30Z

what's new

Introduced the force_one_line property to the InfoMessage class to attempt to show text in a single line.
Modified the InfoMessage:init() function to calculate the maximum height of the widget based on the force_one_line property, ensuring that the text fits within the specified constraints.
Added the force_one_line property to the Screensaver:show() function to ensure the sleep screen message is displayed in a single line.

screenshots

before/after

This change is

poire-z · 2025-01-08T16:55:29Z

+    if self.shrink_to_fit then
+        self:shrinkFontToFit(frame, text_widget, 0.07)
+    elseif not self.height then
+        self:shrinkFontToFit(frame, text_widget, 0.95)


I don't mind moving the code into its own method.
But the 0.07 here feels a bit random in relation to a property name "shrink_to_fit"...
It looks like this makes the infomessage not take more than 0.07 x screen_height - and you hope it will stick on one line (which may not be the case if we provide a smaller font?). Anyway, none of these 2 aims sounds like "shrink to fit".

I guess a more proper way would be to have max_lines = N, and may be you guessing the height taken by N lines of the current font & size - and you would provide max_lines = 1.

Yes the 7pc is a bit of a guesstimate, it comes from measuring the frame at one line with default font (53px on a 800px screen, that is roughly 6.625%, so 7 to give it some wiggle room). I guess it could be instead of boolean, a number (percentage) at which you cap the size of the frame... regardless of font size or other.

if not self.height then if self.max_lines then max_height = self.max_lines * text_widget:getLineHeight() else max_height = screen height * 0.95 end -- and provide directly that max_height to shrinkFontToFit

and then, you wouldn't need to separate the code into its own function (as you don't really know what would happen if we call it from elsewhere than from :init() - that a separate function invites us to think we can).

I took it a slightly different way, I hope you like it...

I am all for making it simpler if possible, however, I don't like the max_lines implementation since it still needs me to figure out a way to guess then the size for values >2. I'd rather keep it a simple on/off boolean if it's all the same to you.

it still needs me to figure out a way to guess then the size for values >2

Isn't it just a matter of:

- max_height = math.max(text_widget:getLineHeight(), icon_height) + 2*frame.bordersize + 2*frame.padding + 3 + max_height = math.max(self.max_lines * text_widget:getLineHeight(), icon_height) + 2*frame.bordersize + 2*frame.padding + 3

but then we would have to account for all the spacing between the different lines, as I said, I'd rather do it boolean if it's all the same to you.

What "spacing between the different lines" ? getLineHeight() should return something that is used for all y and h computations, so there's no room/space in between.

if it's all the same to you

Guess it's not all the same to me :) If it's not more complicated to handle a number than a boolean, it will feel more generic and less adhoc.
Even if I don't really see why one would use it with wanting more than 1 line.

what if I want n = 100? there are a lot more cases to handle there, and as you, yourself said, why would anyone want to use it with anything other than one? it should be all the same to you :)

poire-z · 2025-01-09T14:46:01Z

+        local max_height
+        if self.cap_height_to_one_line then
+            local icon_height = self.show_icon and image_widget:getSize().h or 0
+            -- calculate the size of the frame container when it's only displaying one line, and add a magic number (3) to give it some wiggle room


Does KOReader wiggle ?!!! It shouldn't ! If it does, fix that :)

And don't use magic numbers, KOReader should be deterministic, if you need magic, your reality is poor. Fix it !
I believe you shouldn't need any addition - if you need a +1, it's maybe because there's a < somewhere that could be a <= ?

is not really need it, is just me giving it an extra 3 pixels just in case. I can remove it though.

what I mean by wiggle room is that if we have a large enough font and the text is aaaa, that would be a smaller value than that of bbbb for example. So add those extra pixels in case something like was the case.

Yes, remove it. The "just in case" means you're not sure you don't need it. I'd rather you to be sure :)

that if we have a large enough font and the text is aaaa, that would be a smaller value than that of bbbb for example

This is not the case, the height we get is constant, from the font metrics, a fixed thing - it's not the max of "blackbox/inkbox" of each letter that compose the line.

poire-z · 2025-01-09T15:18:54Z

    _timeout_func = nil,
    width = nil,  -- The width of the InfoMessage. Keep it nil to use default value.
    height = nil,  -- The height of the InfoMessage. If this field is set, a scrollbar may be shown.
+    cap_height_to_one_line = false,  -- Attempt to show text in one single line. This setting and height are not to be used conjointly.


Feels this is still not a good name.
It explains the how we achieve one line (explanation which is not really needed) and moreover it's wrong (we don't cap the height - which could invoke crop & scroll -, we reduce the font size).
So, single_line or force_one_line.

poire-z · 2025-01-09T21:22:46Z

+        local max_height
+        if self.force_one_line then
+            local icon_height = self.show_icon and image_widget:getSize().h or 0
+            -- calculate the size of the frame container when it's only displaying one line.


(If a sentence ends with a dot, it should start with an uppercase.)

Frenzie · 2025-01-11T18:42:48Z

                text = screensaver_message,
                readonly = true,
                dismissable = false,
+                force_one_line = true,


What happens when there's a \n involved?

Nothing, besides the fact that you get a literal \n as part of the message. What i mean is, \n is not recognised as new line neither pre PR nor post PR.

Edit: i don’t think that the founding fathers intended to have a sleep screen message with multiple lines ;)

Let's make the question more generic :) What happens if we call InfoMessage with force_one_line=true and a normal multiline message ? Does it go at displaying it with the minimal font size ?
Just curious - as it would be a coder error.

That's what I meant.

you cheeky ferrets, may I ask why would anyone so obtusely try and do just that? I mean is like wondering why your feet are wet after taking off your shoes during a storm.

but I understand the sentiment, I imagine it would in fact loop all the way to end of the font size (10 or whatever) and then show it like that. I'll see if I can test it though.

quite comical really

Co-authored-by: David <97603719+Commodore64user@users.noreply.github.com>

poire-z · 2025-01-12T12:32:38Z

I think what I'm getting at is amply illustrated by the fact that it happens right here in this very context. Before vs. after \n detection:

Why is there a \n here in this case ? And why wasn't it the case for @Commodore64user use case ? Just because of emulator having F2, or is it a different InfoMessage ?
If the caller knows there is a \n, it should just not provide force_one_line.

Frenzie · 2025-01-12T12:47:36Z

Just because of emulator having F2, or is it a different InfoMessage ?

Yes, no one ever envisioned it might be broken on purpose. My point is the same thing applies to every InfoMessage ever.

poire-z · 2025-01-12T12:50:43Z

I'd say its a 100% @Commodore64user oversight.
It does not apply to every InfoMessage, just to those that get force_one_line=true - and it's in screensaver that we should have set it to false when we know we've added a \nPress F2...

Frenzie · 2025-01-12T13:00:02Z

Well, in any case it's now automatically ignored when it comes across \n. Seems good enough to me. :-)

Commodore64user · 2025-01-12T13:11:57Z

@poire-z acting like the kid in the classroom that upon agreeing for everyone to take the blame for some mischief, immediately points and accuses the kid whose grandiose free kick broke the classroom window. How was I to know that the emulator had that? ;)

I can make the change if you want anyway force_one_line = not Device:isEmulator(),

poire-z · 2025-01-12T13:15:45Z

How was I to know that the emulator had that? ;)

By looking around ? By thinking about the world and not only your little self / use-case ? :)

Anyway, let it be - I'm just mentionning all that so my point of view is remembered by the future humanity.

Commodore64user · 2025-01-12T13:27:47Z

By thinking about the world and not only your little self / use-case ? :)

I only want to Make Kindle Great Again. ;)

Commodore64user added 3 commits January 8, 2025 16:10

add shrink_to_fit flag to infomessage

34acd3a

whitespaces

84f506c

avoid double needless check

7fe5832

poire-z reviewed Jan 8, 2025

View reviewed changes

Commodore64user added 3 commits January 9, 2025 00:45

use a dummy widget to obtain height

82b27ef

calculate max_height differently, no dummy frame

d773164

better comment

21021a5

poire-z reviewed Jan 9, 2025

View reviewed changes

change name of property to force_one_line

82c7d3b

poire-z reviewed Jan 9, 2025

View reviewed changes

Commodore64user commented Jan 9, 2025

View reviewed changes

Comment thread frontend/ui/widget/infomessage.lua

Commodore64user commented Jan 9, 2025

View reviewed changes

Comment thread frontend/ui/widget/infomessage.lua Outdated

Update frontend/ui/widget/infomessage.lua

89d1885

Commodore64user requested a review from poire-z January 9, 2025 22:40

poire-z approved these changes Jan 9, 2025

View reviewed changes

Commodore64user requested a review from Frenzie January 11, 2025 16:37

Frenzie reviewed Jan 11, 2025

View reviewed changes

Commodore64user commented Jan 11, 2025

View reviewed changes

Comment thread frontend/ui/widget/infomessage.lua Outdated

Update frontend/ui/widget/infomessage.lua

f8569e9

Co-authored-by: David <97603719+Commodore64user@users.noreply.github.com>

Frenzie added this to the 2025.01 milestone Jan 12, 2025

Frenzie merged commit f8dec18 into koreader:master Jan 12, 2025

Commodore64user deleted the one-line-msg branch January 12, 2025 11:41

Commodore64user mentioned this pull request Apr 12, 2025

Screensaver: improve sleep screen message #13571

Merged

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

Reduce sleep screen message font size to fit one line (koreader#13026)

27dcc58

Uh oh!

Uh oh!

Conversation

Commodore64user commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what's new

screenshots

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Commodore64user Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Commodore64user Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Commodore64user Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

poire-z commented Jan 12, 2025

Uh oh!

Frenzie commented Jan 12, 2025

Uh oh!

poire-z commented Jan 12, 2025

Uh oh!

Frenzie commented Jan 12, 2025

Uh oh!

Commodore64user commented Jan 12, 2025

Uh oh!

poire-z commented Jan 12, 2025

Uh oh!

Commodore64user commented Jan 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Commodore64user commented Jan 8, 2025 •

edited

Loading

Commodore64user Jan 9, 2025 •

edited

Loading

Commodore64user Jan 11, 2025 •

edited

Loading

Commodore64user Jan 11, 2025 •

edited

Loading