Grid mode use sdk v3 #677

timmydoza · 2022-05-11T22:06:51Z

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

AHOYAPPS-0000

Description

A description of what this PR does.

Burndown

Before review

Updated CHANGELOG.md if necessary
Added unit tests if necessary
Updated affected documentation
Verified locally with npm test
Manually sanity tested running locally
Included screenshot as PR comment (if needed)
Ready for review

Before merge

Got one or more +1s
Re-tested if necessary

* Install storybook and add basic files * Get storybook working * Get grid view working * Hook up menu button * Fix some issues * Fix index.html caching issue * Update publication component and tests * Update Room tests * Update constants * Upgrade typescript * Update menu tests * Update TS things for new version * Rename state variable * Add test for ParticipantAudioTracks * Remove unused passcode stuff from twilio mock * Automatically disable conversations in storybook * Fix tests and linting issues * Add tests for GridView * Fix participant issue in stories * Add comment for ParticipantAudioTracks * Remove unnecessary code

* Install storybook and add basic files * Get storybook working * Get grid view working * Hook up menu button * Fix some issues * Fix index.html caching issue * Update publication component and tests * Update Room tests * Update constants * Upgrade typescript * Update menu tests * Update TS things for new version * Rename state variable * Add test for ParticipantAudioTracks * Remove unused passcode stuff from twilio mock * Automatically disable conversations in storybook * Fix tests and linting issues * Add tests for GridView * Install mui pagination component * Fix issue with storybook controls * Add pagination controls to state * Create usePagination hook * Add pagination to GridView component * Add setting for MaxGridParticipants * Remove unused constant and fix a few glitches * Add tests for usePagination * Update tests for GridView component * Fix linting issue * Only render Pagination component when there is more than one page * Display participant count in menu * Fix tests * remove console log * Fix linter errors * Fix test language * Fix issue with chat window rendering incorrectly

manjeshbhargav

Apart from the suggested changes, we also need to handle audio tracks being switched off by disabling the audio level indicator and changing the mic icon.

manjeshbhargav · 2022-05-12T04:02:01Z

src/hooks/useIsTrackEnabled/useIsTrackEnabled.tsx

@@ -7,16 +7,22 @@ export default function useIsTrackEnabled(track: TrackType) {
  const [isEnabled, setIsEnabled] = useState(track ? track.isEnabled : false);

  useEffect(() => {
-    setIsEnabled(track ? track.isEnabled : false);
+    // @ts-ignore
+    setIsEnabled(track?.mediaStreamTrack === null || track?.switchOffReason !== 'DisabledByPublisher');


Suggested change

setIsEnabled(track?.mediaStreamTrack === null || track?.switchOffReason !== 'DisabledByPublisher');

setIsEnabled(track?.switchOffReason !== Track.DISABLED_BY_PUBLISHER);

manjeshbhargav · 2022-05-12T04:02:31Z

src/hooks/useIsTrackEnabled/useIsTrackEnabled.tsx

-      track.on('disabled', setDisabled);
+      const handleSwitchOff = (_track: TrackType) => {
+        // @ts-ignore
+        if (_track.switchOffReason === 'DisabledByPublisher') {


Suggested change

if (_track.switchOffReason === 'DisabledByPublisher') {

if (_track.switchOffReason === Track.DISABLED_BY_PUBLISHER) {

timmydoza · 2022-05-12T15:08:51Z

Thanks @manjeshbhargav!

I've updated the branch to use the correct string for 'disabled-by-publisher'. I wasn't able to use the enum from Track.SwitchOffReason because it isn't accessible. I think we need to export the enum on line 8 here. You can see the effect of exporting the enum in this TypeScript Playground example.

Also, for this:

we also need to handle audio tracks being switched off by disabling the audio level indicator and changing the mic icon

I think this will already be handled by the changes to the useIsEnabled hook, as this is what determines when we show the muted icon. Unless, do you mean that we should show the muted icon for switchoff reasons other than 'disabled-by-publisher'?

manjeshbhargav · 2022-05-12T16:30:30Z

Thanks @manjeshbhargav!

I've updated the branch to use the correct string for 'disabled-by-publisher'. I wasn't able to use the enum from Track.SwitchOffReason because it isn't accessible. I think we need to export the enum on line 8 here. You can see the effect of exporting the enum in this TypeScript Playground example.

Also, for this:
we also need to handle audio tracks being switched off by disabling the audio level indicator and changing the mic icon
I think this will already be handled by the changes to the useIsEnabled hook, as this is what determines when we show the muted icon. Unless, do you mean that we should show the muted icon for switchoff reasons other than 'disabled-by-publisher'?

@timmydoza We need to handle other switch off reasons as well. Either we can re-use the mic disabled icon, or we can use another icon to help us differentiate between disabled and switched off.

manjeshbhargav

The changes look good. I think we still need to add UI notification for switched off audio tracks (apart from disabled by publisher).

manjeshbhargav · 2022-05-13T03:55:32Z

src/hooks/useIsTrackEnabled/useIsTrackEnabled.tsx

+      const handleSwitchOff = (_track: TrackType) => {
+        // @ts-ignore
+        if (_track.switchOffReason === 'disabled-by-publisher') {
+          setIsEnabled(false);
+        }
+      };
+      const handleSwitchOn = () => setIsEnabled(true);


Can't this be:

Suggested change

const handleSwitchOff = (_track: TrackType) => {

// @ts-ignore

if (_track.switchOffReason === 'disabled-by-publisher') {

setIsEnabled(false);

}

};

const handleSwitchOn = () => setIsEnabled(true);

const handleSwitchOff = (_track: TrackType) => {

// @ts-ignore

setIsEnabled(_track.switchOffReason !== 'disabled-by-publisher');

};

const handleSwitchOn = () => setIsEnabled(true);

* Add useLocalStorageState hook * Fix mocked localStorage class * Add useLocalStorageState hook to app to persist settings * Add comment to useLocalStorageState hook * Update formatting

* Fix linter error * Update code formatting

* Update node version in app.yaml * Fix bug with usePresentationParticipants

timmydoza added 7 commits March 8, 2022 13:16

Install twilio-video version 3 rc

3eb727c

Change VideoBandwidthProfileOptions to VideoBandwidthProfile

aa97f7e

Update useIsTrackEnabled to handle switchedOn and switchedOff events

fa66c75

Update useMediaStreamTrack to handle switchedOn and switchedOff events

cf8eaaf

Add optional chaining to fix error

2f5db14

timmydoza marked this pull request as draft May 11, 2022 22:07

manjeshbhargav requested changes May 12, 2022

View reviewed changes

Tim Mendoza added 2 commits May 12, 2022 09:00

Install new twilo-video RC

378d7e9

Use correct event name

c9aa011

manjeshbhargav reviewed May 13, 2022

View reviewed changes

Tim Mendoza and others added 16 commits May 16, 2022 23:01

Add backend functions that create large group rooms

87d26b8

Add local track support to useIsTrackEnabled

088861d

Reduce max grid participants to 4

c1d04db

Blur switched off videos

51116cd

Add useTrackSwitchOffReason hook

7823c13

Remove AudioLevelIndicator

f5486ed

Show avatar for unpublished video tracks

cbfbce3

Use MediaRegion us2 when creating rooms

048511b

Video 8472 add dominant speaker (#676)

e923527

Use twilio-video 3.0.0-rc4 and enable SDK logging

a053875

Video 8473 mobile grid mode (#681)

80f24e9

Video 10096 persist grid settings (#684)

e4e1ffd

* Add useLocalStorageState hook * Fix mocked localStorage class * Add useLocalStorageState hook to app to persist settings * Add comment to useLocalStorageState hook * Update formatting

Handle screenshare in grid mode (#683)

9dab5ba

* Fix linter error * Update code formatting

Merge branch 'feature/grid-mode' into grid-mode-use-sdk-v3

adb57c0

Use twilio-video 3.0.0 rc5

3c3ae0e

Add new settings for testing

abaccd5

Tim Mendoza and others added 16 commits June 6, 2022 23:50

Add some testing info to participant tile

0146bf5

Merge branch 'master' into feature/grid-mode

05ff339

Add updated room monitor tarball

0b91cbb

Finalize grid mode (#699)

b913d5e

Dogfood feedback (#705)

1f30620

Merge branch 'master' into feature/grid-mode

0ec7685

Merge branch 'feature/grid-mode' into grid-mode-use-sdk-v3

d096932

Grid mode bugfix (#707)

983e818

* Update node version in app.yaml * Fix bug with usePresentationParticipants

Add Chrome 103 fix for AudioLevelIndicator

10cfac9

Add fix for usePresentationParticipants

77e030e

Merge branch 'master' into feature/grid-mode

ec4770b

Video 10448 dogfood feedback (#716)

e4cd381

Pinned participant fix (#717)

00afa5e

Update SDK to RC6

43753f0

Turn off debug logging

9fcae2f

prep release (#719)

f251575

Base automatically changed from feature/grid-mode to master July 6, 2022 16:34

Tim Mendoza and others added 12 commits July 13, 2022 09:20

Update server to handle stage

f8e3c80

Merge branch 'master' into feature/grid-mode

e12d30e

Merge branch 'feature/grid-mode' into grid-mode-use-sdk-v3

8e62a10

Install rc7 sdk

261a151

Update to rc8

129d5bc

Update app to use large rooms branch from the pluing-rtc repo

9bc1b39

Only set maxSwitched on tracks when exlicitly passed as query parameters

df817ce

Add package-lock

f2f8ad0

upgrade twilio-video to preview

80eec7f

Use video sdk rc 10

3cf0024

Use kilobits for max bitrate values

e06f956

Update twilio video SDK RC version

f524fd7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grid mode use sdk v3 #677

Grid mode use sdk v3 #677

timmydoza commented May 11, 2022

manjeshbhargav left a comment

manjeshbhargav May 12, 2022

manjeshbhargav May 12, 2022

timmydoza commented May 12, 2022

manjeshbhargav commented May 12, 2022

manjeshbhargav left a comment

manjeshbhargav May 13, 2022

	setIsEnabled(track?.mediaStreamTrack === null \|\| track?.switchOffReason !== 'DisabledByPublisher');
	setIsEnabled(track?.switchOffReason !== Track.DISABLED_BY_PUBLISHER);

	if (_track.switchOffReason === 'DisabledByPublisher') {
	if (_track.switchOffReason === Track.DISABLED_BY_PUBLISHER) {

Grid mode use sdk v3 #677

Are you sure you want to change the base?

Grid mode use sdk v3 #677

Conversation

timmydoza commented May 11, 2022

Pull Request Details

JIRA link(s):

Description

Burndown

Before review

Before merge

manjeshbhargav left a comment

Choose a reason for hiding this comment

manjeshbhargav May 12, 2022

Choose a reason for hiding this comment

manjeshbhargav May 12, 2022

Choose a reason for hiding this comment

timmydoza commented May 12, 2022

manjeshbhargav commented May 12, 2022

manjeshbhargav left a comment

Choose a reason for hiding this comment

manjeshbhargav May 13, 2022

Choose a reason for hiding this comment