AltStatusBarFix: battery percentage not shown #11976
Conversation
|
luacheck has some minor complaints, looks fine to me otherwise. |
poire-z
left a comment
There was a problem hiding this comment.
Thank you for fixing my mess :)
| if self.battery == 1 and self.battery_percent == 1 then -- append battery percentage | ||
| local batt_pre = "[" | ||
| local batt_post = "]" | ||
| local batt_val = "" |
There was a problem hiding this comment.
Better have batt_val = nil
For devices without battery, it would show as 
| batt_lvl = batt_lvl + aux_batt_lvl | ||
| else | ||
| batt_pre = "+" | ||
| batt_post = "+" |
There was a problem hiding this comment.
The doubled + feel a bit excessive:
The low weight of the + glyph makes it feel like a hole on the right and a bit misaglined. If it ends on the right with the %, it looks better, like proper justficiation :) instead of a hanging +.
poire-z
left a comment
There was a problem hiding this comment.
(No experience about the AuxBattery stuff, but) looks ok.
|
Is the OP still true with these changes? (For the squash commit message.) |
|
The list 2 items yes. |
|
@poire-z Before merging an additional thought. During my last work with the alt status bar (1, 2 or even 3 years ago) I noticed, we could use unicode there too. ↯ would be nice to indicate charging. Does this concerns still holds true? |
|
If it's fed from frontend - and it's found in NotoSans (or the fallback fonts we provide to crengine from koreader frontend), I guess it's fine. It seems to work, adding In any case, if we want to do more stuff like this for the top status bar, I guess feeding strings from frontend (and adding stuff to crengine so it uses these strings) like my recent PR is the simple way to go. In the past, I imagined the way to tweak the top status bar would be to paint some widget of us over the crengine top status bar (even left empty by crengine, but we need to keep having it know about that room taken for all the existing y computations/adjustments when it is enabled). |
|
Ready from my side: |
| local batt_pre = "[" | ||
| local batt_post = "]" |
There was a problem hiding this comment.
Is it ok these [ ] are gone when the battery is charging?
I think they somehow provide the shape of a bettery icon/level [ 45% ] (while also making it different from the book reading progress just before it), it's strange they go away when the battery is charging.
|
Ready? |
(A difference to the original behavior -> if the battery is charging, then show
[↯45%]' instead of[ + ]`)Regression (#11873)
This change is