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.

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

fix case when passcode was lost #4535

Merged
merged 7 commits into from Apr 29, 2024

Conversation

rzdor
Copy link
Contributor

@rzdor rzdor commented Apr 25, 2024

What

If I enter the passcode first, then enter the meetingId, the passcode isn't passed to the URL:
image

Why

How Tested

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@@ -245,10 +247,11 @@ export const HomeScreen = (props: HomeScreenProps): JSX.Element => {
label={'Passcode'}
placeholder={'Enter a meeting passcode'}
onChange={(_, newValue) => {
const meetingId = callLocator && 'meetingId' in callLocator ? callLocator.meetingId : '';
const localMeetingId = meetingId ? meetingId : '';
Copy link
Member

Choose a reason for hiding this comment

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

Is this line needed? meetingId can just be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just undefined check

Copy link
Member

Choose a reason for hiding this comment

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

Right - but why have it? You have the check again below when localMeetingId ? is called

@JamesBurnside JamesBurnside added the do not need changelog Changes that does not affect the published package in any way do not need changelog entry label Apr 25, 2024
Copy link
Contributor

github-actions bot commented Apr 25, 2024

@azure/communication-react jest test coverage for stable.

Lines Statements Functions Branches
Base 25357 / 39283
64.54%
25357 / 39283
64.54%
698 / 1230
56.74%
2006 / 3191
62.86%
Current 25328 / 39283
64.47%
25328 / 39283
64.47%
698 / 1230
56.74%
2014 / 3187
63.19%
Diff -29 / 0
-0.07%
-29 / 0
-0.07%
0 / 0
0%
8 / -4
0.33%

Copy link
Contributor

github-actions bot commented Apr 25, 2024

@azure/communication-react jest test coverage for beta.

Lines Statements Functions Branches
Base 49916 / 79342
62.91%
49916 / 79342
62.91%
1037 / 2301
45.06%
2933 / 4795
61.16%
Current 49918 / 79342
62.91%
49918 / 79342
62.91%
1037 / 2301
45.06%
2930 / 4793
61.13%
Diff 2 / 0
0%
2 / 0
0%
0 / 0
0%
-3 / -2
-0.03%

Copy link
Contributor

github-actions bot commented Apr 26, 2024

Calling bundle size is decreased✅.

  • Current size: 4880025
  • Base size: 4880056
  • Diff size: -31

Copy link
Contributor

github-actions bot commented Apr 26, 2024

Chat bundle size is decreased✅.

  • Current size: 2150423
  • Base size: 2150424
  • Diff size: -1

Copy link
Contributor

github-actions bot commented Apr 26, 2024

CallWithChat bundle size is decreased✅.

  • Current size: 6274620
  • Base size: 6274651
  • Diff size: -31

@rzdor rzdor enabled auto-merge (squash) April 29, 2024 17:57
@rzdor rzdor merged commit 5ad5e0b into main Apr 29, 2024
40 checks passed
@rzdor rzdor deleted the ruslanz/meetingid-passcode-lost-url-parameter branch April 29, 2024 18:17
mgamis-msft pushed a commit that referenced this pull request May 1, 2024
* fix case when passcode was lost in url based on input order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not need changelog Changes that does not affect the published package in any way do not need changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants