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 room heroes to RoomSubscription #2702

Closed
pixlwave opened this issue Oct 11, 2023 · 9 comments · Fixed by #3503
Closed

Add room heroes to RoomSubscription #2702

pixlwave opened this issue Oct 11, 2023 · 9 comments · Fixed by #3503
Labels
good first issue Good for newcomers

Comments

@pixlwave
Copy link
Member

pixlwave commented Oct 11, 2023

Now that matrix-org/sliding-sync#304 has been merged, there's a new parameter that can be included on RoomSubscription - include_heroes: Bool which will in turn include the heroes in the sync response.

@Hywan
Copy link
Member

Hywan commented Nov 2, 2023

Implementation inside Ruma is here, ruma/ruma#1691.

@Hywan
Copy link
Member

Hywan commented May 22, 2024

Implementation inside the SDK, #3441.

@Hywan
Copy link
Member

Hywan commented May 22, 2024

Another PR in Ruma, ruma/ruma#1818

@stefanceriu
Copy link
Member

Closing this ar per the above ^

@Hywan
Copy link
Member

Hywan commented Jun 3, 2024

Closed by #3461

@pixlwave
Copy link
Member Author

pixlwave commented Jun 3, 2024

Reopening this issue, as whilst looking at exposing these properties through the FFI, I noticed that the heroes stored in the SDK are missing their avatar URLs.

It would be great to have access to these as there are future designs where small rooms without an avatar_url will show a cluster of user avatars in Element X, and it would be really helpful to write the code with this in mind whilst fixing element-hq/element-x-ios#1659.

@pixlwave pixlwave reopened this Jun 3, 2024
@pixlwave
Copy link
Member Author

pixlwave commented Jun 3, 2024

Note: from what I can tell, this solution of storing an array of IDs and a separate array of display names would probably need to switch to storing a single array of heroes instead.

@bnjbvr
Copy link
Member

bnjbvr commented Jun 3, 2024

If by heroes you mean a tuple of user_id / optional user name / optional avatar URL, then yes, agreed with you; I just found out the proxy may return the avatar URLs there too. We need to not call this field heroes, though, to avoid conflicts with previous version of the data format where heroes existed. Are you willing to implement this?

@pixlwave
Copy link
Member Author

pixlwave commented Jun 4, 2024

Sure, I'm happy to give it a go whilst I'm here 👍

@bnjbvr bnjbvr closed this as completed in 304350f Jun 18, 2024
Johennes pushed a commit to Johennes/matrix-rust-sdk that referenced this issue Jun 24, 2024
This PR makes 2 changes:
- Updates the storage of room heroes to be a single array containing the user's complete profile.
- Exposes these to the FFI so that client apps can use these for avatar colours/clusters.

Closes matrix-org#2702 (again, now with avatars 🖼️)

---

* rooms: Store heroes as a complete user profile.

* ffi: Expose room heroes on Room and RoomInfo.

* chore: Remove TODO comment.

* Update crates/matrix-sdk-base/src/rooms/normal.rs

Signed-off-by: Benjamin Bouvier <[email protected]>

---------

Signed-off-by: Benjamin Bouvier <[email protected]>
Co-authored-by: Benjamin Bouvier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
4 participants