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.

Sign up for GitHub

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

feat: improve member api for brb, update test-app with brb #4027

Merged
merged 3 commits into from
Dec 18, 2024
+133 −55
Merged

feat: improve member api for brb, update test-app with brb #4027

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
118936a
feat: add logic to sdk test app
Dec 13, 2024
53df0d0
fix: add fixes for PR
Dec 13, 2024
c84ed07
chore: update isBrb
Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions docs/samples/browser-plugin-meetings/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3052,6 +3052,11 @@ function moveFromDevice() {
});
}

function isUserSelf(member) {
const meeting = getCurrentMeeting();
return meeting.selfId === member.id
}

function claimPersonalMeetingRoom() {
console.log('DevicesControls#claimPersonalMeetingRoom()');

Expand Down Expand Up @@ -3094,7 +3099,7 @@ participantTable.addEventListener('click', (event) => {
}
const muteButton = document.getElementById('mute-participant-btn')
if (selectedParticipant.isAudioMuted) {
muteButton.innerText = meeting.selfId === selectedParticipant.id ? 'Unmute' : 'Request to unmute';
muteButton.innerText = isUserSelf(selectedParticipant) ? 'Unmute' : 'Request to unmute';
} else {
muteButton.innerText = 'Mute';
}
Expand Down Expand Up @@ -3334,15 +3339,17 @@ async function toggleBrb() {
const meeting = getCurrentMeeting();

if (meeting) {
const enabled = document.getElementById('brb').checked;
const brbButton = document.getElementById('brb-btn');
const isBrbEnabled = brbButton.innerText === 'Apply away';

szotrabh marked this conversation as resolved.
Show resolved Hide resolved
try {
const result = await meeting.beRightBack(enabled);
console.log(`meeting.beRightBack(${enabled}): success. Result: ${result}`);
const result = await meeting.beRightBack(isBrbEnabled);
console.log(`meeting.beRightBack(${isBrbEnabled}): success. Result: ${result}`);
} catch (error) {
console.error(`meeting.beRightBack({${enabled}): error: `, error);
console.error(`meeting.beRightBack({${isBrbEnabled}): error: `, error);
} finally {
localMedia?.microphoneStream?.setUserMuted(enabled);
localMedia?.cameraStream?.setUserMuted(enabled);
localMedia?.microphoneStream?.setUserMuted(isBrbEnabled);
localMedia?.cameraStream?.setUserMuted(isBrbEnabled);
}
}
}
Expand Down Expand Up @@ -3729,18 +3736,21 @@ function createMembersTable(members) {
const th3 = document.createElement('th');
const th4 = document.createElement('th');
const th5 = document.createElement('th');
const th6 = document.createElement('th');

th1.innerText = 'NAME';
th2.innerText = 'VIDEO';
th3.innerText = 'AUDIO';
th4.innerText = 'STATUS';
th5.innerText = 'SUPPORTS BREAKOUTS';
th6.innerText = 'AWAY';

tr.appendChild(th1);
tr.appendChild(th2);
tr.appendChild(th3);
tr.appendChild(th4);
tr.appendChild(th5);
tr.appendChild(th6);

return tr;
}
Expand All @@ -3752,6 +3762,7 @@ function createMembersTable(members) {
const td3 = document.createElement('td');
const td4 = document.createElement('td');
const td5 = document.createElement('td');
const td6 = document.createElement('td');
const label1 = createLabel(member.id);
const label2 = createLabel(member.id, member.isVideoMuted ? 'NO' : 'YES');
const label3 = createLabel(member.id, member.isAudioMuted ? 'NO' : 'YES');
Expand Down Expand Up @@ -3780,11 +3791,19 @@ function createMembersTable(members) {

td5.appendChild(label5);


if (isUserSelf(member) && member.isInMeeting) {
td6.appendChild(createButton(member.isBrb ? 'Remove away' : 'Apply away', toggleBrb, {id: 'brb-btn'}));
Copy link
Contributor

Choose a reason for hiding this comment

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

I would change the wording here to "Step Away" and "Back to Meeting". That just seems the most clear to me, and having it match the client UI makes it easy to understand that this is mapped to the step away feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brycetham I have tried this combination, but I don't like that "Back to Meeting" span on two lines and I can't do a style fix for that easily, I need to change the style for the table, not sure if this is worth that, but I'm open to discuss

Screenshot 2024-12-13 at 14 51 28

} else {
td6.appendChild(createLabel(member.id, member.isBrb ? 'YES' : 'NO'));
}

tr.appendChild(td1);
tr.appendChild(td2);
tr.appendChild(td3);
tr.appendChild(td4);
tr.appendChild(td5);
tr.appendChild(td6);

return tr;
}
Expand All @@ -3795,7 +3814,7 @@ function createMembersTable(members) {

thead.appendChild(createHeadRow());

Object.entries(members).forEach(([key, value]) => {
Object.entries(members).forEach(([_, value]) => {
edvujic marked this conversation as resolved.
Show resolved Hide resolved
if (value.status !== 'NOT_IN_MEETING') {
const row = createRow(value);

Expand Down
9 changes: 0 additions & 9 deletions docs/samples/browser-plugin-meetings/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,6 @@ <h2 class="collapsible">
<span id="ts-mute-audio-message" class="webex-warning"></span>
</div>
</div>

<div>
<fieldset>
<legend>Step Away</legend>
<input type="checkbox" id="brb" onclick="toggleBrb()">
<label for="brb">Enable Step Away</label><br>
</fieldset>
</div>

</div>
</fieldset>
</form>
Expand Down
6 changes: 3 additions & 3 deletions docs/samples/browser-plugin-meetings/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ button.btn-code {
}

.box {
max-width: 70rem;
max-width: 70rem;
margin-inline: auto;
margin-bottom: 1rem;
}
Expand Down Expand Up @@ -93,7 +93,7 @@ button.btn-code {
display: grid;
grid-template-columns: 1fr 1fr 1fr;
/* border: 10px solid black */
}
}

.video-section {
display: flex;
Expand Down Expand Up @@ -445,4 +445,4 @@ legend {
background-color:lightgrey;
border-radius:5px;
padding: 0 2px;
}
}
9 changes: 9 additions & 0 deletions packages/@webex/plugin-meetings/src/member/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default class Member {
isRecording: any;
isRemovable: any;
isSelf: any;
isBrb: boolean;
isUser: any;
isVideoMuted: any;
roles: IExternalRoles;
Expand Down Expand Up @@ -227,6 +228,13 @@ export default class Member {
* @memberof Member
*/
this.isRemovable = null;
/**
* @instance
* @type {Boolean}
* @public
* @memberof Member
*/
this.isBrb = false;
/**
* @instance
* @type {String}
Expand Down Expand Up @@ -295,6 +303,7 @@ export default class Member {
this.supportsInterpretation = MemberUtil.isInterpretationSupported(participant);
this.supportLiveAnnotation = MemberUtil.isLiveAnnotationSupported(participant);
this.isGuest = MemberUtil.isGuest(participant);
this.isBrb = MemberUtil.isBrb(participant);
this.isUser = MemberUtil.isUser(participant);
this.isDevice = MemberUtil.isDevice(participant);
this.isModerator = MemberUtil.isModerator(participant);
Expand Down
8 changes: 8 additions & 0 deletions packages/@webex/plugin-meetings/src/member/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ export type ParticipantWithRoles = {
};
};

export type ParticipantWithBrb = {
controls?: {
brb?: {
enabled: boolean;
};
};
};
Comment on lines +26 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just looking at ParticipantWithRoles... in that type, neither controls nor role is optional, so I just want to double check that we do want controls and brb to both be optional here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brycetham good point. I believe this type is wrong, because I reused this function getControlsRoles logic and it contains optional chaining inside, so I assume it can be undefined but the type doesn't show that. For example, when this brb feature doesn't work I believe it can be optional. In most cases in code logic when they deal with controls logic we have undefined checks.
Screenshot 2024-12-13 at 14 53 44

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brycetham I couldn't find information in the locus dto documents, so I will remove the optional parameter since none of my test cases require it to be optional.


// values are inherited from locus so don't update these
export enum MediaStatus {
RECVONLY = 'RECVONLY', // participant only receiving and not sending
Expand Down
Loading
Loading