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

Distinguish room state and timeline events when dealing with widgets #28681

Merged
merged 9 commits into from
Jan 22, 2025

Conversation

robintown
Copy link
Member

@robintown robintown commented Dec 6, 2024

This change enables widgets to take advantage of the more reliable method of communicating room state over the widget API provided by this update to MSC2762.

Depends on matrix-org/matrix-widget-api#116

src/stores/widgets/StopGapWidget.ts Show resolved Hide resolved
src/stores/widgets/StopGapWidget.ts Show resolved Hide resolved
@@ -285,6 +291,17 @@ export class StopGapWidget extends EventEmitter {
this.messaging.on("capabilitiesNotified", () => this.emit("capabilitiesNotified"));
this.messaging.on(`action:${WidgetApiFromWidgetAction.OpenModalWidget}`, this.onOpenModal);

// When widgets are listening to events, we need to make sure they're only
// receiving events for the right room
if (this.roomId === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just read through and worked out what's going on here: seems a bit excessive to send the entire room state through to an account widget whenever you switch rooms? I don't see any capability negotiation going on here so seems like this will happen whether the widget actually wants it or not. I guess it is what the spec technically implies but is anything actually going to use it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct that matrix-widget-api is set up to re-sync the room state whenever an account widget encounters a new room. However, there is capability negotiation, and it will only read+push those state entries that match the widget's capabilities: https://github.com/matrix-org/matrix-widget-api/blob/61c45f78e09cafd3094c4a73892bd34be803bd98/src/ClientWidgetApi.ts#L1104

Is that already more along the lines of what you were expecting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I missed that it was done at the event type level, so that makes more sense.

Copy link
Member

@florianduros florianduros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine! A little more documentation would be great!

src/stores/widgets/StopGapWidgetDriver.ts Outdated Show resolved Hide resolved
src/stores/widgets/StopGapWidgetDriver.ts Show resolved Hide resolved
src/stores/widgets/StopGapWidgetDriver.ts Show resolved Hide resolved
@fkwp fkwp self-assigned this Jan 21, 2025
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.

6 participants