readium / r2-testapp-swift

BSD 3-Clause "New" or "Revised" License
146 stars 38 forks source link

Adjust number of books shown in the Collection View #43

Closed aferditamuriqi closed 6 years ago

aferditamuriqi commented 6 years ago

Currently no matter the device we show just 3 books in a row. which is not ideal for larger screens like iPads

HadrienGardeur commented 6 years ago

We're currently working on this for Aldiko and decided to use 4 in portrait and 5 in landscape on the iPad.

aferditamuriqi commented 6 years ago

How does the 4 and 5 look on an iPad Pro? would it make more sense to set the size of the cover and have it dynamically instead of the number of covers per row?

HadrienGardeur commented 6 years ago

I don't own an iPad Pro, so I haven't tried it device in hands yet (which IMO is the only good way to test this out).

On a normal iPad, it looks really quite good. We had 3 covers in portrait before and I really didn't liked it much.

I wouldn't do dynamic scaling based on the size of the covers, I find this confusing from a UX perspective. Ideally this would be entirely adaptive, but once you reach a certain number of items per row, it's better to add whitespace than more content (this will be extremely important on desktop and Chrome OS).

aferditamuriqi commented 6 years ago

ok, that makes sense, I will test it on my iPad Pro and see how it would look with 4 and 5, once we have this update, but it shouldn't be an issue distinguishing between phone, iPad and iPad Pro if needed.

aferditamuriqi commented 6 years ago

I am closing this since this has been already completed and has a merged PR