Skip to content

[Chat] Announce File Attachments in New Messages#4438

Merged
jpeng-ms merged 6 commits intomainfrom
john/a11y-bug
Apr 9, 2024
Merged

[Chat] Announce File Attachments in New Messages#4438
jpeng-ms merged 6 commits intomainfrom
john/a11y-bug

Conversation

@jpeng-ms
Copy link
Copy Markdown
Member

@jpeng-ms jpeng-ms commented Apr 9, 2024

What

Fixed the issue where live messages don't announce file attachments

Why

A11Y bug fix

How Tested

there are 3 scenarios:

  • message with attachments only and no content
  • message with attachments and text content
  • message with attachments and image content

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

CallWithChat bundle size is increased❗.

  • Current size: 6210697
  • Base size: 6210439
  • Diff size: 258

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

Calling bundle size is increased❗.

  • Current size: 4813466
  • Base size: 4813465
  • Diff size: 1

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

Chat bundle size is increased❗.

  • Current size: 2131268
  • Base size: 2131010
  • Diff size: 258

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

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

Lines Statements Functions Branches
Base 25187 / 39111
64.39%
25187 / 39111
64.39%
675 / 1215
55.55%
1957 / 3115
62.82%
Current 25189 / 39118
64.39%
25189 / 39118
64.39%
675 / 1215
55.55%
1956 / 3113
62.83%
Diff 2 / 7
0%
2 / 7
0%
0 / 0
0%
-1 / -2
0.01%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

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

Lines Statements Functions Branches
Base 49639 / 79165
62.7%
49639 / 79165
62.7%
1018 / 2266
44.92%
2881 / 4711
61.15%
Current 49617 / 79203
62.64%
49617 / 79203
62.64%
1019 / 2268
44.92%
2852 / 4696
60.73%
Diff -22 / 38
-0.06%
-22 / 38
-0.06%
1 / 2
0%
-29 / -15
-0.42%

Comment thread packages/react-components/src/components/ChatMessage/ChatMessageContent.tsx Outdated
if (props.message.content || attachments) {
// Replace all <img> tags with 'image' for aria.
const parsedContent = DOMPurify.sanitize(props.message.content, {
const parsedContent = DOMPurify.sanitize(props.message.content ?? '', {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

because line 170

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like sanitize can take undefined?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it doesn't, it takes string | Node

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

Comment thread packages/react-components/src/components/ChatMessage/ChatMessageContent.tsx Outdated
Comment thread packages/react-components/src/components/ChatMessage/ChatMessageContent.tsx Outdated
/* @conditional-compile-remove(mention) */
import { defaultOnMentionRender } from './MentionRenderer';
import DOMPurify from 'dompurify';
import { _AttachmentDownloadCardsStrings } from '../AttachmentDownloadCards';
Copy link
Copy Markdown
Member

@Leah-Xia-Microsoft Leah-Xia-Microsoft Apr 9, 2024

Choose a reason for hiding this comment

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

Not necessarily related to this PR but for @JoshuaLai to confirm, do we still need the type of _AttachmentDownloadCardsStrings and the function of useLocaleStringsTrampoline?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

@jpeng-ms jpeng-ms enabled auto-merge (squash) April 9, 2024 23:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2024

@jpeng-ms jpeng-ms merged commit 76a858a into main Apr 9, 2024
@jpeng-ms jpeng-ms deleted the john/a11y-bug branch April 9, 2024 23:57
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.

4 participants