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

Jump to bottom

fix(Call): introduce intermediary phase before joining call #13329

Merged
merged 10 commits into from
Nov 8, 2024
Merged

fix(Call): introduce intermediary phase before joining call #13329

merged 10 commits into from
Nov 8, 2024

Conversation

DorraJaouad
Copy link
Contributor

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

Before

Recording.2024-09-17.160718.mp4

After

Recording.2024-09-17.160913.mp4

🏁 Checklist

@DorraJaouad DorraJaouad self-assigned this Sep 17, 2024
Antreesy
Antreesy reviewed Sep 18, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Works good in general, but I'd test it in some real calls before merging

src/store/participantsStore.js Show resolved Hide resolved
src/components/CallView/CallFailedDialog.vue Outdated Show resolved Hide resolved
src/services/callsService.js Outdated Show resolved Hide resolved
@Antreesy
Copy link
Contributor

Antreesy commented Sep 24, 2024 •
edited
Loading

  • NcLoadingIcon isn't shown
  • CallButon keeps disabled when leave a call, unless reload

@DorraJaouad
Copy link
Contributor Author

  • NcLoadingIcon isn't shown
  • CallButon keeps disabled when leave a call, unless reload

It was because isConnecting state is reinitiated at the time where it should be truthy

Antreesy
Antreesy approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Tested, looks and works fine

Let's wait for @danxuliu opinion on that

src/components/CallView/CallFailedDialog.vue Outdated Show resolved Hide resolved
src/components/CallView/CallFailedDialog.vue Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the fix/call-joining branch 3 times, most recently from 00f74c3 to 855da4f Compare November 2, 2024 09:13
Antreesy
Antreesy reviewed Nov 4, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Some thoughts from me

src/services/callsService.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/components/TopBar/CallButton.vue Outdated Show resolved Hide resolved
src/components/TopBar/CallButton.vue Outdated Show resolved Hide resolved
src/components/TopBar/CallButton.vue Outdated Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Show resolved Hide resolved
src/utils/signaling.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Show resolved Hide resolved
Antreesy
Antreesy reviewed Nov 7, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The rest seems fine. Some more tests?

src/store/participantsStore.js Show resolved Hide resolved
src/services/callsService.js Outdated Show resolved Hide resolved
src/store/participantsStore.spec.js Show resolved Hide resolved
src/store/participantsStore.js Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
Antreesy
Antreesy approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Acceptable

src/store/participantsStore.spec.js Outdated Show resolved Hide resolved
src/store/participantsStore.spec.js Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Outdated Show resolved Hide resolved
src/store/participantsStore.js Show resolved Hide resolved
@DorraJaouad
fix(ParticipantsStore): remove unnecessary event listeners and timeou…
2e74e61 
…t. It's only received when using internal server. Replace them with events from the base class so we make sure it's always received. Also, add a new event for failed joining

Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
Fix(ParticipantsStore): separate joining states from each others (isI…
609c1b8 
…nCall, connecting, connection failed)

Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
feat(Call): Stop switching to CallView without a successful join. Int…
cc3675d 
…roduce a failed join message dialog.

Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix(Call): stop all media related streams (simulating end call)
063755b 
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix: distinguish the 2 phases of joining the call: connect to the cal…
37a9ef2 
…l AND wait for participants list

Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix: move failed call dialog to root
4511a20 
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix: refactoring
073bf8d 
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix: add loading icon for leave call button
e81854a 
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix: pass actual flags
afd1f58 
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad
fix: add tests
dba5c4b 
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad DorraJaouad merged commit 4b0f62f into main Nov 8, 2024
47 checks passed
@DorraJaouad DorraJaouad deleted the fix/call-joining branch November 8, 2024 11:03
@DorraJaouad
Copy link
Contributor Author

/backport to stable30

@backportbot backportbot bot mentioned this pull request Nov 8, 2024
Merged
2 tasks
@DorraJaouad DorraJaouad mentioned this pull request Nov 18, 2024
Merged
9 tasks
@Antreesy Antreesy mentioned this pull request Dec 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antreesy Antreesy Antreesy approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@danxuliu danxuliu Awaiting requested review from danxuliu

Assignees

@DorraJaouad DorraJaouad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DorraJaouad @Antreesy @danxuliu