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

Update Calling Hero to the Latest version #269

Merged
merged 6 commits into from
Jan 17, 2025
Merged

Conversation

carocao-msft
Copy link
Contributor

@carocao-msft carocao-msft commented Jan 17, 2025

Purpose

Update Calling Hero to the Latest version 1.23.0

  • ...

Pull Request Type

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Upstream sample reference

Links to PR(s) that made the original change in the upstream sample:

  • ...

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

@@ -19,3 +19,4 @@ createRoot(domNode).render(
</SwitchableFluentThemeProvider>
</React.StrictMode>
);
export {}; /*The above line is generated by conditional compilation, when no export detected after CC.*/
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove please

<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<meta name="description" content="Azure Communication Services - UI Library: Calling Sample app" />
<title>UI Library Calling Sample</title>

Copy link
Member

Choose a reason for hiding this comment

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

please remove this as its for render tracker a debugging tool we don't provide in the hero

@@ -93,6 +93,7 @@ const App = (): JSX.Element => {
getMeetingIdFromUrl() ||
getGroupIdFromUrl() ||
createGroupId();

Copy link
Member

Choose a reason for hiding this comment

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

please remove the new lines generated by CC code

@@ -27,3 +27,4 @@ describe('ContosoUtils tests', () => {
test('createRandomDisplayName should return a valid string for a user id', () =>
expect(createRandomDisplayName()).toBeTruthy());
});
export {}; /*The above line is generated by conditional compilation, when no export detected after CC.*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line as well

@@ -14,7 +14,7 @@ import { v1 as generateGUID } from 'uuid';
export const fetchTokenResponse = async (): Promise<any> => {
const response = await fetch('token?scope=voip');
if (response.ok) {
const responseAsJson = await response.json();
const responseAsJson = await response.json(); //(await response.json())?.value?.token;
Copy link
Contributor

Choose a reason for hiding this comment

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

Also remove the commented out code here

@carocao-msft carocao-msft merged commit e3fef3a into main Jan 17, 2025
3 checks passed
@carocao-msft carocao-msft deleted the carocao/update branch January 17, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants