Skip to content

Commit

Permalink
fix: use popper to properly position message actions box (#2241)
Browse files Browse the repository at this point in the history
### 🎯 Goal

Message actions box is currently relatively positioned inside
MessageList, which means it sometimes gets clipped by MessageList's
boundaries.

This PR implements proper positioning for the actions box, preventing it
from being clipped in (almost) every case.

### 🛠 Implementation details

`stream-chat-react` already uses `react-popper` for tooltips, so it made
sense to reuse it for the actions box as well.

My initial plan was to render the actions box into a portal and position
it using popper, but using a portal turned out to be unnecessary: with
the positioning strategies that popper implements, the action box is
never clipped by it's parent container.

See also: GetStream/stream-chat-css#260

### 🎨 UI Changes

Previously:


![image](https://github.com/GetStream/stream-chat-react/assets/975978/cbeb3f34-7e4d-45bc-94f3-0473b376db76)


![image](https://github.com/GetStream/stream-chat-react/assets/975978/6a71ef77-47b6-42c5-b45c-20c01d114d35)

After the fixes:

The actions box automatically flips if it's close to the clipping
boundary:


![image](https://github.com/GetStream/stream-chat-react/assets/975978/605a99c7-d2dd-4a7a-b039-69918fc444b7)


![image](https://github.com/GetStream/stream-chat-react/assets/975978/df6ab182-8d8a-4992-8323-9ddafbf51742)

### ✅ To-Do

- [x] Check with all message types
- [x] Check with Angular app
- [x] ~~Deal with theming v1~~
- [x] ~~Fix E2E~~
  • Loading branch information
myandrienko committed Jan 18, 2024
1 parent a95f3b5 commit 651d3e7
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 43 deletions.
49 changes: 37 additions & 12 deletions src/components/MessageActions/MessageActions.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import React, { PropsWithChildren, useCallback, useEffect, useState } from 'react';
import React, {
ElementRef,
PropsWithChildren,
useCallback,
useEffect,
useRef,
useState,
} from 'react';

import { MessageActionsBox } from './MessageActionsBox';

Expand All @@ -9,6 +16,7 @@ import { useChatContext } from '../../context/ChatContext';
import { MessageContextValue, useMessageContext } from '../../context/MessageContext';

import type { DefaultStreamChatGenerics, IconProps } from '../../types/types';
import { useMessageActionsBoxPopper } from './hooks';

type MessageContextPropsToPick =
| 'getMessageActions'
Expand Down Expand Up @@ -71,6 +79,7 @@ export const MessageActions = <
const handleMute = propHandleMute || contextHandleMute;
const handlePin = propHandlePin || contextHandlePin;
const message = propMessage || contextMessage;
const isMine = mine ? mine() : isMyMessage();

const [actionsBoxOpen, setActionsBoxOpen] = useState(false);

Expand Down Expand Up @@ -109,6 +118,14 @@ export const MessageActions = <
};
}, [actionsBoxOpen, hideOptions]);

const actionsBoxButtonRef = useRef<ElementRef<'button'>>(null);

const { attributes, popperElementRef, styles } = useMessageActionsBoxPopper<HTMLDivElement>({
open: actionsBoxOpen,
placement: isMine ? 'top-end' : 'top-start',
referenceElement: actionsBoxButtonRef.current,
});

if (!messageActions.length && !customMessageActions) return null;

return (
Expand All @@ -117,22 +134,30 @@ export const MessageActions = <
inline={inline}
setActionsBoxOpen={setActionsBoxOpen}
>
<MessageActionsBox
getMessageActions={getMessageActions}
handleDelete={handleDelete}
handleEdit={setEditingState}
handleFlag={handleFlag}
handleMute={handleMute}
handlePin={handlePin}
isUserMuted={isMuted}
mine={mine ? mine() : isMyMessage()}
open={actionsBoxOpen}
/>
<div
{...attributes.popper}
className='str-chat__message-actions-box-wrapper'
ref={popperElementRef}
style={styles.popper}
>
<MessageActionsBox
getMessageActions={getMessageActions}
handleDelete={handleDelete}
handleEdit={setEditingState}
handleFlag={handleFlag}
handleMute={handleMute}
handlePin={handlePin}
isUserMuted={isMuted}
mine={isMine}
open={actionsBoxOpen}
/>
</div>
<button
aria-expanded={actionsBoxOpen}
aria-haspopup='true'
aria-label='Open Message Actions Menu'
className='str-chat__message-actions-box-button'
ref={actionsBoxButtonRef}
>
<ActionsIcon className='str-chat__message-action-icon' />
</button>
Expand Down
31 changes: 3 additions & 28 deletions src/components/MessageActions/MessageActionsBox.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useState } from 'react';
import React from 'react';
import clsx from 'clsx';

import { MESSAGE_ACTIONS } from '../Message/utils';
Expand Down Expand Up @@ -44,44 +44,21 @@ const UnMemoizedMessageActionsBox = <
handleMute,
handlePin,
isUserMuted,
mine,
open = false,
} = props;

const {
CustomMessageActionsList = DefaultCustomMessageActionsList,
} = useComponentContext<StreamChatGenerics>('MessageActionsBox');
const { setQuotedMessage } = useChannelActionContext<StreamChatGenerics>('MessageActionsBox');
const { customMessageActions, message, messageListRect } = useMessageContext<StreamChatGenerics>(
const { customMessageActions, message } = useMessageContext<StreamChatGenerics>(
'MessageActionsBox',
);

const { t } = useTranslationContext('MessageActionsBox');

const [reverse, setReverse] = useState(false);

const messageActions = getMessageActions();

const checkIfReverse = useCallback(
(containerElement: HTMLDivElement) => {
if (!containerElement) {
setReverse(false);
return;
}

if (open) {
const containerRect = containerElement.getBoundingClientRect();

if (mine) {
setReverse(!!messageListRect && containerRect.left < messageListRect.left);
} else {
setReverse(!!messageListRect && containerRect.right + 5 > messageListRect.right);
}
}
},
[messageListRect, mine, open],
);

const handleQuote = () => {
setQuotedMessage(message);

Expand All @@ -96,15 +73,13 @@ const UnMemoizedMessageActionsBox = <
};

const rootClassName = clsx('str-chat__message-actions-box', {
'str-chat__message-actions-box--mine': mine,
'str-chat__message-actions-box--open': open,
'str-chat__message-actions-box--reverse': reverse,
});
const buttonClassName =
'str-chat__message-actions-list-item str-chat__message-actions-list-item-button';

return (
<div className={rootClassName} data-testid='message-actions-box' ref={checkIfReverse}>
<div className={rootClassName} data-testid='message-actions-box'>
<div aria-label='Message Options' className='str-chat__message-actions-list' role='listbox'>
<CustomMessageActionsList customMessageActions={customMessageActions} message={message} />
{messageActions.indexOf(MESSAGE_ACTIONS.quote) > -1 && (
Expand Down
39 changes: 36 additions & 3 deletions src/components/MessageActions/__tests__/MessageActions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,18 @@ describe('<MessageActions /> component', () => {
data-testid="message-actions"
onClick={[Function]}
>
<div />
<div
className="str-chat__message-actions-box-wrapper"
style={
Object {
"left": "0",
"position": "absolute",
"top": "0",
}
}
>
<div />
</div>
<button
aria-expanded={false}
aria-haspopup="true"
Expand Down Expand Up @@ -243,7 +254,18 @@ describe('<MessageActions /> component', () => {
data-testid="message-actions"
onClick={[Function]}
>
<div />
<div
className="str-chat__message-actions-box-wrapper"
style={
Object {
"left": "0",
"position": "absolute",
"top": "0",
}
}
>
<div />
</div>
<button
aria-expanded={false}
aria-haspopup="true"
Expand Down Expand Up @@ -283,7 +305,18 @@ describe('<MessageActions /> component', () => {
data-testid="message-actions"
onClick={[Function]}
>
<div />
<div
className="str-chat__message-actions-box-wrapper"
style={
Object {
"left": "0",
"position": "absolute",
"top": "0",
}
}
>
<div />
</div>
<button
aria-expanded={false}
aria-haspopup="true"
Expand Down
1 change: 1 addition & 0 deletions src/components/MessageActions/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './useMessageActionsBoxPopper';
45 changes: 45 additions & 0 deletions src/components/MessageActions/hooks/useMessageActionsBoxPopper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { Placement } from '@popperjs/core';
import { useEffect, useRef } from 'react';
import { usePopper } from 'react-popper';

export interface MessageActionsBoxPopperOptions {
open: boolean;
placement: Placement;
referenceElement: HTMLElement | null;
}

export function useMessageActionsBoxPopper<T extends HTMLElement>({
open,
placement,
referenceElement,
}: MessageActionsBoxPopperOptions) {
const popperElementRef = useRef<T>(null);
const { attributes, styles, update } = usePopper(referenceElement, popperElementRef.current, {
modifiers: [
{
name: 'eventListeners',
options: {
// It's not safe to update popper position on resize and scroll, since popper's
// reference element might not be visible at the time.
resize: false,
scroll: false,
},
},
],
placement,
});

useEffect(() => {
if (open) {
// Since the popper's reference element might not be (and usually is not) visible
// all the time, it's safer to force popper update before showing it.
update?.();
}
}, [open, update]);

return {
attributes,
popperElementRef,
styles,
};
}

0 comments on commit 651d3e7

Please sign in to comment.