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

Real Time Text API update and tests #5580

Merged
merged 10 commits into from
Jan 28, 2025
Merged

Real Time Text API update and tests #5580

merged 10 commits into from
Jan 28, 2025

Conversation

carocao-msft
Copy link
Contributor

What

Caption Banner API update, and update the merged caption list to only store 50 entries. Also added tests for the sorting logic

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.

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

github-actions bot commented Jan 24, 2025

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

Lines Statements Functions Branches
Base 27937 / 44356
62.98%
27937 / 44356
62.98%
783 / 1424
54.98%
2345 / 3715
63.12%
Current 27942 / 44356
62.99%
27942 / 44356
62.99%
783 / 1424
54.98%
2358 / 3726
63.28%
Diff 5 / 0
0.01%
5 / 0
0.01%
0 / 0
0%
13 / 11
0.16%

Copy link
Contributor

github-actions bot commented Jan 24, 2025

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

Lines Statements Functions Branches
Base 56813 / 93591
60.7%
56813 / 93591
60.7%
1157 / 2670
43.33%
3433 / 5696
60.27%
Current 56806 / 93604
60.68%
56806 / 93604
60.68%
1158 / 2670
43.37%
3432 / 5700
60.21%
Diff -7 / 13
-0.02%
-7 / 13
-0.02%
1 / 0
0.04%
-1 / 4
-0.06%

Copy link
Contributor

@@ -227,7 +228,7 @@ export const CaptionsBanner = (props: CaptionsBannerProps): JSX.Element => {

/* @conditional-compile-remove(rtt) */
const mergedCaptions: (CaptionsInformation | RealTimeTextInformation)[] = useMemo(() => {
return [...combinedList, ...(realTimeTexts?.currentInProgress ?? []), realTimeTexts?.myInProgress] as (
return [...combinedList, ...(realTimeTexts?.currentInProgress ?? []), realTimeTexts?.myInProgress].slice(-50) as (
Copy link
Contributor

Choose a reason for hiding this comment

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

Does mean that inprogress captions counts towards the 50?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@@ -351,6 +345,13 @@ export const CaptionsBanner = (props: CaptionsBannerProps): JSX.Element => {
}
data-ui-id="captions-banner-inner"
>
{
/* @conditional-compile-remove(rtt) */ isRealTimeTextOn && (
<div style={{ paddingTop: '0.5rem' }}>
Copy link
Contributor

@edwardlee-msft edwardlee-msft Jan 27, 2025

Choose a reason for hiding this comment

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

Since this is a component, should we extract this style to a style?
Should this be a stack?

@carocao-msft carocao-msft enabled auto-merge (squash) January 28, 2025 00:43
Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Calling bundle size is not changed.

  • Current size: 12401100
  • Base size: 12401100
  • Diff size: 0

Copy link
Contributor

Chat bundle size is decreased✅.

  • Current size: 1783838
  • Base size: 1783839
  • Diff size: -1

Copy link
Contributor

CallWithChat bundle size is not changed.

  • Current size: 12401112
  • Base size: 12401112
  • Diff size: 0

Copy link
Contributor

@carocao-msft carocao-msft merged commit 288b9cd into main Jan 28, 2025
41 checks passed
@carocao-msft carocao-msft deleted the carocao/RTTAPI branch January 28, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants