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

feat: Reaction Popover is stuck on a message or several messages #17495

Merged
merged 4 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*
*/

import {useMemo, useState, useEffect} from 'react';
import {useMemo, useState, useEffect, useCallback, useRef} from 'react';

import {QualifiedId} from '@wireapp/api-client/lib/user';
import cx from 'classnames';
Expand Down Expand Up @@ -47,6 +47,7 @@ import {ContextMenuEntry} from '../../../../ui/ContextMenu';
import {EphemeralTimer} from '../EphemeralTimer';
import {MessageTime} from '../MessageTime';
import {useMessageFocusedTabIndex} from '../util';

export interface ContentMessageProps extends Omit<MessageActions, 'onClickResetSession'> {
contextMenu: {entries: ko.Subscribable<ContextMenuEntry[]>};
conversation: Conversation;
Expand Down Expand Up @@ -86,6 +87,7 @@ export const ContentMessageComponent = ({
onClickReaction,
onClickDetails,
}: ContentMessageProps) => {
const messageRef = useRef<HTMLDivElement | null>(null);
// check if current message is focused and its elements focusable
const msgFocusState = useMemo(() => isMsgElementsFocusable && isFocused, [isMsgElementsFocusable, isFocused]);
const messageFocusedTabIndex = useMessageFocusedTabIndex(msgFocusState);
Expand Down Expand Up @@ -125,13 +127,16 @@ export const ContentMessageComponent = ({

const [isActionMenuVisible, setActionMenuVisibility] = useState(false);
const isMenuOpen = useMessageActionsState(state => state.isMenuOpen);

useEffect(() => {
setActionMenuVisibility(isFocused || msgFocusState);
}, [msgFocusState, isFocused]);

const isConversationReadonly = conversation.readOnlyState() !== null;

const contentMessageWrapperRef = (element: HTMLDivElement | null) => {
messageRef.current = element;

setTimeout(() => {
if (element?.parentElement?.querySelector(':hover') === element) {
// Trigger the action menu in case the component is rendered with the mouse already hovering over it
Expand All @@ -148,6 +153,26 @@ export const ContentMessageComponent = ({

const isAssetMessage = isFileMessage || isAudioMessage || isVideoMessage || isImageMessage;

const handleOutsideClick = useCallback((event: Event) => {
if (!messageRef.current) {
return;
}

event.preventDefault();

if (!messageRef.current.contains(event.target as Node)) {
setActionMenuVisibility(false);
}
}, []);

useEffect(() => {
window.addEventListener('click', handleOutsideClick);

return () => {
window.removeEventListener('click', handleOutsideClick);
};
}, [handleOutsideClick]);

return (
<div
aria-label={messageAriaLabel}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface MessageParams extends MessageActions {
isLastDeliveredMessage: boolean;
isSelfTemporaryGuest: boolean;
message: BaseMessage;
/** whether the message should display the user avatar and user name before the actual content */
/** whether the message should display the user avatar and userName before the actual content */
hideHeader: boolean;
messageActions: {
deleteMessage: (conversation: Conversation, message: BaseMessage) => void;
Expand All @@ -76,13 +76,13 @@ export interface MessageParams extends MessageActions {
isFocused: boolean;
/** will visually highlight the message when it's being loaded */
isHighlighted: boolean;
handleFocus: (id: string) => void;
handleFocus: (id?: string) => void;
handleArrowKeyDown: (e: React.KeyboardEvent) => void;
isMsgElementsFocusable: boolean;
setMsgElementsFocusable: (isMsgElementsFocusable: boolean) => void;
}

export const Message: React.FC<MessageParams & {scrollTo?: ScrollToElement}> = props => {
export const Message = (props: MessageParams & {scrollTo?: ScrollToElement}) => {
const {
message,
isHighlighted,
Expand All @@ -95,15 +95,17 @@ export const Message: React.FC<MessageParams & {scrollTo?: ScrollToElement}> = p
isMsgElementsFocusable,
setMsgElementsFocusable,
} = props;

const messageElementRef = useRef<HTMLDivElement>(null);
const messageRef = useRef<HTMLDivElement>(null);

const {status, ephemeral_expires} = useKoSubscribableChildren(message, ['status', 'ephemeral_expires']);
const messageFocusedTabIndex = useMessageFocusedTabIndex(isFocused);

useLayoutEffect(() => {
if (!messageElementRef.current) {
return;
}

if (isHighlighted) {
scrollTo?.({center: true, element: messageElementRef.current});
// for reply message, focus on the original message when original message link is clicked for keyboard users
Expand All @@ -114,7 +116,7 @@ export const Message: React.FC<MessageParams & {scrollTo?: ScrollToElement}> = p
const handleDivKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
// when a message is focused set its elements focusable
if (!event.shiftKey && isTabKey(event)) {
if (!messageRef.current) {
if (!messageElementRef.current) {
return;
}
setMsgElementsFocusable(true);
Expand All @@ -130,21 +132,21 @@ export const Message: React.FC<MessageParams & {scrollTo?: ScrollToElement}> = p
useEffect(() => {
// Move element into view when it is focused
if (isFocused) {
messageRef.current?.focus();
messageElementRef.current?.focus();
}
}, [isFocused]);

// set message elements focus for non content type mesages
// some non content type message has interactive element like invite people for member message
// set message elements focus for non-content type messages
// some non-content type message has interactive element like invite people for member message
useEffect(() => {
if (!messageRef.current || message.isContent()) {
if (!messageElementRef.current || message.isContent()) {
return;
}
const interactiveMsgElements = getAllFocusableElements(messageRef.current);
const interactiveMsgElements = getAllFocusableElements(messageElementRef.current);
setElementsTabIndex(interactiveMsgElements, isMsgElementsFocusable && isFocused);
}, [isFocused, isMsgElementsFocusable, message]);

const content = (
const messageContent = (
<MessageWrapper
{...props}
hideHeader={hideHeader}
Expand All @@ -153,40 +155,32 @@ export const Message: React.FC<MessageParams & {scrollTo?: ScrollToElement}> = p
/>
);

const wrappedContent = onVisible ? (
<InViewport requireFullyInView allowBiggerThanViewport checkOverlay onVisible={onVisible}>
{content}
</InViewport>
) : (
content
);

return (
/*eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions*/
<div
ref={messageElementRef}
className={cx('message', {
'message-marked': isHighlighted,
'content-message': message.isContent(),
'system-message': !message.isContent(),
})}
ref={messageElementRef}
role="list"
tabIndex={messageFocusedTabIndex}
data-uie-uid={message.id}
data-uie-value={message.super_type}
data-uie-expired-status={ephemeral_expires}
data-uie-send-status={status}
data-uie-name="item-message"
role="list"
onKeyDown={handleDivKeyDown}
onClick={() => handleFocus(message.id)}
>
{/*eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions*/}
<div
tabIndex={messageFocusedTabIndex}
ref={messageRef}
role="listitem"
onKeyDown={handleDivKeyDown}
onClick={() => handleFocus(message.id)}
className="message-wrapper"
>
{wrappedContent}
</div>
{onVisible ? (
<InViewport requireFullyInView allowBiggerThanViewport checkOverlay onVisible={onVisible}>
{messageContent}
</InViewport>
) : (
messageContent
)}
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {FederationStopMessage} from './FederationStopMessage';
import {FileTypeRestrictedMessage} from './FileTypeRestrictedMessage';
import {LegalHoldMessage} from './LegalHoldMessage';
import {MemberMessage} from './MemberMessage';
import {MessageParams} from './Message';
import {MissedMessage} from './MissedMessage';
import {PingMessage} from './PingMessage';
import {SystemMessage} from './SystemMessage';
Expand All @@ -55,8 +56,6 @@ import {CompositeMessage} from '../../../entity/message/CompositeMessage';
import {TeamState} from '../../../team/TeamState';
import {ContextMenuEntry} from '../../../ui/ContextMenu';

import {MessageParams} from './index';

const isOutgoingQuote = (quoteEntity: QuoteEntity): quoteEntity is OutgoingQuote => {
return quoteEntity.hash !== undefined;
};
Expand Down
20 changes: 20 additions & 0 deletions src/script/components/MessagesList/Message/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Wire
* Copyright (C) 2024 Wire Swiss GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*
*/

export * from './Message';
27 changes: 13 additions & 14 deletions src/style/content/conversation/message-list.less
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@
// MESSAGE
.message {
position: relative;
outline-offset: -0.1rem;

&.content-message {
&:hover,
&:focus-visible {
background-color: #fff;

body.theme-dark & {
background-color: var(--gray-90);
}
}
}

& * {
.accent-selection;
Expand Down Expand Up @@ -87,7 +99,7 @@

.content-message {
&:has(.message-header) {
padding-top: 6px;
margin-top: 6px;
}
}

Expand All @@ -107,15 +119,6 @@

.content-message-wrapper {
padding-block: 6px;

&:hover,
&:focus-visible {
background-color: #fff;

body.theme-dark & {
background-color: var(--gray-90);
}
}
}

.message-marked {
Expand Down Expand Up @@ -264,10 +267,6 @@
height: 16px;
}

.message-wrapper {
outline-offset: -0.1rem;
}

.message-mention {
outline-offset: 0.4rem;
}
Expand Down
Loading