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

Make page 1 the last page number for pages left in a chapter #7549

Closed
JSWolf opened this issue Apr 14, 2021 · 22 comments · Fixed by #8093
Closed

Make page 1 the last page number for pages left in a chapter #7549

JSWolf opened this issue Apr 14, 2021 · 22 comments · Fixed by #8093
Milestone

Comments

@JSWolf
Copy link

JSWolf commented Apr 14, 2021

Please change the page numbering so that the last page number for pages left in a chapter show as 1 instead of 0. 1 makes a lot more sense as it's the last page before the next chapter. Also, it means no more having to always add one to the number of pages left to get the true number.

@Jellby
Copy link
Contributor

Jellby commented Apr 14, 2021

See #6831 too.

It all depends on the interpretation. Think of it as "pages left after this one", or look at the number once you finish reading the page. Or, if a chapter has 7 pages and you are on the seventh page in the chapter, that's page 7/7... should the "pages left" be 1 or 0? Of course then the first page is 1/7, but one would "wish" that since it's the first page, it would say that there are 7 pages left... The problem is this needs an mind-reading interface ;)

@JSWolf
Copy link
Author

JSWolf commented Apr 14, 2021

The problem comes when you see the number of pages for the chapter. It's always off by one. So if it shows12, it's incorrect as it should be 13.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 14, 2021

What @Jellby said ;).

@JSWolf
Copy link
Author

JSWolf commented Apr 14, 2021

0 just doesn't work. It doesn't give you the correct number of pages. If the chapter is 12 pages, I get shows 11 pages and that's incorrect. The number of pages should be showing how many pages I have left to read. 1 means I have 1 page left to read. 0 means I have no pages left to read and not 1 page left to read.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 14, 2021

A page shown is considered as a page read(ing). So, yes, in a 12 page chapter, when you're on its first page, you indeed have 11 pages left to read (... after that one ;p).

@Jellby
Copy link
Contributor

Jellby commented Apr 14, 2021

11 means there are 11 more pages you haven't seen yet. 0 means there's nothing more, all you see is all you get, i.e. this is the last page.

@JSWolf
Copy link
Author

JSWolf commented Apr 15, 2021

When has a pBook ever has a page 0? The page number should always be how many pages left to read and when you turn to the last page, you should see 1.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 15, 2021

That's... precisely what we're doing: page 1 is, well, page 1, meaning what's left is total - 1, meaning 0 on the last page.

@Frenzie
Copy link
Member

Frenzie commented Apr 15, 2021

I don't think I've ever seen a paper book point out how many pages are left in the chapter. You can't be on page 0 of a book, but if you're on the last page of the chapter there are no remaining pages. If you're on the first page of a 22 page chapter then there are 21 pages remaining.

@Galunid
Copy link
Member

Galunid commented Apr 15, 2021

diff --git a/frontend/apps/reader/modules/readerfooter.lua b/frontend/apps/reader/modules/readerfooter.lua
index ce52a445..395ad0c5 100644
--- a/frontend/apps/reader/modules/readerfooter.lua
+++ b/frontend/apps/reader/modules/readerfooter.lua
@@ -258,7 +258,7 @@ local footerTextGeneratorMap = {
         local symbol_type = footer.settings.item_prefix or "icons"
         local prefix = symbol_prefix[symbol_type].pages_left
         local left = footer.ui.toc:getChapterPagesLeft(footer.pageno)
-        return prefix .. " " .. (left or footer.ui.document:getTotalPagesLeft(footer.pageno))
+        return prefix .. " " .. ((left or footer.ui.document:getTotalPagesLeft(footer.pageno)) + 1)
     end,
     percentage = function(footer)
         local symbol_type = footer.settings.item_prefix or "icons"

Don't have a pdf to test on hand, but I don't see a reason for it not to work.

@JSWolf
Copy link
Author

JSWolf commented Apr 15, 2021

@Galunid That's perfect. Thank you.

@JSWolf
Copy link
Author

JSWolf commented Apr 26, 2021

diff --git a/frontend/apps/reader/modules/readerfooter.lua b/frontend/apps/reader/modules/readerfooter.lua
index ce52a445..395ad0c5 100644
--- a/frontend/apps/reader/modules/readerfooter.lua
+++ b/frontend/apps/reader/modules/readerfooter.lua
@@ -258,7 +258,7 @@ local footerTextGeneratorMap = {
         local symbol_type = footer.settings.item_prefix or "icons"
         local prefix = symbol_prefix[symbol_type].pages_left
         local left = footer.ui.toc:getChapterPagesLeft(footer.pageno)
-        return prefix .. " " .. (left or footer.ui.document:getTotalPagesLeft(footer.pageno))
+        return prefix .. " " .. ((left or footer.ui.document:getTotalPagesLeft(footer.pageno)) + 1)
     end,
     percentage = function(footer)
         local symbol_type = footer.settings.item_prefix or "icons"

Don't have a pdf to test on hand, but I don't see a reason for it not to work.

Can this be made a permanent fix in KOReader?

@NiLuJe
Copy link
Member

NiLuJe commented Apr 26, 2021

No, because it makes no sense.

@poire-z
Copy link
Contributor

poire-z commented Apr 27, 2021

Well, depends on the mindset of the user. And I'm with @JSWolf on this one :/

Very early when I discovered KOReader, I did patch this in my own patch, because I was always doing +1 addition in my head for these numbers to talk to me.
May be the current behaviour is true to "pages left in chapter", but what I expected to see is 12/12 to 1/12, not 11/12 to 0/12. May be this was accentuated by the fact I added this chapter total pages, don't remember which problem (having to add +1, or now seeing the nb of pages) came first :)
Fwiw, my patch:

@@ -258,7 +266,12 @@
         local symbol_type = footer.settings.item_prefix or "icons"
         local prefix = symbol_prefix[symbol_type].pages_left
         local left = footer.ui.toc:getChapterPagesLeft(footer.pageno)
-        return prefix .. " " .. (left or footer.ui.document:getTotalPagesLeft(footer.pageno))
+        -- return prefix .. " " .. (left or footer.ui.document:getTotalPagesLeft(footer.pageno))
+        -- so we see the total number of pages for this chapter
+        local done = footer.ui.toc:getChapterPagesDone(footer.pageno)
+        left = left or footer.ui.document:getTotalPagesLeft(footer.pageno)
+        done = done or footer.pageno-1
+        return (left+1).." > "..(left+done+1)
     end,
     percentage = function(footer)
         local symbol_type = footer.settings.item_prefix or "icons"

@Frenzie
Copy link
Member

Frenzie commented Apr 27, 2021

because I was always doing +1 addition in my head for these numbers to talk to me.

Indeed? I pretty much only care about something like:

<5 will almost certainly finish the chapter
<10 will probably finish the chapter

20 okay, time to sleep, it'd take too long

Whether it's one page off hardly matters. (But as I said, it's not.)

@Jellby
Copy link
Contributor

Jellby commented Apr 27, 2021

Perhaps I'd think differently if I displayed 0/12, but I just display 0 and have no issues. Also, if I'm at page 10/12, there are 2 pages left, and 10+2 = 12; if it said there are 3 pages left (as suggested), then 10+3 = 13, but there are only 12 page, that would be confusing.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 27, 2021

Yeah, that's what I was going to say ;). What @poire-z showed makes some kind of sense (even if I'd also find it confusing), but it's arguably doing something completely different than the current widget.

Something that possibly warrants being a different mode on its own would perhaps be something like the "Current page" widget, but for the current chapter. So, yeah, 1 / N to N/ N.

EDIT: Which, err, isn't exactly what @poire-z uses, actually, but, oh, well :D.

@Galunid
Copy link
Member

Galunid commented Apr 27, 2021

I think the problem here is whether we count the current page as read or not. When you start the last page of the chapter you technically have 1 page left to finish the chapter. When you finish the last page you have 0 pages left. I think it boils down to personal preference. We could probably add a setting for this since @JSWolf is not alone I guess. I've found it a bit confusing too when I first got into KOReader, but I've gotten used to it since.

@Frenzie
Copy link
Member

Frenzie commented Apr 28, 2021

I think the problem here is whether we count the current page as read or not.

I don't count it as read. I'm on the page. It's not remaining because it's right here.

It's exactly as @Jellby said. Any other way and you'd say you're reading the before last page of a book while you're reading the last page.

@JSWolf
Copy link
Author

JSWolf commented Apr 30, 2021

0 means there are no pages left. But when I turn to the last page of the chapter, there is one page left because I've not yet read it. So 0 should be on a blank page if you really want to keep 0. When I turn the page that should show 1 I should go to the first page of the next chapter/flow.

But given that not everyone agrees with this, an option to be 0 or 1 would be a good idea. This would solve the problem for everyone. I won't even mind if 0 was made the default. While I can fix this, every time I update, the fix is removed. So this is why an option would be the best solution.

@poire-z
Copy link
Contributor

poire-z commented Aug 15, 2021

A new setting and 3 instances of:

         local left = footer.ui.toc:getChapterPagesLeft(footer.pageno) or footer.ui.document:getTotalPagesLeft(footer.pageno)
+        if footer.settings.pages_left_includes_current_page then
+            left = left + 1
+        end

could give us the following when on the first page of a book with a first chapter of 7 pages:

Current/default:
image

Option for those with the other mindset:
image

(I'll fix my broken \n once I get rewording suggetions :) - and/or fix the awkward "Pages left includes" english plural/singular.

@Jellby
Copy link
Contributor

Jellby commented Aug 15, 2021

Suggestions:

"Include current page in pages left"

"Normally, the current page is not counted as remaining, so "pages left" (in a book or chapter with n pages) will run from n-1 to 0 on the last page. With this enabled, the current page is included, so the count goes from n to 1 instead."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants