Skip to content

Commit

Permalink
Start refactoring some business logic into view models
Browse files Browse the repository at this point in the history
As Element Call grows in complexity, it has become a pain point that our business logic remains so tightly coupled to the UI code. In particular, this has made testing difficult, and the complex semantics of React hooks are not a great match for arbitrary business logic. Here, I show the beginnings of what it would look like for us to adopt the MVVM pattern. I've created a CallViewModel and TileViewModel that expose their state to the UI as rxjs Observables, as well as a couple of helper functions for consuming view models in React code.

This should contain no user-visible changes, but we need to watch out for regressions particularly around focus switching and promotion of speakers, because this was the logic I chose to refactor first.
  • Loading branch information
robintown committed Dec 1, 2023
1 parent 445c7c4 commit 169ccd9
Show file tree
Hide file tree
Showing 23 changed files with 837 additions and 521 deletions.
9 changes: 9 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ module.exports = {
"jsx-a11y/media-has-caption": "off",
// We should use the js-sdk logger, never console directly.
"no-console": ["error"],
"no-restricted-imports": [
"error",
{
name: "@react-rxjs/core",
importNames: ["Subscribe", "RemoveSubscribe"],
message:
"These components are easy to misuse, please use the 'subscribe' component wrapper instead",
},
],
},
settings: {
react: {
Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
},
"dependencies": {
"@juggle/resize-observer": "^3.3.1",
"@livekit/components-core": "^0.7.0",
"@livekit/components-react": "^1.1.0",
"@matrix-org/olm": "https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz",
"@opentelemetry/api": "^1.4.0",
Expand All @@ -37,6 +38,7 @@
"@react-aria/tabs": "^3.1.0",
"@react-aria/tooltip": "^3.1.3",
"@react-aria/utils": "^3.10.0",
"@react-rxjs/core": "^0.10.7",
"@react-spring/web": "^9.4.4",
"@react-stately/collections": "^3.3.4",
"@react-stately/select": "^3.1.3",
Expand Down Expand Up @@ -70,6 +72,7 @@
"react-router-dom": "^5.2.0",
"react-use-clipboard": "^1.0.7",
"react-use-measure": "^2.1.1",
"rxjs": "^7.8.1",
"sdp-transform": "^2.14.1",
"tinyqueue": "^2.0.3",
"unique-names-generator": "^4.6.0",
Expand Down
4 changes: 0 additions & 4 deletions src/icons/Mic.svg

This file was deleted.

5 changes: 0 additions & 5 deletions src/icons/MicMuted.svg

This file was deleted.

5 changes: 0 additions & 5 deletions src/icons/MuteMic.svg

This file was deleted.

10 changes: 1 addition & 9 deletions src/livekit/MediaDevicesContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
} from "react";
import { createMediaDeviceObserver } from "@livekit/components-core";
import { Observable } from "rxjs";
import { logger } from "matrix-js-sdk/src/logger";

import {
isFirefox,
Expand Down Expand Up @@ -83,14 +82,7 @@ function useMediaDevice(
// Tragically, the only way to get device names out of LiveKit is to specify a
// kind, which then results in multiple permissions requests.
const deviceObserver = useMemo(
() =>
createMediaDeviceObserver(
kind,
() => {
logger.error("Error creating MediaDeviceObserver");
},
requestPermissions,
),
() => createMediaDeviceObserver(kind, requestPermissions),
[kind, requestPermissions],
);
const available = useObservableState(deviceObserver, []);
Expand Down
Loading

0 comments on commit 169ccd9

Please sign in to comment.