Display two columns on the iPhone Duo and wide landscape viewports - #903
jeremygrenier wants to merge 1 commit into
Conversation
|
Hi, thank you for addressing this issue! However, we do not modify Readium CSS directly in this project. It must be fixed upstream before we can pull it into the toolkit: https://github.com/readium/css Upgrading to Readium CSS v2 is the top priority for the final 4.0 version, so we will be able to pull the changes pretty quickly if they are merged in Readium CSS. |
|
CSS v2 no longer handles auto columns and media queries so it should already be OK once you migrate. |
|
@jeremygrenier Do you still have the issue using #890? |
|
Note that if you wanted a patch version of v1, I'd have to set up a branch from tagged v1.1.1 in Readium CSS so that your PR can be made into it. We haven't had the need to maintain v1 so far, so it's not been set up. Not sure how the migration to v2 deals with auto columns – that we implemented in TS toolkit has an algo based on available space and min, optimal, max line-length. But if there is nothing in place for switching from 1 to 2 columns and vice versa, a single column will be displayed by default. |
|
Good point, I don't think it was implemented yet in this PR, so it probably won't help. I don't think this warrants a Readium CSS v1 patch since the goal is to move to v2 for Swift toolkit 4. |
|
Noted. Thanks. I'm not sure how that will be implemented given we use It's been simplified quite a lot following your feedback and input at the time so it'd probably be more of a refresher but of course your biggest challenge is about measuring – or well approximating. |
|
@mickael-menu @JayPanoz Thank you for the feedback. The reason I went with this approach was because I wasn’t sure if the v2 migration would be ready for the iPhone Duo launch, so I went with a quick fix that only impacts this repo and iOS devices. I haven’t looked too closely at the v2 PR yet, but I can do so if needed. Do you have an ETA for when the PR should land so I can work on top of it? |
|
It is likely, but I cannot guarantee it. I am considering making an exception and merging this, since it will be removed during the migration to Readium CSS v2 anyways. Because the column strategy will change completely, this will not be a direct port. Though this means you will need to use a v4 alpha or beta of the Readium toolkit in production. We already use it in production with Thorium Mobile, the biggest risk is a few API changes before the final stable version. |
|
@mickael-menu I didn’t realize |
|
We still have one month before the Duo ships. Let's wait a bit to see how the Readium CSS v2 merge progresses, as toolkit v4 will be mostly ready after that. |
|
@mickael-menu as discussed yesterday during the engineering call, could you open an issue in CSS? Or transfer it if an issue already existed in Swift? I am pretty sure I will forget some details e.g. media query etc. if I do so myself as I'm juggling vastly different tasks across projects ATM. 😅 |
|
@JayPanoz Sure, here you go: readium/css#247 |
|
FYI The iPhone Duo is supposed to ship on October 23rd. |
|
Thanks! |
This branch has not been deployed
3 participants
The current column logic doesn't work on the new iPhone Duo: unfolded in landscape, it renders a single column across the full width.
Viewport dimensions measured on the device:
Two columns are gated behind a media query relying on
device-width, which reports the cover display, and on a 960px floor sitting just above the Duo's 951px. This PR makes the query viewport-based and lowers the floor to 950px. It also adds ahorizontal-viewport-segments: 2arm for the half-folded posture, inert until WebKit supports it.Note that a 13" iPad in portrait now shows one column instead of two.
I tried to keep the change as simple as possible, but we will need to be careful with #890 (Readium CSS v2).