readium / readium-css

🌈 A set of reference stylesheets for EPUB Reading Systems, starting with Readium Mobile
https://readium.org/readium-css/
BSD 3-Clause "New" or "Revised" License
90 stars 20 forks source link

Setting vertical margins of :root #98

Closed mickael-menu closed 2 years ago

mickael-menu commented 3 years ago

I'm submitting a feature request.

In some context (notch screens, scroll mode), the RS should be able to set the vertical margins.

What is the current behaviour?

Readium CSS forces :root to have 0 margin and padding:

https://github.com/readium/readium-css/blob/583011453612e6f695056ab6c086a2c4f4cac9c0/css/src/modules/ReadiumCSS-pagination.css#L62-L63

I couldn't find any way to customize this in Readium CSS.

Do you have screenshots, GIFs, demos or samples which demonstrate the problem or enhancement?

There are two problematic areas:

1) With a notch screen / display cutouts, the desired behavior is to have the background fill the screen, but the content should not overlap with the cutouts. Right now it's the case:

Screenshot 2020-11-18 at 15 39 24

2) In scroll mode, the content has no room to breath at the top and bottom of resources, some vertical margin would help:

Screenshot_20201118-161838

Do you know which CSS modules (stylesheets) are impacted?

pagination, I think.

Please tell us about your environment:

Other information (e.g. related issues, suggestions how to fix, links for us to have context)

I could have the expected behavior with the display cutouts by setting padding–top and padding-bottom on html. Somehow margin-top worked as well but not margin-bottom.

Screenshot 2020-11-18 at 15 39 44
mickael-menu commented 3 years ago

If a new setting is needed, it can't be a single variable for the whole vertical axis, like pageMargin, because the cutouts at the top and bottom might be of different heights.

JayPanoz commented 3 years ago

TL;DR: I can’t, as this would break vh (viewport height unit) for authors, so we deemed it the responsibility of the app through web view UI margins very early during ReadiumCSS development.

There are examples of vendors pushing this unit for image sizing (e.g. covers) on top of it so there is definitely significant usage. On the opposite vw (viewport width) was already broken in various and nasty ways in the ecosystem so we thought it was acceptable to handle horizontal margins/paddings in ReadiumCSS directly.

mickael-menu commented 3 years ago

I see, too bad :/ I'll introduce padding in the web view and try to find a way to extract the background color from the web view.

Incidentally, maybe it could be useful to mention it in the documentation?

JayPanoz commented 3 years ago

Ah yeah I see maybe that could be better explained here: https://readium.org/readium-css/docs/CSS03-injection_and_pagination.html#margins-and-dimensions (it’s currently more of a note than an emphasis).

mickael-menu commented 3 years ago

I'm a bit confused how authors handle notches in the browser though, it looks like it's done setting the padding:

https://developer.mozilla.org/en-US/docs/Web/CSS/env()

But I can't think that website not using this are broken in native browsers with notch screens...

JayPanoz commented 3 years ago

Yeah you have to opt-in by adding the following meta in your doc’s head so probably the first thing to check:

<meta name="viewport" content="viewport-fit=cover" />

Otherwise browsers apply some kind of “letterboxing.” Not sure how webviews in native apps are dealing with that though.

mickael-menu commented 3 years ago

Thanks for the insights 👍

It seems to be working alright by applying some padding. The background is not seamless in some cases but works for most books, by using the theme color.

However I'm not sure how to approach the second use case: scroll mode. Applying some padding outside of the web view doesn't give the expected experience because the content is cut off when scrolling, instead of overlapping the display cutouts. In that particular case, would it be a problem to be able to set the vertical margins of :root, since it's not paginated?

mickael-menu commented 3 years ago

After some trial and error I got a solution that visually works for scroll mode, using a NestedScrollView and android:clipToPadding:

    <androidx.core.widget.NestedScrollView
        android:layout_width="match_parent"
        android:layout_height="match_parent"

        android:paddingTop="50dp"
        android:paddingBottom="50dp"
        android:clipToPadding="false"
        >

        <org.readium.r2.navigator.R2WebView
            android:id="@+id/webView"
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            />
    </androidx.core.widget.NestedScrollView>

However I'm afraid it's asking for trouble and might trigger some conflicts with gestures in some devices or edge cases. So if applying the padding in the HTML (for scroll mode) is not detrimental to the content, that would be a safer and more portable solution.

EDIT: Too good to be true, that broke paginated mode.