Show error screens in group calls #29254
Conversation
| Disconnected = "disconnected", | ||
|
|
||
| Connecting = "connecting", | ||
| Connected = "connected", | ||
| Disconnecting = "disconnecting", |
There was a problem hiding this comment.
why do we drop the connecting state but not the disconnecting?
I think it still could be useful to display the connecting state in the room list tile.
Is there any way we can compute this indirectly?
But I see, that we can still tell if its connected or not by using the presented. (which is the more important state)
There was a problem hiding this comment.
It would be nice to have a 'connecting' state in the room list but, in practice it's become completely invisible ever since we got rid of Element Web's built-in lobby component. 'Disconnecting' on the other hand is still used to keep the toWidget and fromWidget 'hangup' actions from conflicting with one another.
There was a problem hiding this comment.
I would reintroduce the 'connecting' state in a future task, maybe when we're implementing the new DM call designs. It would need even more changes from EC presumably so I'm not inclined to squeeze it in here.
There was a problem hiding this comment.
This is not really a review since its still in draft. It is mostly that I wanted to get myself familiar with the general solution. It looks really good to me. Addresses a lot of things that were wrong about the EW call logic.
(Probably the comments are not that helpful since you would address the things anyways.)
| useTypedEventEmitter( | ||
| call, | ||
| CallEvent.Presentation, | ||
| useCallback( | ||
| (presented: boolean) => { | ||
| if (!presented && mounted.current) onClose(); | ||
| }, | ||
| [onClose], | ||
| ), | ||
| ); |
There was a problem hiding this comment.
I think this can also get comment phrasing out the logic and why just this is enough.
There was a problem hiding this comment.
It got even simpler now ('Close' action → onClose is fired)
src/models/Call.ts
Outdated
| private onTileLayout = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => { | ||
| ev.preventDefault(); | ||
| this.layout = Layout.Tile; | ||
| await this.messaging!.transport.reply(ev.detail, {}); // ack | ||
| this.messaging!.transport.reply(ev.detail, {}); // ack | ||
| }; | ||
|
|
||
| private onSpotlightLayout = async (ev: CustomEvent<IWidgetApiRequest>): Promise<void> => { | ||
| ev.preventDefault(); | ||
| this.layout = Layout.Spotlight; | ||
| await this.messaging!.transport.reply(ev.detail, {}); // ack | ||
| this.messaging!.transport.reply(ev.detail, {}); // ack | ||
| }; | ||
|
|
There was a problem hiding this comment.
Do you think this is the right place to start deprecating those actions?
It seems design and product have moved away from this on all platforms for some time.
(Or do we still use this.layout)
I think a first step could be to deprecate the this.layout field.
There was a problem hiding this comment.
Oh, I see now that the switch has been moved out of the header in https://www.figma.com/design/NKjLzSlbD8JnldYQ5sFpce/Element-Call-Updates-(New)---Q1-2025?node-id=243-16166&p=f&t=rwZYJ2V8QQ5kM0Rp-0. I can remove this stuff in a follow-up PR.
robintown
force-pushed
the
robin/call-errors
branch
from
dbc8d2e to
29145c6
Compare
robintown
force-pushed
the
robin/call-errors
branch
from
29145c6 to
1c21949
Compare
robintown
force-pushed
the
robin/call-errors
branch
from
1c21949 to
3d1dbcc
Compare
We often want calls to exist even when no more participants are left in the MatrixRTC session. So, we should avoid destroying calls as long as they're being presented in the UI; this means that the user has an intent to either join the call or continue looking at an error screen, and we shouldn't interrupt that interaction. The RoomViewStore is now what takes care of creating and destroying calls, rather than the CallView. In general it seems kinda impossible to safely create and destroy model objects from React lifecycle hooks, so moving this responsibility to a store seemed appropriate and resolves existing issues with calls in React strict mode.
This creates a distinction between the user hanging up and the widget being ready to close, which is useful for allowing Element Call to show error screens when disconnected from the call, for example.
robintown
force-pushed
the
robin/call-errors
branch
from
3d1dbcc to
569d525
Compare
This refactors the lifecycle management of the Call class a little bit to allow the UI to continue presenting a call as long as it wants, even if the MatrixRTC session has ended. This is primarily so we can keep the Element Call widget visible in cases where we've left the call but are still showing an error screen—but it's had other benefits, like fixing the infinite loop encountered in React's strict mode and avoiding a spurious reload of the lobby screen in certain cases.
Depends on element-hq/element-call#3011
Closes #29196
Closes element-hq/element-call#2955