Giter VIP home page Giter VIP logo

Comments (21)

danielweck avatar danielweck commented on June 11, 2024

screenshot

Insightful comments by @JayPanoz starting here:
readium/r2-navigator-js#18 (comment)

from readium-css.

danielweck avatar danielweck commented on June 11, 2024

Possibly related:
#50
#10

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

As regards “possibly related”.

  • #50: this sample is really, really, a stink bomb in the sense it “implements” one of the worst edge cases possible. Nothing personal to the people who made it, obviously, but this is a case that is not even discussed in the EPUB spec i.e. publications with documents in both horizontal and vertical writing. It’s been popping up over and over and over again but the cover styling doesn’t even work properly in a “vanilla” web browser – I checked that like 5 times minimum, with Readium CSS, without Readium CSS, without any EPUB logic, etc., and it failed every time. That being said, it’s kinda my fault, I should have opened an issue about this sample long ago.
  • #10 is about how the viewport is altered, and how that impacts viewport relative lengths. When using iframe, you can for exemple set an explicit height and all of a sudden, 100vh can become 15000px instead of being the browser’s window’s 860px because it’s the height of the iframe that counts for the viewport in the EPUB styling context. I can confirm that is something that shouldn’t happen when using web views.

So to sum up, this issue can be scoped to the additional margin we’re setting up in scroll mode.

/* Make sure line-length is limited in all configs */
:root:--scroll-view body {
--RS__maxLineLength: 40rem !important;
margin-top: var(--RS__pageGutter) !important;
margin-bottom: var(--RS__pageGutter) !important;
}

Thanks for opening this issue BTW. 🙏

from readium-css.

danielweck avatar danielweck commented on June 11, 2024

I can confirm that is something that shouldn’t happen when using web views.

:)

...well, unless the said "web view" is Electron's own interpretation/implementation (derived from Chromium webview) which is in fact an embedded object (custom element + shadow DOM) that emulates a sandboxed iframe (isolated in a separate process), so vh and vw may in fact apply (to be honest, I haven't checked thoroughly).

See:
https://electronjs.org/docs/api/webview-tag
... I know, there is an alarming disclaimer at the top, but readium-desktop, r2-testapp-js and of course r2-navigator-js all build on top of Electron 1.* which isn't so deprecating about webview:
https://github.com/electron/electron/blob/v1.8.8/docs/api/webview-tag.md
We are tracking this problem for later:
readium/r2-navigator-js#15

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

Right, I’ll follow up tomorrow as I’m quite frankly exhausted right now and some comments may appear passive-agressive – sorry about that, wasn’t my intention – but we’d better have a solid reason for changing how that works, especially as it’s in the 2017 CSS snapshot, on which EPUB 3.2 is now relying – and well, if I’m being honest and although I’m trying really hard to be neutral, it’s very difficult to not be on some kind of a mission as I personally saw the nicest and most talented people quit ebooks because ebooks break such standards and they couldn’t take it anymore.

For vw (viewport width) I’m totally cool saying “Hey, don’t count on that, it’s not compatible with CSS multicol, that we’re using for pagination.” For vh though, I’m super reluctant, as this is quite possibly the only way out for styling covers as far as authors are concerned – and we’re also using that actually, in multiple places – and spoiler alert: authors are primarily writing their styles for pagination, most won’t even test their ebooks in scroll.

At first I thought it would maybe impact scroll-continuous, but this isn’t necessarily the case – and the scope may be well wider. So I’ll take a look tomorrow, thanks for the links BTW.

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

That + some vendors advise vh actually so people are already following those guidelines.

In Flowing EPUB books, it is recommended to size images using viewport units to maintain adaptability for various screen sizes.

Source: https://help.apple.com/itc/booksassetguide/#/itca71ad3c33

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

But then again, note the primary issue is about adding a margin-top and margin-bottom in scroll mode so that text content is not “glued to the edges” of the RS there.

Works for text but not for files consisting of images only. So I’d really like to scope this issue to that and see what our options are – not so much using CSS only unfortunately, but it’s worth discussing imho.

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

Alright, here’s an illustrated recap on scroll styling.

So right now, we have the following box model for body:

body-box

Don’t pay attention to horizontal margins (372, we actually don’t set this one, our value is auto). Other than that:

  1. body has vertical margins of var(--RS__pageGutter) so 50px here ;
  2. body has horizontal padding of var(--RS__pageGutter) so 50px too;
  3. the “content container” is therefore 540px wide.

But why did we set vertical margins?

It was all about text.

Note: In the following screenshots, orange (outer) is body’s margins, green (in-between) is body’s padding, and blue (inner) is “content container” (div, figure, etc.)

For instance, here’s a chapter without any margin-/padding-top.

text-top-nomargin

I can recall a heading in our test files that was somehow partly hidden at the top of the chapter because we didn’t apply any margin. I can’t remember why exactly (maybe negative margin-top, relative positioning, or the iOS navbar hiding part of it only in the scroll view) but we added that as some sort of safeguard so that it can’t happen.

text-top-margin

And same for the bottom of chapters.

Without a margin

text-bottom-nomargin

With a margin

text-bottom-margin

So the idea was to give some extra vertical spacing for content to breathe.

Let’s now look at images (e.g. cover).

Without a margin (note we constrain the height to 95vh (so 95% of the viewport’s height so that we don’t accidentally create blank pages in the paged view).

cover-nomargin

With a margin

cover-margin

I can confirm there’s overflow in this last screenshot, although you can’t see the scrollbar (because MacOS is buggy). But now the height of the document is the height of the image (95vh) + the margins (50px * 2) so it is obviously overflowing.

This is obviously something we don’t want, we want no extra vertical margins there.

What the issue is

In CSS, you can’t conditionally apply margins depending on the document’s content (text vs. only one image). I indeed can’t use such logic:

if body has only an image
then remove margin-top and -bottom from body

Because when checking this image, I’m already getting out of body’s scope, I’m further down the DOM (the image) and I can’t “bubble up” i.e. styling its parent elements.

All I can do is expecting some kind of flag, so in ReadiumCSS by default, a custom property such as style="--RS__imgOnly: true" either on html or body e.g.

:root[style*="--RS__imgOnly: true"] body {
  margin-top: 0;
  margin-bottom: 0;
}

/* Note that could be [style*="--RS__imgOnly: false"] but since `--RS__imgOnly` is a boolean, no need to use it */
:root body {
  margin-top: var(--RS__pageGutter); /* 50px */
  margin-bottom: var(--RS__pageGutter); /* 50px */
}

So all we can do with vanilla CSS is either applying vertical margins to all documents or not applying them.

Hypothetical fixes

Such logic is JS territory, either in the form of:

  1. an :has() (CSS pseudo-class) polyfill – but brace yourself for the handling of all author markups possible;
  2. a logic that checks what is in the document and apply a flag to html or body, and that I can use in CSS to target body (could also be parser and not JS at runtime per se).

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

Related: scrolled-continuous rendition e.g. how will it be implemented, will it break vh, how to make a clear distinction between files, etc.?

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

After further reflection, I’m wondering whether rolling back to no vertical margins and letting the app handle those, as in the paged view, wouldn’t be the most reasonable call. That would enforce consistency for implementers, which in my opinion is most important.

from readium-css.

ronghester avatar ronghester commented on June 11, 2024

I have observed a similar stretching of cover image issue with ebook. The cover image is rendered from start.html and since it is provided by the publisher it can have different html and css properties. For example i am showing below the html of two ebooks where in first book the cover image rendered perfectly but in other it gets stretched to multiple pages.

HTML in start.html where cover image does not stretch.

<body style="margin:0.00em; text-align:center;">
   <p class="cover" style="text-align:center;"><img style="height: 100%;" src="9780857633446_cover_epub.jpg" alt=""/></p>
 </body>

Cover image is stretch in this book

<body style="margin-top:0px; margin-left: 0px; margin-right: 0px; margin-bottom: 0px;  text-align: center; background-color: #ffffff;">
 <div><img src="docimages/cover_ader.jpg" alt="cover" style="height: 100%" /></div>
</body>

Apparently we have observed this issue when migrated to recent readium-sdk along with LCP. In the previous setup which was using 2-3 yr old readium this problem was not so much evident.

We are using readium on android & IOS platform.

Is there any easy fix for this ?

Thanks

from readium-css.

danielweck avatar danielweck commented on June 11, 2024

we have observed this issue when migrated to recent readium-sdk

Just to be clear: you mean Readium2, where ReadiumCSS is used to apply document formatting, right?
(I am asking just in case you use a hybrid Readium1 + ReadiumCSS solution)

from readium-css.

ronghester avatar ronghester commented on June 11, 2024

We are using Readium1 from this branch : https://github.com/readium/readium-sdk/tree/feature/latest-working-build-config/Platform/Android

from readium-css.

danielweck avatar danielweck commented on June 11, 2024

Oh, so you are not using ReadiumCSS, right?

from readium-css.

danielweck avatar danielweck commented on June 11, 2024

For Readium1, please report in readium-shared-js, e.g.

readium/readium-shared-js#342

https://github.com/readium/readium-shared-js/blob/7f245beba1ed97eaabce0aa5e9cf2f3b23e8f8f6/js/views/reflowable_view.js#L988-L1018

(the image handling code in Readium1 SDK has not changed for months/years, but your migration from a very old revision of Readium1 may indeed have introduced different behaviours / regression bugs)

from readium-css.

ronghester avatar ronghester commented on June 11, 2024

Yes we are not using ReadiumCSS in the current setup. Also we were not using it earlier. In asset folder there are some css file present such as SDK.css etc.. I believe readiumCSS is applicable only to readium 2 and onward, right?

Thanks i will report in readium-shared-js project.

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

So yeah, readiumCSS is applicable to r2 onwards.

Note we have a list of CSS-related issues from r1 that this project is attempting to fix in the wiki: https://github.com/readium/readium-css/wiki/CSS-related-issues-in-Readium-1 – uncrossed items don’t necessarily mean we haven’t fixed it, maybe it doesn’t apply, or we haven’t had the opportunity to address it yet, etc.

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

cc @danielweck Removed those vertical margins in develop branch. Please let me know if this fix the overflow: scroll issue for covers.

from readium-css.

danielweck avatar danielweck commented on June 11, 2024

Thanks Jiminy, I've merged your updates into the ReadiumCSS copy of r2-navigator-js:
readium/r2-navigator-js@4d95642

Test with Metropolis (which I found in your screenshots readium/r2-navigator-js#18 (comment) ), from Feedbooks:
http://www.feedbooks.com/book/1123/metropolis

Load the remote EPUB directly in r2-testapp-js using the command line: npm run electron http://www.feedbooks.com/book/1123.epub.

Cover and text in paginated mode (CSS columns):
Readium2_testappJS_Metropolis_Margins3
Readium2_testappJS_Metropolis_Margins2

Cover and text in scroll mode:
Readium2_testappJS_Metropolis_Margins4
Readium2_testappJS_Metropolis_Margins1

As you can see, no extraneous scroll extent, thanks to the removed top/bottom margins in ReadiumCSS.

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

No problem, and thanks for the confirmation.

Will resolve this issue when merging this change.

from readium-css.

JayPanoz avatar JayPanoz commented on June 11, 2024

Fixed via #63

from readium-css.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.