Skip to content

Conversation

@mario
Copy link
Contributor

@mario mario commented May 24, 2018

Videos + Photos need to be merged into one item as gallery. This adds library support for it.

cc @ardevd @AndyScherzinger @tobiasKaminsky

@AndyScherzinger
Copy link
Member

code-wise it looks fine to me, is there a corresponding app branch where this is integrated?
On a general note @mario @tobiasKaminsky how do we handle this combined view for servers which simply don't create previews for video files? One way would be to just show the "default video type icon" while that doesn't look that nice but might be the only way...

@mario
Copy link
Contributor Author

mario commented May 25, 2018

@AndyScherzinger I just did a quick hack for the app to test this while developing, there's no corresponding app branch since someone else will have to do it.

The reason is that over a year ago we agreed to merge videos+photos into a Gallery both into iOS and Android, so I had to see how to do it. This PR is the result of it :)

@AndyScherzinger
Copy link
Member

@mario could you just push your hack to a branch on the app repo, that we I/we would have an impression of how to integrate and could pick it up from there :) That be of great help :)

@mario
Copy link
Contributor Author

mario commented May 30, 2018

@AndyScherzinger I deleted this as it was literally a few lines change to send GALLERY_SEARCH over to the SearchOperation. :-/

@tobiasKaminsky
Copy link
Member

After release of 3.2 I can have a look to implement it in app.
But then we really need the "load only first 50/100 items" and then load next ones after reaching bottom, as with both pictures and videos the search will never ever finish…

@mario
Copy link
Contributor Author

mario commented May 30, 2018

Well, didn't we talk about search pagination and caching the proper way @icewind1991? :P

@AndyScherzinger
Copy link
Member

@tobiasKaminsky @mario for whatever reason the iOS client is super fast for this view?!

@tobiasKaminsky
Copy link
Member

There is already some work done by Roeland and me: nextcloud/android#2250

@AndyScherzinger
Copy link
Member

There is already some work done by Roeland and me

@tobiasKaminsky iOS is super fast now! Not needing any Nc14 stuff, so that doesn't matter it seems that this can be solved today...

@tobiasKaminsky
Copy link
Member

nextcloud/ios#583
I asked over there :-)

@mario
Copy link
Contributor Author

mario commented Jun 9, 2018

I need review on this @AndyScherzinger @tobiasKaminsky @ardevd

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jun 9, 2018

👍

Approved with PullApprove

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Jun 12, 2018

👍
I will have a look at the PR for app soon.

Approved with PullApprove

mario added 4 commits June 12, 2018 08:39
Signed-off-by: Mario Danic <[email protected]>
Signed-off-by: Mario Danic <[email protected]>
@nextcloud nextcloud deleted a comment Jun 12, 2018
@tobiasKaminsky tobiasKaminsky merged commit 5be8718 into master Jun 12, 2018
@tobiasKaminsky tobiasKaminsky deleted the gallery-search branch June 12, 2018 07:08
@AndyScherzinger AndyScherzinger added this to the NC lib 1.0.43 milestone Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants