Reorganize bottom menu config panels #6131

poire-z · 2020-05-07T14:52:21Z

Mostly for the PDF bottom menu. See #6122. Closes #6122.

Reorganize by topic, trying to limit the nb of widgets per panel to 4. Screenshots and reasoning in Reorganize PDF bottom menu config panels #6122 (comment)
Re-order some toggles from low to high ('off' then 'on').
Show font size as number instead of a list of "Aa". Screenshots in Reorganize PDF bottom menu config panels #6122 (comment).
PDF: add more font size values, and increase usable contrast values.
Adds help_text to most PDF toggle titles (as I understand the feature, please correct me and my english when I'm wrong :)

This change is

Mostly for the PDF bottom menu. - Reorganize by topic, trying to limit the nb of widgets per panel to 4. - Re-order some toggles from low to high ('off' then 'on'). - Show font size as number instead of a list of "Aa" - PDF: add more font size values, and increase usable contrast values. - Adds help_text to most PDF toggle titles.

Frenzie · 2020-05-07T15:51:50Z

frontend/ui/data/koptoptions.lua

@@ -129,6 +149,8 @@ local KoptOptions = {
                end,
                labels = {S.AUTO, S.LEFT, S.CENTER, S.RIGHT, S.JUSTIFY},
                name_text_hold_callback = optionsutil.showValues,
+                help_text = _([[In reflow mode, sets the lines alignment.


I think you meant "lines' alignment" but I'm not sure if that's any clearer than just alignment?

text alignment then ? (avoid wondering if we align words (on a line) or the line itself)

Frenzie · 2020-05-07T15:53:32Z

frontend/ui/data/koptoptions.lua

                default_value = 0,
                advanced = true,
                name_text_hold_callback = optionsutil.showValues,
+                help_text = _([[Force the use of OCR for text selection, even if the document has some text layer.]]),


You might be talking about something technical with multiple text layers or whatnot? But I think that just unnecessarily complicates things.

Suggested change

help_text = _([[Force the use of OCR for text selection, even if the document has some text layer.]]),

help_text = _([[Force the use of OCR for text selection, even if the document has a text layer.]]),

Frenzie · 2020-05-07T15:55:30Z

frontend/ui/data/strings.lua

@@ -7,16 +7,16 @@ S.DUAL_PAGES = _("Dual Pages")
 S.PAGE_CROP = _("Page Crop")
 S.FULL_SCREEN = _("Full Screen")
 S.ZOOM_DPI = _("Zoom (dpi)")
-S.PAGE_MARGIN = _("Margin")
+S.PAGE_MARGIN = _("Margins")


This change looks a bit suspicious, what's it about?

Asked you the question in #6122 (comment):

Now includes Margin (should it be plural?)

It is plural in cre:

I'd say that it shouldn't be.

IIRC, margin is the four all at once (e.g., page margin), or a single one. Two/three of them (e.g., left & right margins) are plural.

Because it's just "the margin." As @NiLuJe wrote.

"The margin of a written page is the empty space at the side of the page."

(There's empty space on all four sides.)

Allright, english has the right to be strange.
So, even when talking about its size, and not only as a general concept? The page margin is small ?

NiLuJe · 2020-05-07T15:56:52Z

frontend/ui/data/koptoptions.lua

                name_text_hold_callback = optionsutil.showValues,
-            }
+                help_text = _([[Allows cutting out blank page margins in the original document.


I'd keep using crop here, it should be common enough (i.e., cropping or cropping out instead of cutting) ;).

Initially went with "crop out", but Google tells me it's only used in "(of rock) appear or be exposed at the surface of the earth." :)
Thought cutting out might be an alternative explanation for people not used to "crop" (is that a technical word - or casual english word?)

That's an outcrop, yeah ;).

Cropping should be instantly recognizable by anyone who's ever done even the tiniest bit of image manipulation or photography.

Cropping allows you to crop something out.

And crop can be synonymous to cut in a number of contexts ;).

No reason to limit it to the max of our progress bar. Might be useful with some Arabic fonts.

NiLuJe · 2020-05-07T17:29:38Z

frontend/ui/data/creoptions.lua

@@ -254,7 +254,7 @@ Note that your selected font size is not affected by this setting.]]),
                more_options = true,
                more_options_param = {
                  value_min = 70,


I wouldn't mind lowering this to 50, too ;p.

(I have a vague memory that Noto has insanely large line-gaps by default, and 70 was still pretty roomy. I may be misremembering :D).

Okay, scratch that, I checked again w/ both Noto, and 70 is when ascenders and descenders start to meet, so, eh, fine by me at 70 ;).

Sorry for the noise.

(Can't hurt anyway, thanks!)

Oh, github comments lags.
Well, still keeping the 50, might be useful with some crappy large line height fonts.

poire-z · 2020-05-07T17:46:47Z

Can I merge this this evening (and give some work to translators) - or should I wait a bit more?

Frenzie

lgtm

Frenzie · 2020-05-07T18:18:46Z

@poire-z Let's get translation started ASAP, release shouldn't be too far off besides me not having written any release notes. :-)

poire-z · 2020-05-07T18:24:55Z

OK, and easier to spot typo or wording issues on the device than on GitHub.

hius07 · 2020-05-08T12:32:09Z

Can I reduce the font size of the info tips?
I would prefer the same size as the bottom menu items are.

poire-z · 2020-05-08T13:18:26Z

Not without tweaking values in the code.
(It uses the same default font & size that most InfoMessages get - and InfoMessage will aditionally reduce it if the text would overflow the screen.)
But why? It's not something you would spent hours contemplating :)

poire-z · 2020-05-08T19:19:01Z

Some unrelated question: should we normalize a bit more our top menus, and generalize the use of text_func for menu items with a list of selectable stuff as their submenu - or setable via some widget, to show the thing selected or set?

We have a few of them, with the thing shown in parenthesis or after a colon (should these be normalized too?)

Typography rules: French >
Font size (20)
Installed dictionaries (3/5) >
Back to exit (always) >
Previous: That so-so book.epub

Should we show:

Font: Noto Sans Devanagari UI >
Language: French >
Style: epub.css >
Highlight action: Translate >
Wikipedia languages: en fr ru
Screen DPI: Medium (160) >

Or that would make the menu more crowded and less readable - and it's desirable to not abuse that?

Frenzie · 2020-05-08T19:33:11Z

As a rule of thumb I'd say shorter is indeed better.

end? re koreader#6131

@poire-z

* Wherever possible, do an actual dumb invert on the Screen BB instead of repainting the widget, *then* inverting it (which is what the "invert" flag does). * Instead of playing with nextTick/tickAfterNext delays, explicitly fence stuff with forceRePaint * And, in the few cases where said Mk. 7 quirk kicks in, make the fences more marked by using a well-placed WAIT_FOR_UPDATE_COMPLETE * Fix an issue in Button where show/hide & enable/disable where actually all toggles, which meant that duplicate calls or timing issues would do the wrong thing. (This broke dimming some icons, and mistakenly dropped the background from FM chevrons, for example). * Speaking of, fix Button's hide/show to actually restore the background properly (there was a stupid typo in the variable name) * Still in Button, fix the insanity of the double repaint on rounded buttons. Turns out it made sense, after all (and was related to said missing background, and bad interaction with invert & text with no background). * KeyValuePage suffered from a similar issue with broken highlights (all black) because of missing backgrounds. * In ConfigDialog, only instanciate IconButtons once (because every tab switch causes a full instantiation; and the initial display implies a full instanciation and an initial tab switch). Otherwise, both instances linger, and catch taps, and as such, double highlights. * ConfigDialog: Restore the "don't repaint ReaderUI" when switching between similarly sized tabs (re #6131). I never could reproduce that on eInk, and I can't now on the emulator, so I'm assuming @poire-z fixed it during the swap to SVG icons. * KeyValuePage: Only instanciate Buttons once (again, this is a widget that goes through a full init every page). Again, caused highlight/dimming issues because buttons were stacked. * Menu: Ditto. * TouchMenu: Now home of the gnarliest unhilight heuristics, because of the sheer amount of different things that can happen (and/or thanks to stuff not flagged covers_fullscreen properly ;p). * Bump base koreader/koreader-base#1280 koreader/koreader-base#1282 koreader/koreader-base#1283 koreader/koreader-base#1284 * Bump android-luajit-launcher koreader/android-luajit-launcher#284 koreader/android-luajit-launcher#285 koreader/android-luajit-launcher#286 koreader/android-luajit-launcher#287

poire-z mentioned this pull request May 7, 2020

Reorganize PDF bottom menu config panels #6122

Closed

some rewording

f827200

Frenzie added this to the 2020.05 milestone May 7, 2020

Frenzie added the UX label May 7, 2020

Frenzie reviewed May 7, 2020

View reviewed changes

NiLuJe reviewed May 7, 2020

View reviewed changes

poire-z added 3 commits May 7, 2020 19:03

cleanup and review rewording suggestions

ab5e537

Avoid zoom info messages on book load

1cd46e9

cre line spacing: increase fine tuning max from 130 to 300

26e5468

No reason to limit it to the max of our progress bar. Might be useful with some Arabic fonts.

NiLuJe reviewed May 7, 2020

View reviewed changes

cre line spacing: decrease fine tuning min from 70 to 50

1db75ba

shorten title to avoid truncation

4764ca6

Frenzie approved these changes May 7, 2020

View reviewed changes

poire-z merged commit 8991540 into koreader:master May 7, 2020

poire-z deleted the bottom_panel_reorg branch May 7, 2020 18:24

Frenzie mentioned this pull request Jun 21, 2020

some warning when opening a PDF (it's unnecessary) #5385

Closed

NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Jan 7, 2021

I *think* we can restore that bit of trickery, sizes appear stable on my

399451f

end? re koreader#6131

NiLuJe mentioned this pull request Jan 7, 2021

Revamp "flash_ui" handling #7118

Merged

poire-z mentioned this pull request Jun 17, 2021

UI: Move "Writing Direction" and "Document Columns" from Cog to Aa? (mupdf engine) #7863

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize bottom menu config panels #6131

Reorganize bottom menu config panels #6131

poire-z commented May 7, 2020 •

edited by Frenzie

Loading

Frenzie May 7, 2020

poire-z May 7, 2020

Frenzie May 7, 2020

Frenzie May 7, 2020

poire-z May 7, 2020

Frenzie May 7, 2020

poire-z May 7, 2020

NiLuJe May 7, 2020

Frenzie May 7, 2020

Frenzie May 7, 2020 •

edited

Loading

poire-z May 7, 2020

NiLuJe May 7, 2020

NiLuJe May 7, 2020 •

edited

Loading

poire-z May 7, 2020

NiLuJe May 7, 2020

NiLuJe May 7, 2020

NiLuJe May 7, 2020

NiLuJe May 7, 2020 •

edited

Loading

poire-z May 7, 2020

NiLuJe May 7, 2020

poire-z May 7, 2020

poire-z commented May 7, 2020

Frenzie left a comment

Frenzie commented May 7, 2020

poire-z commented May 7, 2020

hius07 commented May 8, 2020

poire-z commented May 8, 2020

poire-z commented May 8, 2020

Frenzie commented May 8, 2020

	help_text = _([[Force the use of OCR for text selection, even if the document has some text layer.]]),
	help_text = _([[Force the use of OCR for text selection, even if the document has a text layer.]]),

Reorganize bottom menu config panels #6131

Reorganize bottom menu config panels #6131

Conversation

poire-z commented May 7, 2020 • edited by Frenzie Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frenzie May 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiLuJe May 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiLuJe May 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poire-z commented May 7, 2020

Frenzie left a comment

Choose a reason for hiding this comment

Frenzie commented May 7, 2020

poire-z commented May 7, 2020

hius07 commented May 8, 2020

poire-z commented May 8, 2020

poire-z commented May 8, 2020

Frenzie commented May 8, 2020

poire-z commented May 7, 2020 •

edited by Frenzie

Loading

Frenzie May 7, 2020 •

edited

Loading

NiLuJe May 7, 2020 •

edited

Loading

NiLuJe May 7, 2020 •

edited

Loading