Skip to content

Conversation

@robintown
Copy link
Member

@robintown robintown commented Mar 13, 2025

Previously we were watching for changes to the room state to know when you become connected to a call. However, the room state might not change if you had a stuck membership event prior to re-joining the call. It's going to be more reliable to watch for the 'join' action that Element Call sends, and use that to track the connection state.

Sonar is complaining about code duplication here, but I petition that we bypass the warning on this PR. The code in question describes the widget actions used by the Jitsi and Element Call widgets for joining/leaving/disconnecting from calls, and I see it as mostly being a coincidence that Jitsi and Element Call now have similar widget action APIs. I would like for the superclass Call to continue to be something that you could implement for any arbitrary widget, so I think this is a case where a little duplication is better than the wrong abstraction.

Closes #29429

Notes: Fix calls displaying the wrong connection state or not triggering a notification

Previously we were watching for changes to the room state to know when you become connected to a call. However, the room state might not change if you had a stuck membership event prior to re-joining the call. It's going to be more reliable to watch for the 'join' action that Element Call sends, and use that to track the connection state.
@robintown robintown force-pushed the robin/call-cleanup branch from 244c80c to c6a1784 Compare August 7, 2025 11:05
@t3chguy
Copy link
Member

t3chguy commented Aug 18, 2025

Bypassing the sonar warning would mean a force merge which

  1. bypasses a bunch of merge-queue only tests
  2. bypasses all downstream tests

This is to be avoided.

@robintown
Copy link
Member Author

Right, that's what force merging entails… do you agree with my assessment that the Sonar tool is pushing me to do the wrong thing? Or would you rather I introduce a new class between Call and JitsiCall/ElementCall in the hierarchy to consolidate the similar widget communications code that they have?

@t3chguy
Copy link
Member

t3chguy commented Aug 20, 2025

do you agree with my assessment that the Sonar tool is pushing me to do the wrong thing?

I have no opinion on that, might be worth bringing up at the Web Weekly call for discussion

renderRoomTile();
screen.getByText("Video");
await act(() => call.start());
act(() => call.setConnectionState(ConnectionState.Connected));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
act(() => call.setConnectionState(ConnectionState.Connected));
await act(() => call.setConnectionState(ConnectionState.Connected));

Copy link
Member Author

Choose a reason for hiding this comment

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

When I add this my IDE warns me "'await' has no effect on the type of this expression". And looking at the type definitions I see:

    // While act does always return Thenable, if a void function is passed, we pretend the return value is also void to not trigger dangling Promise lint rules.
    export function act(callback: () => VoidOrUndefinedOnly): void;
    export function act<T>(callback: () => T | Promise<T>): Promise<T>;

So it seems to me that the React team is encouraging people to not insert await when the operation is synchronous.

Copy link
Member

Choose a reason for hiding this comment

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

Looking at https://react.dev/reference/react/act#await-act-async-actfn, I get the impression that they expect you to make the callback async even if you don't await anything.

);
defaultDispatcher.unregister(dispatcherRef);
await act(() => call.start());
act(() => call.setConnectionState(ConnectionState.Connected));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
act(() => call.setConnectionState(ConnectionState.Connected));
await act(() => call.setConnectionState(ConnectionState.Connected));

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

@dbkr
Copy link
Member

dbkr commented Aug 27, 2025

Sounds like we agreed to skip sonar for this one: bypassing.

@dbkr dbkr merged commit 4b4cb89 into develop Aug 27, 2025
103 of 111 checks passed
@dbkr dbkr deleted the robin/call-cleanup branch August 27, 2025 08:04
Dileep9999 pushed a commit to hemanth-nag/element-web that referenced this pull request Oct 8, 2025
…-hq#29492)

Previously we were watching for changes to the room state to know when you become connected to a call. However, the room state might not change if you had a stuck membership event prior to re-joining the call. It's going to be more reliable to watch for the 'join' action that Element Call sends, and use that to track the connection state.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 25, 2025
Changes in [1.12.2](https://github.com/element-hq/element-web/releases/tag/v1.12.2) (2025-10-21)
================================================================================================

* Room List: Extend the viewport to avoid so many black spots when scrolling the room list ([#30867](element-hq/element-web#30867)). Contributed by @langleyd.
* Hide calling buttons in room header before a room is created ([#30816](element-hq/element-web#30816)). Contributed by @Half-Shot.
* Improve invite dialog ui - Part 2 ([#30836](element-hq/element-web#30836)). Contributed by @florianduros.

* Fix platform settings race condition and make auto-launch tri-state ([#30977](element-hq/element-web#30977)). Contributed by @t3chguy.
* Fix: member count in header and member list ([#30982](element-hq/element-web#30982)). Contributed by @florianduros.
* Fix duration of voice message in timeline ([#30973](element-hq/element-web#30973)). Contributed by @florianduros.
* Fix voice notes rendering at 00:00 when playback had not begun. ([#30961](element-hq/element-web#30961)). Contributed by @Half-Shot.
* Improve handling of animated images, add support for AVIF animations ([#30932](element-hq/element-web#30932)). Contributed by @t3chguy.
* Update key storage toggle when key storage status changes ([#30934](element-hq/element-web#30934)). Contributed by @uhoreg.
* Fix jitsi widget popout ([#30908](element-hq/element-web#30908)). Contributed by @dbkr.
* Improve keyboard navigation on invite dialog ([#30930](element-hq/element-web#30930)). Contributed by @florianduros.
* Prefer UIA flows with supported UIA stages ([#30926](element-hq/element-web#30926)). Contributed by @richvdh.
* Enhance accessibility of dropdown ([#30928](element-hq/element-web#30928)). Contributed by @florianduros.
* Improve accessibility of the `\<AvatarSetting> component ([#30907](element-hq/element-web#30907)). Contributed by @MidhunSureshR.

Changes in [1.12.1](https://github.com/element-hq/element-web/releases/tag/v1.12.1) (2025-10-07)
================================================================================================

* New Room List: Change the order of filters to match those on mobile ([#30905](element-hq/element-web#30905)). Contributed by @langleyd.
* New Room List: Don't clear filters on space change ([#30903](element-hq/element-web#30903)). Contributed by @langleyd.
* Add release announcement for the sounds ([#30900](element-hq/element-web#30900)). Contributed by @langleyd.
* Rich Text Editor: Add emoji suggestion support ([#30873](element-hq/element-web#30873)). Contributed by @langleyd.
* feat: Disable session lock when running in element-desktop ([#30643](element-hq/element-web#30643)). Contributed by @kaylendog.
* Improve invite dialog ui - Part 1 ([#30764](element-hq/element-web#30764)). Contributed by @florianduros.
* Update Message Sound for Element ([#30804](element-hq/element-web#30804)). Contributed by @beatdemon.
* Add new and improved ringtone ([#30761](element-hq/element-web#30761)). Contributed by @Half-Shot.
* Disable RTE formatting buttons when the content contains a slash command ([#30802](element-hq/element-web#30802)). Contributed by @langleyd.

* New Room List: Improve robustness of keyboard navigation ([#30888](element-hq/element-web#30888)). Contributed by @langleyd.
* Fix a11y issue on list in invite dialog ([#30878](element-hq/element-web#30878)). Contributed by @florianduros.
* Switch Export and Import Icons to match intuition ([#30805](element-hq/element-web#30805)). Contributed by @micartey.
* Hide breadcrumb option when new room list is enabled ([#30869](element-hq/element-web#30869)). Contributed by @florianduros.
* Avoid creating multiple call objects for the same widget ([#30839](element-hq/element-web#30839)). Contributed by @robintown.
* Add a test for #29882, which is fixed by matrix-org/matrix-js-sdk#5016 ([#30835](element-hq/element-web#30835)). Contributed by @andybalaam.
* fix: use `help_encryption_url` of config instead of hardcoded `https://element.io/help#encryption5` ([#30746](element-hq/element-web#30746)). Contributed by @florianduros.
* Fix html export when feature\_jump\_to\_date is enabled ([#30828](element-hq/element-web#30828)). Contributed by @langleyd.
* Fix #30439: "Forgot recovery key" should go to "reset" ([#30771](element-hq/element-web#30771)). Contributed by @andybalaam.

Changes in [1.12.0](https://github.com/element-hq/element-web/releases/tag/v1.12.0) (2025-09-23)
================================================================================================

* Remove remaining support for outdated .well-known settings ([#30702](element-hq/element-web#30702)). Contributed by @richvdh.

* Add decline button to call notification toast (use new notification event) ([#30729](element-hq/element-web#30729)). Contributed by @toger5.
* Use the new room list by default ([#30640](element-hq/element-web#30640)). Contributed by @langleyd.
* "Verify this device" redesign ([#30596](element-hq/element-web#30596)). Contributed by @uhoreg.
* Set Element Call "intents" when starting and answering DM calls. ([#30730](element-hq/element-web#30730)). Contributed by @Half-Shot.
* Add axe compliance for new room list ([#30700](element-hq/element-web#30700)). Contributed by @langleyd.
* Stop ringing and remove toast if another device answers a RTC call. ([#30728](element-hq/element-web#30728)). Contributed by @Half-Shot.
* Automatically adjust history visibility when making a room private ([#30713](element-hq/element-web#30713)). Contributed by @Half-Shot.
* Release announcement for new room list ([#30675](element-hq/element-web#30675)). Contributed by @dbkr.

* [Backport staging] Room list: make the filter resize correctly ([#30795](element-hq/element-web#30795)). Contributed by @RiotRobot.
* [Backport staging] Avoid flicker of the room list filter on resize ([#30794](element-hq/element-web#30794)). Contributed by @RiotRobot.
* Don't show release announcements while toasts are displayed ([#30770](element-hq/element-web#30770)). Contributed by @dbkr.
* Fix enabling key backup not working if there is an untrusted key backup ([#30707](element-hq/element-web#30707)). Contributed by @Half-Shot.
* Force `preload` to be false when setting an intent on an Element Call. ([#30759](element-hq/element-web#30759)). Contributed by @Half-Shot.
* Fix handling of 413 server response when uploading media ([#30737](element-hq/element-web#30737)). Contributed by @hughns.
* Make landmark navigation work with new room list ([#30747](element-hq/element-web#30747)). Contributed by @dbkr.
* Prevent voice message from displaying spurious errors ([#30736](element-hq/element-web#30736)). Contributed by @florianduros.
* Align default avatar and fix colors in composer pills ([#30739](element-hq/element-web#30739)). Contributed by @florianduros.
* Use configured URL for link to desktop app in message search settings ([#30742](element-hq/element-web#30742)). Contributed by @t3chguy.
* Fix history visibility when creating space rooms ([#30745](element-hq/element-web#30745)). Contributed by @dbkr.
* Check HTML-encoded quotes when handling translations for embedded pages (such as welcome.html) ([#30743](element-hq/element-web#30743)). Contributed by @Half-Shot.
* Fix local room encryption status always not enabled ([#30461](element-hq/element-web#30461)). Contributed by @BillCarsonFr.
* fix: make url in topic in room intro clickable ([#30686](element-hq/element-web#30686)). Contributed by @florianduros.
* Block change recovery key button while a change is ongoing. ([#30664](element-hq/element-web#30664)). Contributed by @Half-Shot.
* Hide advanced settings during room creation when `UIFeature.advancedSettings=false` ([#30684](element-hq/element-web#30684)). Contributed by @florianduros.
* A11y: improve accessibility of pinned messages ([#30558](element-hq/element-web#30558)). Contributed by @florianduros.

Changes in [1.11.112](https://github.com/element-hq/element-web/releases/tag/v1.11.112) (2025-09-16)
====================================================================================================
Fix [CVE-2025-59161](https://www.cve.org/CVERecord?id=CVE-2025-59161) / [GHSA-m6c8-98f4-75rr](GHSA-m6c8-98f4-75rr)

Changes in [1.11.111](https://github.com/element-hq/element-web/releases/tag/v1.11.111) (2025-09-10)
====================================================================================================

* Do not hide media from your own user by default ([#29797](element-hq/element-web#29797)). Contributed by @Half-Shot.
* Remember whether sidebar is shown for calls when switching rooms ([#30262](element-hq/element-web#30262)). Contributed by @bojidar-bg.
* Open the proper integration settings on integrations disabled error ([#30538](element-hq/element-web#30538)). Contributed by @Half-Shot.
* Show a "progress" dialog while invites are being sent ([#30561](element-hq/element-web#30561)). Contributed by @richvdh.
* Move the room list to the new ListView(backed by react-virtuoso)  ([#30515](element-hq/element-web#30515)). Contributed by @langleyd.

* [Backport staging] Ensure container starts if it is mounted with an empty /modules directory. ([#30705](element-hq/element-web#30705)). Contributed by @RiotRobot.
* Fix room joining over federation not specifying vias or using aliases ([#30641](element-hq/element-web#30641)). Contributed by @t3chguy.
* Fix stable-suffixed MSC4133 support ([#30649](element-hq/element-web#30649)). Contributed by @dbkr.
* Fix i18n of message when a setting is disabled ([#30646](element-hq/element-web#30646)). Contributed by @dbkr.
* ListView should not handle the arrow keys if there is a modifier applied ([#30633](element-hq/element-web#30633)). Contributed by @langleyd.
* Make BaseDialog's div keyboard focusable and fix test. ([#30631](element-hq/element-web#30631)). Contributed by @langleyd.
* Fix: Allow triple-click text selection to flow around pills ([#30349](element-hq/element-web#30349)). Contributed by @AlirezaMrtz.
* Watch for a 'join' action to know when the call is connected ([#29492](element-hq/element-web#29492)). Contributed by @robintown.
* Fix: add missing tooltip and aria-label to lock icon next to composer ([#30623](element-hq/element-web#30623)). Contributed by @florianduros.
* Don't render context menu when scrolling ([#30613](element-hq/element-web#30613)). Contributed by @langleyd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Call state can't transition to 'connected' if your call membership already exists

6 participants