-
Notifications
You must be signed in to change notification settings - Fork 136
Unify LiveKit and Matrix connection states #3611
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.
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
Changes from 16 commits
2e646bf
88721be
7c40b0e
bf80136
7af89b4
0ebc607
ef2f53c
6efce23
1941fc9
667a3d0
b380532
8dac036
e626698
aabd760
170a38c
328cc71
08306d6
9a7e797
207b161
8225e4f
7edc97b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,7 +113,8 @@ async function registerUser( | |
| await page.getByRole("button", { name: "Register" }).click(); | ||
| const continueButton = page.getByRole("button", { name: "Continue" }); | ||
| try { | ||
| await expect(continueButton).toBeVisible({ timeout: 5000 }); | ||
| await expect(continueButton).toBeVisible({ timeout: 700 }); | ||
| // why do we need to put in the passwor if there is a continue button? | ||
| await page | ||
| .getByRole("textbox", { name: "Password", exact: true }) | ||
| .fill(PASSWORD); | ||
|
|
@@ -124,6 +125,16 @@ async function registerUser( | |
| await expect( | ||
| page.getByRole("heading", { name: `Welcome ${username}` }), | ||
| ).toBeVisible(); | ||
|
|
||
| // Dismiss incompatible browser toast | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add more test to be sure that we really dismiss the popup we think we dismiss (check the title)? This is fixed by updating playwright, right? is it really needed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It helps us save time if we are slow with merging renovate and need to debug why our test fail. |
||
| const dismissButton = page.getByRole("button", { name: "Dismiss" }); | ||
| try { | ||
| await expect(dismissButton).toBeVisible({ timeout: 700 }); | ||
| await dismissButton.click(); | ||
| } catch { | ||
| // dismissButton not visible, continue as normal | ||
| } | ||
|
|
||
| await setDevToolElementCallDevUrl(page); | ||
|
|
||
| const clientHandle = await page.evaluateHandle(() => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,6 +127,7 @@ export const ActiveCall: FC<ActiveCallProps> = (props) => { | |
| const mediaDevices = useMediaDevices(); | ||
| const trackProcessorState$ = useTrackProcessorObservable$(); | ||
| useEffect(() => { | ||
| logger.info("START CALL VIEW SCOPE"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add some prefix to the log until we have proper prefixed logger?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a proper logger. |
||
| const scope = new ObservableScope(); | ||
| const reactionsReader = new ReactionsReader(scope, props.rtcSession); | ||
| const { autoLeaveWhenOthersLeft, waitForCallPickup, sendNotificationType } = | ||
|
|
@@ -151,7 +152,9 @@ export const ActiveCall: FC<ActiveCallProps> = (props) => { | |
| setVm(vm); | ||
|
|
||
| vm.leave$.pipe(scope.bind()).subscribe(props.onLeft); | ||
|
|
||
| return (): void => { | ||
| logger.info("END CALL VIEW SCOPE"); | ||
| scope.end(); | ||
| }; | ||
| }, [ | ||
|
|
@@ -270,7 +273,10 @@ export const InCallView: FC<InCallViewProps> = ({ | |
| const ringOverlay = useBehavior(vm.ringOverlay$); | ||
| const fatalCallError = useBehavior(vm.fatalError$); | ||
| // Stop the rendering and throw for the error boundary | ||
| if (fatalCallError) throw fatalCallError; | ||
| if (fatalCallError) { | ||
| logger.debug("fatalCallError stop rendering", fatalCallError); | ||
| throw fatalCallError; | ||
| } | ||
|
|
||
| // We need to set the proper timings on the animation based upon the sound length. | ||
| const ringDuration = pickupPhaseAudio?.soundDuration["waiting"] ?? 1; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be removed now? Looks like it was some old onboarding UI from web that is not needed anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool that makes sense. I was wondering in what cases it would be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove all
continueButtonrelated code.