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.

Sign up for GitHub

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

Jump to bottom

Fix keyboard navigation for message menu button #5576

Merged
merged 23 commits into from
Jan 21, 2025
Merged

Fix keyboard navigation for message menu button #5576

merged 23 commits into from
Jan 21, 2025

Conversation

vhuseinova-msft
Copy link
Member

What

  • updated keyboard navigation for messages (only the inner fluent message component is accessible for keyboard navigation)
  • removed usage of FluentUI actions component for message (discussed with FluentUI team and they mentioned that it's not recommended to use it as it breaks keyboard navigation)
  • moved message actions menu implementation

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.

@github-actions GitHub Actions
Copy link
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions GitHub Actions
Copy link
Contributor

Storybook 8 URL https://60c7ae6891f0e90039d7cd54-slbdfiudwl.chromatic.com/

@github-actions GitHub Actions
Copy link
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions GitHub Actions
Copy link
Contributor

Storybook 8 URL https://60c7ae6891f0e90039d7cd54-fqxtwqayua.chromatic.com/

@github-actions GitHub Actions
Copy link
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions GitHub Actions
Copy link
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions GitHub Actions
Copy link
Contributor

Storybook 8 URL https://60c7ae6891f0e90039d7cd54-lrxsjhunxx.chromatic.com/

@github-actions GitHub Actions
Copy link
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 20, 2025 •
edited
Loading

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

Lines Statements Functions Branches
Base 27905 / 44311
62.97%		 27905 / 44311
62.97%		 782 / 1423
54.95%		 2316 / 3695
62.67%
Current 27886 / 44346
62.88%		 27886 / 44346
62.88%		 783 / 1424
54.98%		 2329 / 3699
62.96%
Diff -19 / 35
-0.09%		 -19 / 35
-0.09%		 1 / 1
0.03%		 13 / 4
0.29%

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 20, 2025 •
edited
Loading

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

Lines Statements Functions Branches
Base 56708 / 93467
60.67%		 56708 / 93467
60.67%		 1156 / 2669
43.31%		 3425 / 5692
60.17%
Current 56770 / 93531
60.69%		 56770 / 93531
60.69%		 1157 / 2670
43.33%		 3411 / 5686
59.98%
Diff 62 / 64
0.02%		 62 / 64
0.02%		 1 / 1
0.02%		 -14 / -6
-0.19%

@vhuseinova-msft vhuseinova-msft added the update_snapshots Set this label to request automated update of UI snapshots label Jan 21, 2025
@github-actions github-actions bot removed the update_snapshots Set this label to request automated update of UI snapshots label Jan 21, 2025
@vhuseinova-msft vhuseinova-msft added the update_snapshots Set this label to request automated update of UI snapshots label Jan 21, 2025
@vhuseinova-msft
Merge branch 'vhuseinova/message-menu-fix' of github.com:Azure/commun…
5d2a53e 
…ication-ui-library into vhuseinova/message-menu-fix
@vhuseinova-msft vhuseinova-msft added update_snapshots Set this label to request automated update of UI snapshots and removed update_snapshots Set this label to request automated update of UI snapshots labels Jan 21, 2025
@vhuseinova-msft vhuseinova-msft requested review from a team as code owners January 21, 2025 17:52
emlynmac
emlynmac reviewed Jan 21, 2025
View reviewed changes
packages/react-components/src/components/utils/ChatMessageComponentUtils.tsx
*
* @param node - The HTMLDivElement from which to remove the keyboard navigation styles. If null, the function does nothing.
*/
export const removeFluentUIKeyboardNavigationStyles = (node: HTMLDivElement | null): void => {
Copy link
Member

Choose a reason for hiding this comment

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

Curious; how stable is this going to be?
Do we have tests to cover the changes when fluent gets updated?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's removed every time when a body component ref is set or changed (see the usage).
Yeah, I was thinking about the testing too and one of the cases that I see is to test that the keyboard navigation is actually correct but not sure what else we can do here.

Please share suggestions if any :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added the keyboard navigation tests

@github-actions GitHub Actions
Copy link
Contributor

CallWithChat bundle size is not changed.

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

@github-actions GitHub Actions
Copy link
Contributor

Calling bundle size is not changed.

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

@github-actions GitHub Actions
Copy link
Contributor

Chat bundle size is increased❗.

  • Current size: 1783692
  • Base size: 1783440
  • Diff size: 252

@github-actions GitHub Actions
Copy link
Contributor

Storybook 8 URL https://60c7ae6891f0e90039d7cd54-vabqcgurux.chromatic.com/

@vhuseinova-msft vhuseinova-msft merged commit 765d226 into main Jan 21, 2025
41 checks passed
@vhuseinova-msft vhuseinova-msft deleted the vhuseinova/message-menu-fix branch January 21, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Leah-Xia-Microsoft Leah-Xia-Microsoft Leah-Xia-Microsoft approved these changes

@emlynmac emlynmac emlynmac approved these changes

@JamesBurnside JamesBurnside Awaiting requested review from JamesBurnside JamesBurnside is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@alkwa-msft alkwa-msft Awaiting requested review from alkwa-msft alkwa-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@dmceachernmsft dmceachernmsft Awaiting requested review from dmceachernmsft dmceachernmsft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@carocao-msft carocao-msft Awaiting requested review from carocao-msft carocao-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@edwardlee-msft edwardlee-msft Awaiting requested review from edwardlee-msft edwardlee-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@mgamis-msft mgamis-msft Awaiting requested review from mgamis-msft mgamis-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-2

@prabhjot-msft prabhjot-msft Awaiting requested review from prabhjot-msft prabhjot-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-2

@jimchou-dev jimchou-dev Awaiting requested review from jimchou-dev jimchou-dev is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@jpeng-ms jpeng-ms Awaiting requested review from jpeng-ms jpeng-ms is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@waynemo waynemo Awaiting requested review from waynemo waynemo is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

@JoshuaLai JoshuaLai Awaiting requested review from JoshuaLai JoshuaLai is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

@palatter palatter Awaiting requested review from palatter palatter is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

Assignees

@emlynmac emlynmac

@JoshuaLai JoshuaLai

@Leah-Xia-Microsoft Leah-Xia-Microsoft

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vhuseinova-msft @emlynmac @Leah-Xia-Microsoft @JoshuaLai