Skip to content

Allow adding an EventHandler to a specific room #177

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

Closed
jsparber opened this issue Mar 15, 2021 · 7 comments · Fixed by #902
Closed

Allow adding an EventHandler to a specific room #177

jsparber opened this issue Mar 15, 2021 · 7 comments · Fixed by #902
Assignees
Labels
help wanted Interested in working on the project? These are great additions we'd like to have!

Comments

@jsparber
Copy link
Contributor

jsparber commented Mar 15, 2021

Merge request #168 introduced a high-level API for the matrix-sdk which allows use to move the matrix-sdk-base::EventHandler to the matrix-sdk. In addition to that I would suggest moving the EventHandler to the room::Common since each event is emitted for a specific room. Additionally, I think we can add an EventHandler to the Client that emits events that happen outside of a room e.g. changes to the user's name and avatar.

@jsparber jsparber changed the title Move EventHandler to matrix-sdk Allow add EventHandler to a specific room Mar 18, 2021
@jsparber
Copy link
Contributor Author

The event handler was moved in #183.

@zecakeh
Copy link
Collaborator

zecakeh commented Mar 18, 2021

Maybe it's not the right place, but to continue our talk from the matrix room:

Alternatively, rooms could provide Subscribers from the sled store. If everything from the state is stored in sled, the sdk can just update the proper tree in the store and the subscribers will be notified of the change and can update as a result.

Of course this has more value if the timeline is stored in the store.

@jsparber
Copy link
Contributor Author

Maybe it's not the right place, but to continue our talk from the matrix room:

Alternatively, rooms could provide Subscribers from the sled store. If everything from the state is stored in sled, the sdk can just update the proper tree in the store and the subscribers will be notified of the change and can update as a result.

Of course this has more value if the timeline is stored in the store.

I think this is something interesting to explore.

@jsparber jsparber changed the title Allow add EventHandler to a specific room Allow adding an EventHandler to a specific room Mar 19, 2021
@poljar
Copy link
Contributor

poljar commented Mar 22, 2021

Alternatively, rooms could provide Subscribers from the sled store. If everything from the state is stored in sled, the sdk can just update the proper tree in the store and the subscribers will be notified of the change and can update as a result.

This is going to be hard to support without crippling the support for different store implementations. Our state store is a trait and us using sled is just an implementation detail.

@poljar
Copy link
Contributor

poljar commented Sep 9, 2021

Now that #309 is merged, this may have become more viable, though not sure that this is needed considering that you now get the room object in your callback. @jplatte any thoughts on this?

@jplatte
Copy link
Collaborator

jplatte commented Sep 9, 2021

Shouldn't be hard to support, so if there's a compelling use case why not 🙂

@jplatte
Copy link
Collaborator

jplatte commented Apr 8, 2022

Assigning to myself so I remember to write some instructions. My current thinking is that we should probably have room::Common::register_event_handler as the public interface. We'll have to decide how the handler context stuff will work, too.

@jplatte jplatte self-assigned this Apr 8, 2022
@gnunicorn gnunicorn added the help wanted Interested in working on the project? These are great additions we'd like to have! label Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Interested in working on the project? These are great additions we'd like to have!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants