Skip to content

Commit

Permalink
Fix for an issue with narrator focus for message menu button (#3728)
Browse files Browse the repository at this point in the history
  • Loading branch information
vhuseinova-msft authored Nov 3, 2023
1 parent a42d925 commit a62eeda
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "patch",
"area": "fix",
"workstream": "FluentUI",
"comment": "Fix for an issue with narrator focus for message menu button",
"packageName": "@azure/communication-react",
"email": "98852890+vhuseinova-msft@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "patch",
"area": "fix",
"workstream": "FluentUI",
"comment": "Fix for an issue with narrator focus for message menu button",
"packageName": "@azure/communication-react",
"email": "98852890+vhuseinova-msft@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ export const chatMessageActionMenuProps = (menuProps: {
ariaLabel?: string;
/** Whether the action menu button is enabled, if not this will always return undefined */
enabled: boolean;
/** Whether to force showing the action menu button - this has no effect if the action menu button is not enabled */
forceShow: boolean;
/** Whether the menu is shown */
menuExpanded: boolean;
menuButtonRef: React.MutableRefObject<HTMLDivElement | null>;
onActionButtonClick: () => void;
theme: Theme;
}): ChatMessageActionMenuProps | undefined => {
const { ariaLabel, enabled, forceShow, theme } = menuProps;
const showActionMenu = enabled || forceShow;
const { ariaLabel, enabled, theme, menuExpanded } = menuProps;
// Show the action button while the flyout is open (otherwise this will dismiss when the pointer is hovered over the flyout)
const showActionMenu = enabled || menuExpanded;

const actionMenuProps: ChatMessageActionMenuProps = {
children: (
Expand All @@ -42,9 +43,12 @@ export const chatMessageActionMenuProps = (menuProps: {
style={{ margin: showActionMenu ? '1px' : 0, minHeight: showActionMenu ? undefined : '30px' }}
role="button"
aria-label={showActionMenu ? ariaLabel : undefined}
aria-haspopup={showActionMenu}
// set expanded to true, only when the action menu is open
aria-expanded={menuExpanded}
>
{showActionMenu ? (
<Icon iconName="ChatMessageOptions" aria-label={ariaLabel} styles={iconWrapperStyle(theme, forceShow)} />
<Icon iconName="ChatMessageOptions" aria-label={ariaLabel} styles={iconWrapperStyle(theme, menuExpanded)} />
) : undefined}
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export const ChatMessageActionFlyout = (props: ChatMessageActionFlyoutProps): JS
};
const { onRenderAvatar } = props;
return {
'data-ui-id': 'chat-composite-message-contextual-menu-read-name-list-item',
key: person.displayName,
text: person.displayName,
itemProps: { styles: props.increaseFlyoutItemSize ? menuItemIncreasedSizeStyles : undefined },
Expand Down Expand Up @@ -145,7 +146,6 @@ export const ChatMessageActionFlyout = (props: ChatMessageActionFlyoutProps): JS
},
calloutProps: preventUnwantedDismissProps,
subMenuProps: {
id: 'chat-composite-message-contextual-menu-read-name-list',
items: messageReadByList ?? [],
calloutProps: preventUnwantedDismissProps,
styles: concatStyleSets({
Expand Down Expand Up @@ -226,7 +226,6 @@ export const ChatMessageActionFlyout = (props: ChatMessageActionFlyoutProps): JS
// gap space uses pixels
return (
<ContextualMenu
id="chat-composite-message-contextual-menu"
alignTargetEdge={true}
gapSpace={2 /*px*/}
isBeakVisible={false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const ChatMessageComponentAsEditBox = (props: ChatMessageComponentAsEditB
return (
<>
<InputBoxComponent
id={'editbox'}
data-ui-id="edit-box"
textFieldRef={editTextFieldRef}
inputClassName={editBoxStyle}
placeholderText={strings.editBoxPlaceholderText}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ const MessageBubble = (props: ChatMessageComponentAsMessageBubbleProps): JSX.Ele
ariaLabel: strings.actionMenuMoreOptions ?? '',
enabled: chatActionsEnabled,
menuButtonRef: messageActionButtonRef,
// Force show the action button while the flyout is open (otherwise this will dismiss when the pointer is hovered over the flyout)
forceShow: chatMessageActionFlyoutTarget === messageActionButtonRef,
menuExpanded: chatMessageActionFlyoutTarget === messageActionButtonRef,
onActionButtonClick: () => {
if (message.messageType === 'chat') {
props.onActionButtonClick(message, setMessageReadBy);
Expand Down Expand Up @@ -313,7 +312,7 @@ const MessageBubble = (props: ChatMessageComponentAsMessageBubbleProps): JSX.Ele
const attached = message.attached === true ? 'center' : message.attached === 'bottom' ? 'bottom' : 'top';
const chatMessage = (
<>
<div key={props.message.messageId} ref={messageRef}>
<div key={props.message.messageId}>
{message.mine ? (
<ChatMyMessage
attached={attached}
Expand All @@ -330,19 +329,22 @@ const MessageBubble = (props: ChatMessageComponentAsMessageBubbleProps): JSX.Ele
attached !== 'top' ? chatMyMessageStyles.bodyAttached : undefined,
mergeStyles(messageContainerStyle)
),
style: { ...createStyleFromV8Style(messageContainerStyle) }
style: { ...createStyleFromV8Style(messageContainerStyle) },
ref: messageRef
}}
root={{
className: chatMyMessageStyles.root,
onBlur: (e) => {
// copy behavior from North*
// `focused` controls is focused the whole `ChatMessage` or any of its children. When we're navigating
// with keyboard the focused element will be changed and there is no way to use `:focus` selector
if (chatMessageActionFlyoutTarget?.current) {
// doesn't dismiss action button if flyout is open, otherwise, narrator's focus will stay on the closed action menu
return;
}
const shouldPreserveFocusState = e.currentTarget.contains(e.relatedTarget);
setFocused(shouldPreserveFocusState);
},
onFocus: () => {
// copy behavior from North*
// react onFocus is called even when nested component receives focus (i.e. it bubbles)
// so when focus moves within actionMenu, the `focus` state in chatMessage remains true, and keeps actionMenu visible
setFocused(true);
Expand All @@ -365,7 +367,6 @@ const MessageBubble = (props: ChatMessageComponentAsMessageBubbleProps): JSX.Ele
}
details={getMessageDetails()}
actions={{
tabIndex: 0,
children: actionMenuProps?.children,
className: mergeClasses(
chatMyMessageStyles.menu,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ test.describe('ErrorBar is shown correctly', async () => {
// test resend button in contextual menu
await pageClick(page, dataUiId('chat-composite-message'));
await pageClick(page, dataUiId('chat-composite-message-action-icon'));
await page.waitForSelector('[id="chat-composite-message-contextual-menu"]');
await page.waitForSelector(dataUiId('chat-composite-message-contextual-menu-edit-action'));

expect(await stableScreenshot(page)).toMatchSnapshot(
'error-bar-send-message-with-wrong-thread-id-show-resend-button.png'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,9 @@ test.describe('Filesharing Edit Message', async () => {
await page.waitForSelector(dataUiId('file-download-card-group'));
await page.locator(dataUiId('chat-composite-message')).click();
await page.locator(dataUiId('chat-composite-message-action-icon')).click();
await page.waitForSelector('[id="chat-composite-message-contextual-menu"]');
await page.waitForSelector(dataUiId('chat-composite-message-contextual-menu-edit-action'));
await page.locator(dataUiId('chat-composite-message-contextual-menu-edit-action')).click();
await page.waitForSelector('[id="editbox"]');
await page.waitForSelector(dataUiId('edit-box'));

expect(
await stableScreenshot(page, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ test.describe('Chat Composite E2E Tests', () => {
await screenshotOnFailure(page, async () => {
await page.locator(dataUiId('chat-composite-message')).first().click();
await page.locator(dataUiId('chat-composite-message-action-icon')).first().click();
await waitForSelector(page, '[id="chat-composite-message-contextual-menu"]');
await page.waitForSelector(dataUiId('chat-composite-message-contextual-menu-edit-action'));
await page.locator(dataUiId('chat-composite-message-contextual-menu-read-info')).click();
await waitForSelector(page, '[id="chat-composite-message-contextual-menu-read-name-list"]');
await page.waitForSelector(dataUiId('chat-composite-message-contextual-menu-read-name-list-item'));
});

expect(await stableScreenshot(page, { stubMessageTimestamps: true })).toMatchSnapshot(
Expand Down

0 comments on commit a62eeda

Please sign in to comment.