-
Notifications
You must be signed in to change notification settings - Fork 509
Add backend for conversation avatars #4737
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
Conversation
nickvergessen
left a comment
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.
Find a folder name that can not clash with a user id
Talk in appdata is fine
Add system message when the room avatar is changed
Yes
09ab900 to
37c62f5
Compare
Also someone brought up that the OCS endpoint for the avatar is a bad idea, because it needs the |
The RoomAvatar does not provide (yet) a default avatar for a room if it has not been set. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Now the avatar of the other user will be returned when getting the avatar of a one-to-one conversation. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Although the avatar itself is stored in the app data the avatar id and its version are stored in the database for convenience. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The image itself is always got directly from the user avatar, so it is always up to date. However, as the version is specific to the room avatar and stored in the database it needs to be explicitly bumped. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The generic avatar system will not be included in Nextcloud 21, so for the time being Talk needs to provide its own endpoints for room avatars. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
|
At least in the browser OCS requests are cached (I have verified it on #4872). If mobile apps can not cache OCS requests then yes, it would be better to move the get endpoint to something cacheable. But it would be good to get a confirmation that it does not work before doing the change. Or should it be a |
7332d60 to
74cdd1e
Compare
|
I will close the PR for now (as there was no active development since 6 months). Anyone can pick it up whenever they want and reopen the PR. |
Requires nextcloud/server#24579The generic avatar system will not be available in Nextcloud 21, so a specific Talk endpoint was added instead.Fixes (the backend part of) #927
The avatar endpoint returns custom room avatars (that is, avatars that have been explicitly set by a participant) as well as one-to-one room avatars (the avatar of the other participant in the conversation). Other room avatars are just default icons (
icon-public,icon-contacts...) and getting the avatar in that case returns an error 404.Room list entries now also return
avatarIdandavatarVersionparameters. The first can be used to know the type of avatar (custom,user,icon-public...), and the second to know when an avatar has been updated. Due to all this a client should get the avatar whenever the version changes, but only if the avatar id iscustomoruser.As the avatar is provided in an OCS endpoint and thus requires the
OCS-APIRequestheader to be set it is not possible to just set the endpoint URL as the URL of an HTML image element or a CSS background image. Instead the request needs to be done specifying that aBlobis expected and then getting the URL for thatBlob. For example, something like:The original idea was that the endpoint would return default avatars too. Unfortunately this was more complex than anticipated;
IAvatardoes not support SVGs, and converting from SVG to PNG (which might be needed even if SVGs can be used for the maximum compatibility with clients) is only possible when ImageMagick is available. As the default avatars are common to all the conversations of the same type and the clients should be able to provide the icons by themselves for the time being this should be good enough.Pending:
OC\Avatar\Avatarinstead of implementingOCP\IAvatarto get the implementation of [avatarBackgroundColor](https://github.com/nextcloud/server/blob/caff1023ea72bb2ea94130e18a2a6e2ccf819e5f/lib/private/Avatar/Avatar.php#L298]? I am not a fan of extending private classes, but maybe in this case it would be acceptable to avoid code duplication - The method is not really needed, so a dummy implementation was added insteadicon-public,icon-group,icon-mime-audio,icon-mime-text,custom...) when fetching room informationFuture: