[WIP] Seamless layout for pictures

[jbtbnl]

Refs owncloud/gallery#5

TODO list:

• show icon-loading until first pictures are displayed
• fix opening pictures
• fix album navigation
• improve performance
• fix public access

CC @owncloud/designers

[karlitschek]

I think this looks very cool and a lot better. @icewind1991 @jancborchardt What do you think?

[jbtbnl]

@owncloud/designers @jancborchardt @tanghus I'd love to get some feedback before I start working on the public part :)

[tanghus]

Hmm, having a bit of trouble loading the app:

PHP Catchable fatal error:  Argument 1 passed to OCP\\Share::expireItem() must be of the type
 array, boolean given, called in /core/lib/public/share.php on line 357 and defined in 
/owncloud/core/lib/public/share.php on line 843
PHP Stack trace:, referer: http://owncloud/index.php/apps/gallery
PHP   1. {main}() /owncloud/core/index.php:0
PHP   2. OC::handleRequest() /owncloud/core/index.php:30
PHP   3. OC_Router->match() /owncloud/core/lib/base.php:666
PHP   4. call_user_func() /owncloud/core/lib/private/router.php:127
PHP   5. OC::loadAppScriptFile() /owncloud/core/lib/private/router.php:127
PHP   6. require_once() /owncloud/core/lib/base.php:742
PHP   7. OCP\\Share::getShareByToken() /owncloud/gallery/ajax/thumbnail.php:12
PHP   8. OCP\\Share::expireItem() /owncloud/core/lib/public/share.php:357

The keyword here being Catchable

Edit: Wrong branch in core :P

[tanghus]

After checking out the proper branch in core it works great 😉 but I do get a weird blank in the middle of the page.

Sometimes I get double folders when clicking from a sub-folder up to the parent using the breadcrumbs.
I don't know how to reproduce it.

And when loading the page there's a horizontal scroll bar. It disappears when resizing the window, so maybe trigger resize on page load?

[jbtbnl]

@tanghus thanks for your feedback!

I'm not sure what causes the blank space in the middle of the page. Could you please send me the generated HTML?

I suppose the duplicate images you got where only related to sub-folders? I do somehow need to filter out pictures that are already on their way from server to client but belong to the previous view.

I do filter out some outdated pictures here https://github.com/owncloud/gallery/pull/17/files#diff-721c4fdccbf9e7c9a389daf09b558bfcR165 but that does not work for duplicates or sub-folder images...

[tanghus]

The link I was hovering had no image inside, only an empty label, and as you can see on the left, it wrapped to the next line.

[tanghus]

I suppose the duplicate images you got where only related to sub-folders? I do somehow need to filter out pictures that are already on their way from server to client but belong to the previous view.

It was in the root folder, and actually both folders and images were doubled. I'll see if I can find a way to reproduce it.

[jbtbnl]

The link I was hovering had no image inside

@tanghus can you check if the thumbnail does exist? I seems to be the only image with JPEG extension.

[tanghus]

@jbtbnl The thumbnail is there. It looks like somehow it creates a double entry for that image. One without a thumbnail, one with:

<a class="image" data-path="tanghus/Images/Do not look behind you.jpeg" href="/index.php/apps/gallery/ajax/image.php?file=tanghus%2FImages%2FDo%20not%20look%20behind%20you.jpeg" style="height: 196.1px; width: 253.6px;">
<label> </label>
</a>
<a class="image" data-path="tanghus/Images/Do not look behind you.jpeg" href="/index.php/apps/gallery/ajax/image.php?file=tanghus%2FImages%2FDo%20not%20look%20behind%20you.jpeg" style="height: 196.1px; width: 253.6px;">
<img src="/index.php/apps/gallery/ajax/thumbnail.php?file=tanghus%2FImages%2FDo%20not%20look%20behind%20you.jpeg" style="height: 196.1px; width: 253.6px;">
<label> </label>
</a>

[tanghus]

OT, but maybe this PR in core is relevant for Pictures app? owncloud/core#6692

[jancborchardt]

Woah, nice! Good to get a cool layout for the pictures app. (Also see owncloud-archive/apps#1527 ;)

@owncloud/designers we need to test it on different devices to make sure it works well on mobile as well.

[jancborchardt]

(Also cc @xMartin and @anagromataf because they’ll like the new layout. ;)

[anagromataf]

@jancborchardt I definitely need to spend some time on this. Maybe as inspiration my hack during the AHT: http://pina.delphinus.uberspace.de

[jancborchardt]

@anagromataf yup, check out the above screenshots and this code. :) Looks very similar to your layout / to the Flickr layout.

[anagromataf]

I already saw it. Looks great.

[davidak]

looks very good. thanks @jbtbnl

[jancborchardt]

Some Feedback:

• we need a better design for the folders. We should really go back to the »4 small images from inside the folder« design. Otherwise it looks like it’s just a single image, even when we use this »stack« design.
• the folder name currently overlaps, we need to ellipsize it
• there should be a bit more margin between the images, like 5px. Also, it needs to be harmonious, visually the same as the the margin on the side.
• for mobile, the size should also shrink. Currently when viewing on a smartphone, you see only 1 or 2 images. You should be able to see about 6 or 8.

What do you think @jbtbnl? Awesome work on it in general!

[jbtbnl]

@jancborchardt I agree with you on all points, although we should take a closer look at the margin of 5px since it will make pictures with a lot of white look ugly. Maybe introduce a subtle border?

@jancborchardt I hope that someone will pick up owncloud/core#7881 since it could really improve the speed of the seamless gallery and it could enable the app to beforehand decide what size of the image should be downloaded, leading to lower bandwidth usage on mobile phones. Maybe you'd like to stress the importance of the feature there ;)

[jancborchardt]

@jbtbnl that touches a point I wanted to mention, but removed because I thought it would be too much:

We should probably make the background dark instead of light to distract less from the pictures. Most photo viewers or picture management software has a dark background so it’s less jarring. It’s already like that on the public share (although we probably need to change the color slightly because it conflicts with the dark app list which has a slight blue hue.)

What do you think?

[jbtbnl]

@jancborchardt I quite like the solution in Google+, they have a slightly transparent 1px border that overlaps with the outer pixels of the image. Therefore the border always darkens the outline of the picture a bit, being invisible at normal pictures but resulting in a nice outline for white edges.

Google+ has a weird solution with div's surrounding the image, probably because they try to support every browser possible. A nice CSS solution would be:

box-shadow: inset 0 0 1px rgba(0, 0, 0, 0.3); background-size: 100%; and then setting the image as background to the link element instead of inserting an image.

[tanghus]

Google+ has a weird solution with div's surrounding the image, probably because they try to support every browser possible.

I think they're just trying to obfuscate the markup as much as possible. Have you ever tried to inspect Gmail? It's so fugly you won't believe it :D The opposite of semantic markup.

[icewind1991]

I've started implementing infinite scroll for the gallery to properly handle loading of large albums here which also uses seamless layout since the row based loading method made implementing seamless easy

cc @jbtbnl @jancborchardt

[karlitschek]

@icewind1991 Nice. Would this also fix the problem of the slow performance with a huge number of pictures? Someone mentioned that we use a huge json ajax response for all pictures at the moment

[icewind1991]

That solves the issue of the gallery loading a huge numbers of thumbnails at once when opening a large album, listing images is still done in one request for now (although I did reduce the size of the ajax response by about 5 times earlier)

[jancborchardt]

@icewind1991 nice! But that’s one huge commit ;) Shouldn’t we tackle layout and loading separately?

[icewind1991]

Both loading and layout require splitting the images into rows. With lazy loading already generating the rows showing them in a seamless layout is only a little bit of work.

[jancborchardt]

@icewind1991 so, you incorporated this code in your loading fixes? Do we need this pull request still then? (Also, can you please link the pull request?)

[icewind1991]

The functionality of this PR is included in the loading fixes, there isn't a PR for it yet (displaying shared pictures needs to be implemented first) but the code can be found here: https://github.com/owncloud/gallery/tree/infinite-scroll

[jancborchardt]

@icewind1991 can you open a Work-In-Progress PR for it so everyone knows the state of it?

[jancborchardt]

And then I guess we could close this one if @jbtbnl’s commits are in your branch?