Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add activity previews #73

Merged
merged 4 commits into from
Dec 14, 2018
Merged

Add activity previews #73

merged 4 commits into from
Dec 14, 2018

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Aug 15, 2017

Tobi: this adds support for using preview url instead of generating the thumbnails locally.

Implementation on client side: nextcloud/android#3130

Needs: nextcloud/activity#300

@nextcloud nextcloud deleted a comment Aug 15, 2017
@nextcloud nextcloud deleted a comment Aug 15, 2017
@nextcloud nextcloud deleted a comment Aug 15, 2017
@nextcloud nextcloud deleted a comment Aug 15, 2017
@tobiasKaminsky
Copy link
Member

So this should work now, please feel free to ask me on irc at any time

@tobiasKaminsky
Copy link
Member

Quick reminder: we need to pass the desired height/width to it, otherwise we will download a too big/too blurry image.

@nextcloud nextcloud deleted a comment Aug 24, 2017
@nextcloud nextcloud deleted a comment Aug 24, 2017
@nextcloud nextcloud deleted a comment Aug 24, 2017
@nextcloud nextcloud deleted a comment Aug 26, 2017
@nextcloud nextcloud deleted a comment Aug 26, 2017
@nextcloud nextcloud deleted a comment Aug 26, 2017
@nextcloud nextcloud deleted a comment Aug 26, 2017
@nextcloud nextcloud deleted a comment Oct 19, 2017
@nextcloud nextcloud deleted a comment Oct 19, 2017
@nextcloud nextcloud deleted a comment Oct 19, 2017
@nextcloud nextcloud deleted a comment Oct 19, 2017
@mario
Copy link
Contributor

mario commented Nov 25, 2017

Do we merge this or what

@AndyScherzinger
Copy link
Member

Quick reminder: we need to pass the desired height/width to it, otherwise we will download a too big/too blurry image.

@tobiasKaminsky is this issue resolved?

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Nov 27, 2017

@nickvergessen what is the status here?
If it is not yet finished, do we want to have a look into it on xmas hacking?

Edit: Talked to joas. As he will soon be on vacation I will have look into it.

@nextcloud nextcloud deleted a comment Dec 7, 2017
@nextcloud nextcloud deleted a comment Dec 7, 2017
@nextcloud nextcloud deleted a comment Dec 7, 2017
@nextcloud nextcloud deleted a comment Dec 7, 2017
@tobiasKaminsky
Copy link
Member

I fear that I need to talk to Joas to understand what the benefit of this is -> postponing.

nickvergessen and others added 4 commits December 10, 2018 09:49
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
@nextcloud-android-bot
Copy link
Collaborator

Lint

TypeMasterPR
Warnings00
Errors00

FindBugs (new)

Warning TypeNumber
Bad practice Warnings11
Correctness Warnings42
Internationalization Warnings5
Malicious code vulnerability Warnings9
Multithreaded correctness Warnings3
Performance Warnings18
Security Warnings28
Dodgy code Warnings85
Total201

FindBugs (master)

Warning TypeNumber
Bad practice Warnings11
Correctness Warnings42
Internationalization Warnings5
Malicious code vulnerability Warnings9
Multithreaded correctness Warnings3
Performance Warnings18
Security Warnings28
Dodgy code Warnings84
Total200

@tobiasKaminsky
Copy link
Member

This one is backward compatible.
Tested on NC14

@tobiasKaminsky
Copy link
Member

Quick reminder: we need to pass the desired height/width to it, otherwise we will download a too big/too blurry image.

@nickvergessen currently this seems to be hardcoded to 150x150 px.
Is there a way that we can pass a desired height/width?

@nickvergessen
Copy link
Member Author

I guess instead of a bool for preview we could make it also an integer and pipe the size through until:
https://github.com/nextcloud/activity/pull/300/files#diff-9dabc5ebf438ea378e7ca813d2ba0f19L390

@nickvergessen
Copy link
Member Author

Feel free to open an issue in the activity app if that's an issue. But wont happen before 15.0.1

@tobiasKaminsky
Copy link
Member

Thanks for pointing out how to do this.
I created an issue.

Once this is implemented, I can develop it for android, but this should not prevent this PR from merging :-)

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Dec 13, 2018

👍

Approved with PullApprove

@AndyScherzinger AndyScherzinger added this to the NC lib 1.3.0 milestone Dec 13, 2018
@mario
Copy link
Contributor

mario commented Dec 13, 2018

👍

Approved with PullApprove

@tobiasKaminsky tobiasKaminsky merged commit d472d37 into master Dec 14, 2018
@tobiasKaminsky tobiasKaminsky deleted the activity-previews branch December 14, 2018 06:59
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.

5 participants