edrlab / thorium-reader

A cross platform desktop reading app, based on the Readium Desktop toolkit
https://www.edrlab.org/software/thorium-reader/
BSD 3-Clause "New" or "Revised" License
1.85k stars 157 forks source link

Create a UI related to epub:type=pagebreak #645

Closed llemeurfr closed 5 years ago

llemeurfr commented 5 years ago

From Pierre:

I think that the page number should be displayed in the bottom slider :

image

For now we can access to a specific page without known the page number. Perhaps we can open an issue with this feature ?

Originally posted by @panaC in https://github.com/readium/readium-desktop/issues/368#issuecomment-507305711

llemeurfr commented 5 years ago

Related to #20

llemeurfr commented 5 years ago

Two markups coexist in the real world:

Markup 1:

<span id="page001" title="1" epub:type="pagebreak"/>

and Markup 2:

<span epub:type="pagebreak" id="page001" class="whatever">1</span>

See article: http://sketchytech.blogspot.com/2017/01/when-is-page-break-not-page-break-epub.html

We need samples: are there IDPF samples containing epub:type="pagebreak"?

llemeurfr commented 5 years ago

So, instead of trying to display page breaks in the context of the content itself, like Jiminy Panoz tried in #20, could we display page numbers (when present) in the timeline?

danielweck commented 5 years ago

We need samples: are there IDPF samples containing epub:type="pagebreak"?

Sure, wherever there is a pagelist, there are page-breaks :) (in the EPUB epub:type sense, and/or ARIA role mapping equivalents)

All the details are in the original issue: https://github.com/readium/readium-desktop/issues/82#issuecomment-521457136 (I suggest we don't duplicate info, if we can avoid it)

danielweck commented 5 years ago

Two markups coexist in the real world: ...

Actually, there is also role equivalents to epub:type, and aria-label vs. title attributes too. See: https://github.com/readium/readium-desktop/issues/82#issuecomment-526295012

danielweck commented 5 years ago

So, instead of trying to display page breaks in the context of the content itself

I don't think reading systems should do anything special with HTML elements marked with epub:type="pagebreak" or role="doc-pagebreak" ... content authors can decide (and usually do) to hide skippable items such as "page numbers / breaks" (CSS display: none is the usual technique), or to inline versus block-display them, etc. This is naturally an area where ReadiumCSS could be opinionated, by enforcing design choices (see discussions in https://github.com/readium/readium-desktop/issues/20 ).

So I think we should just focus on what affordances Thorium can offer to expose the "pagelist" nav (not any arbitrary pagebreaks in HTML content documents, but just those that are authored in the EPUB NavDoc).

1) flat, non-hierarchical list (fully visible + scrollable, or on-demand pulldown list) => trivial 2) search input text ("goto page number x") => trivial too (string matching, number radix) 3) maybe some mapping with bottom timeline, but this would require locator computations with percentage progress, just like mapping individual (sub)headings => not trivial

JayPanoz commented 5 years ago

Re. ReadiumCSS could be opinionated, it’s worth noting experiments in Readium CSS definitely ended up altering the DOM with pseudo elements – generated content and screen readers already being an issue in and of itself.

llemeurfr commented 5 years ago

The reason why I opened this issue separate from #82 is that I thought that page lists (as a nav structure) and page breaks (as inline elements in the HTML content) were two alternative ways to indicate page numbers. I see now that they are complementary (the nav reference points to the inline id). Before we close this issue and centralize the discussion in #82, a quick question: does EPUBCheck checks in referential integrity of such ref/id system?

danielweck commented 5 years ago

a quick question: does EPUBCheck checks in referential integrity of such ref/id system?

Yes. EPUBCheck verifies the linear order requirement too (within HTML documents, as well as across the spine).

References:

https://github.com/w3c/epubcheck/blob/6af3b9800e1481b634c1fb064afff594207f7357/src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java#L230-L253

https://github.com/w3c/epubcheck/blob/6af3b9800e1481b634c1fb064afff594207f7357/src/test/resources/30/expanded/valid/nav-page-list-reading-order/OPS/nav.xhtml#L15-L22

https://github.com/w3c/epubcheck/blob/6af3b9800e1481b634c1fb064afff594207f7357/src/test/resources/30/expanded/invalid/nav-page-list-unordered-fragments/OPS/nav.xhtml#L14-L20

https://github.com/w3c/epubcheck/blob/6af3b9800e1481b634c1fb064afff594207f7357/src/test/resources/30/expanded/invalid/nav-page-list-unordered-spine/OPS/nav.xhtml#L16-L22

danielweck commented 5 years ago

Laurent, can we close this now? ("goto page" feature for EPUB nav@epub:type="pagelist" is what we have right now)

llemeurfr commented 5 years ago

The "Go to page" feature now added is sufficient. Let's close this issue.