-
Notifications
You must be signed in to change notification settings - Fork 18
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
implement dashboard api for recommended files #543
Conversation
Bug: iconUrl must always be set, as clients cannot know via API which type it is, they cannot fall back to their placeholder. |
linter is also not happy here |
|
||
return array_map(function(IRecommendation $recommendation) { | ||
$url = $this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $recommendation->getNode()->getId()]); | ||
$icon = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreviewByFileId', [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tobiasKaminsky do you mean we should pre-check if a preview exists, and if yes, return the preview URL and if not return the mime type icon URL instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, clients always need a real icon, may it be svg or png.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least in Qt/QML, this is not required as you can easily detect if the image loaded and add a fallback (e.g. mimetype icon). See e.g. https://invent.kde.org/frameworks/kirigami/-/blob/master/src/controls/Avatar.qml#L235
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we do not have any info about the mimetype.
And sending mimetype along side, and then have clients deteminate it and show correct placeholder is way more work than directly sending the correct image.
(widgets on Android / iOS are limited in opposite to our full apps, so we cannot do too much calculation/work there)
@tobiasKaminsky do you happen to have a curl call handy to call this API ? (so that someone else can take over and continue without needing the full setup) |
CURL:
|
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
a93b9a5
to
950869c
Compare
It seems the @PVince81 Need to clarify if this is enough Example of result of the ocs request:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 awesome
@tobiasKaminsky can you review if this solution is now suitable for clients ? thanks |
This works @ArtificialOwl 👏 |
/backport to stable25 |
There was no backport @PVince81 |
/backport to stable25 |
Implement the dashboard api from nextcloud/server#26430 and nextcloud/server#33658
requires: