Stable page numbers 2#14473
Conversation
| and G_reader_settings:isTrue("pagemap_notify_document_provided") | ||
| and not (self.use_page_labels or self.show_page_labels) then |
There was a problem hiding this comment.
Now that it's optionnal and default to off, may be remove that not (use or show) condition ? (So one can see the source when he is using them).
There was a problem hiding this comment.
It would require the cancel_callback, to disable use (and show?).
There was a problem hiding this comment.
It can be just some informative InfoMessage, no need to have any question when invoked from Book information. Wrong comment :)
There was a problem hiding this comment.
Or, in this case show InfoMessage instead of ConfirmBox.
There was a problem hiding this comment.
I'm fine with an InfoMessage (no risk to hit the wrong button :))
(The menu says "notify me", not "ask me if I want to use them" :))
| end | ||
| end | ||
| end | ||
| table.insert(kv_pairs, { self.prop_text["pages"], pages or n_a, separator = true }) |
There was a problem hiding this comment.
No callback to see the source ? (Just asking, not requesting.)
Given that the last_page, as on your screenshot with a scrambled book, can look odd or be confusing without more context, if tapping on it could explain it, it would be self documenting.
There was a problem hiding this comment.
The same infomessage?
koreader/frontend/apps/reader/modules/readerpagemap.lua
Lines 527 to 530 in d9700e0
There was a problem hiding this comment.
May be with a bit more blahblah:
This book has publisher page numbers (first_label) > last_label) from the source (print edition):
page_map_source
| end, | ||
| }, | ||
| { | ||
| text = _("Notify if book has publisher page numbers"), |
There was a problem hiding this comment.
@Frenzie @Commodore64user : does it read ok "Notify..." without a who, as in "Notify me" ? (I think we don't address the user as "me" in the other UI items).
There was a problem hiding this comment.
I hadn't noticed there was a new message. I think we usually write "Show notification …"
There was a problem hiding this comment.
Except that here, it's not a regular Notification, which would be too small for this text.
There was a problem hiding this comment.
That sounds like a technical implementation detail? I don't think the user cares if the larger widget isn't technically a Notification with a capital N. :-P
There was a problem hiding this comment.
| text = _("Notify if book has publisher page numbers"), | |
| text = _("Show notification if book has publisher page numbers"), |
There was a problem hiding this comment.
Can we get away with just Show notification when available?
There was a problem hiding this comment.
Assuming this is going to be the dialog box as before, I don't think we should use "notification" as this term is reserved. How about "Show prompt when ℗ available", or just "Prompt when ℗ available"
(Also, little nit-pick. The ℗ in the Override... menu item should be in parentheses: (℗).)
|
Unified info message (on new books opening, on Show source info, in Book info). 
|
|
Unrelated to this PR, but when selecting chars per page, long-press on the arrows move only by 4. Going from 1500 to 500 could take a long time :) (before I thought I could tap on the number to input it directly). |
You can. But otherwise agreed, 4 seems overly cautious. |
|
That whole page numbers by the publisher sounds very clunky, perhaps something along the lines of "This ebook retains the publisher’s original page numbering". A bit more natural |
|
I'd make that provides since something like retains or keeps sounds like you're forced to live with it. I wouldn't mention any publisher at all, because who else would provide them? Santa Claus? Simply: |
Synthetic Charperpage Jr. |
|
I’m pretty sure i made the same comment before “provided by” which seems to have been ignored… very funny @Frenzie but if we drop the facetiousness, is not that someone might assume that some higher up force is magically giving them page numbers, but about clarity of their origin. |
|
While that argument about clarity strikes me as far-fetched — they were put in the EPUB by whoever prepared the EPUB, simple as that — turning it around might sound better: |
|
What's the final wording? |
|
I think keeping it short and simple (i.e., |
|
Is not that i disagree, I simply think that adding the publisher bit sets the record straight. i.e., These are numbers that match a hardback and we have nothing to do with them. |
I prefer the latter: "included/in this book" makes it clear it's there in the epub, and that we don't get to go fetch them elsewhere where they are provided :)
|
|
I think it goes against good editing practice ("this book includes author-written words!") but whatever I suppose, it's not ungrammatical. |
only authors don't paginate the content, When writing War and Peace (og name "War, what is it good for?", it was his mistress who convinced him to change the name), Tolstoy was not worried whether a certain scene would be on an even or odd page, the numbers are simply a byproduct of the words. |
|
Thanks for adding the Book info details. I have a few suggestions (screenshots are as-is with current PR): 1. Synthetic page numbers used, PPN available: 
As a random user, I would interpret this as "131 publisher page numbers". I'd suggest showing it like this:
Left-hand info shows that synthetic pages are currently in use (with current char settings), and the PPN availability is delineated by a vertical bar to minimize confusion. 2. Publisher page numbers used: 
The last publisher page label isn't necessarily the same as the number of pages. This book actually starts at publisher page number "[6]", so the actual publisher page count is only 92. I'd suggest to show the integer count of the publisher page numbers instead, with the (P) to indicate PPN:
3. Screen page numbers used, PPN available: 
Suggestion: Number by itself indicates screen pages, number right of bar with (P)-Symbol indicates publisher page number count. |
|
Book info Pages - 8 pagemap combinations (use_pagemap, has_synthetic, has_publisher). |
poire-z
left a comment
There was a problem hiding this comment.
Hard to grasp the 8 combinations, so trusting you.
|
3 above cases. Use screen pages, got both synthetic and publisher pages. |
|
Looks good :) |
|
Looks good to me, thanks. (Personally I would leave the 'chars per page' info out if stable pages are not enabled (4th screenshot). Or show it with the total synthetic page count. But without the total page count it doesn't carry much significance.) |
This. |
|
Once synthetic page numbers are generated, they cannot be removed from the book via the UI. |
|
Late to the update, but a few more comments: the about page has the following bit "or if you prefer to have consistent page lengths for all books", that is clunky as well, better "or if you prefer a consistent page length for all books" I would also change "one screen equals one page" to "one page displays exactly one screen of content" Another “a page will be counted for this amount of characters” sounds weird, try "each page will contain this number of characters" The second to last paragraph seems a bit difficult to parse if you are unfamiliar with how all that works under the hood, i would remove the "since..." and leave just the "you can choose..." |
In all this, when we say "page", we mean "page numbers". "one page displays" just doesn't work for me, it is the screen that displays content.
"Since stable page numbers can start anywhere on the screen" is also for me one of the key sentence that could answer questions that the above text could bring (ie. for me, when just seeing "page" and not "page numbers", the meaning gets less clear). |
|
I'm thinking that it could be helpful to always show screen pages as well in the Book info, even if something else is used? It will be the only place in the UI where the screen page count can still be checked (besides the stable page list). (Also, in the current release, it still shows screen pages in the Book info even if Reference pages are used, so some users might be missing it.) Accordingly, the first two of the above cases would become:
Thoughts? |
|
Any corrections to the wordings? |
| table.insert(t, pages) | ||
| end | ||
| if self.ui.pagemap.chars_per_synthetic_page then | ||
| local cpp = "(" .. T(N_("1 char per page", "%1 chars per page", |
There was a problem hiding this comment.
| local cpp = "(" .. T(N_("1 char per page", "%1 chars per page", | |
| local cpp = "(" .. T(N_("1 character per page", "%1 characters per page", |
There was a problem hiding this comment.
Even as "chars per page" it is using a rather large amount of space. Example:
(And mind that books can have 5 digit page numbers.)
On the contrary, I would suggest we use the acronym "CPP" and also add it in the About text.
"CPP" is commonly used by typesetting caculators, e.g. this one
There was a problem hiding this comment.
In that case it should have a translator's note to point out that it should be short (or what CPP means, if going that route, but it seems a bit jargon).
There was a problem hiding this comment.
koreader/frontend/apps/reader/modules/readerpagemap.lua
Lines 407 to 408 in b79f777
There was a problem hiding this comment.
Already translated is good. 👍 Best to duplicate the note in case someone hits that one first. :-)
|
|
||
| function ReaderPageMap:showDocumentProvidedInfo(get_text) | ||
| local t = _([[ | ||
| This book has stable page numbers by the publisher (℗). |
There was a problem hiding this comment.
| This book has stable page numbers by the publisher (℗). | |
| This book includes stable page numbers from the publisher (℗). |
There was a problem hiding this comment.
This suggestion continues to be as awkward as the text is replaces. I’ll say it one last time “provided by” solves the issue, or use the other suggestion i can’t remember now
There was a problem hiding this comment.
(If you're talking about the wrap between publisher and (p), that will depend also on your screen DPI (people may get it also on 9", and I may not get it on 6"). If you want to avoid it, you should include a U+00A0 (non-break space) in between.)
There was a problem hiding this comment.
Yes, the wrap. 6" is still the most frequent eink screen size, isn't it?
There was a problem hiding this comment.
But the text should simply be This book includes stable page numbers, period. Anything else violates proper editing practice.
There was a problem hiding this comment.
It can be in the menu
[ ] Prompt when publisher page numbers available
and in the message
Publisher page numbers available
There was a problem hiding this comment.
Yes, the wrap. 6" is still the most frequent eink screen size, isn't it?
Yes. But what I mean is that what you see on your 6" may not be similar to what I see on my 6": there are other things at play when eg. scaleBySize the font size: the nb of pixels, the screen DPI selected in the menu (I may not have "auto", and a 6" device with 300 dpi or 272 dpi may get different things).
If you see some bad wrap, don't solve this by adding a word or increasing the width of the infomessage :) you should use a NBSP to avoid it in any conditions.
There was a problem hiding this comment.
You guys need to step out and focus on what's more important :D please don't fight over small nitpicks
|
See the three open comment threads. |
|
|
||
| function ReaderPageMap:showDocumentProvidedInfo(get_text) | ||
| local t = _([[ | ||
| This book includes stable page numbers from the publisher (℗). |
There was a problem hiding this comment.
Sorry for the noise. I think this is the last thing.
| This book includes stable page numbers from the publisher (℗). | |
| This book includes stable page numbers provided by the publisher (℗). |
|
Having just witness Book info tell me |
This change is