fix(chat): Improved usability, UI fixes, et al. (bigbluebutton#21555)

* fix(chat): Loading a new message page freezes the client - When the user clicks 'Load more messages' the client freezes for a few seconds before rendering the new page. This was happening because the message component was overheaded with hook calls and data access. In order to fix that, all those hook calls within the message component were placed in the parent component (more stable, doesn't rerender frequently) and passed as props to the message component. * fix(chat): Remove redundant condition * fix(chat): Add some arg validation in the getValueByPointer function * fix(chat): Memoize the onEmojiSelected handler * fix(chat): Memoize reaction mutation functions * fix(chat): Remove duplicated prop comparison * fix(chat): Move comparison prop arrys outside comparison functions * fix(chat): Add logging for edit, delete and reaction mutations * fix(chat): Memoize chat message ref updater * fix(chat): Restore original message input value after editing a message * fix(chat): Messages showing up behind welcome message * fix(chat): Message input focus ring as inset shadow * fix(chat): Tweak toolbar button hover styles * fix(chat): Disable chat roving if all toolbar settings are disabled * fix(chat): Add infinite scroll * fix(chat): Close reply intention button hiding when chat panel in narrowed * fix(chat): Only add top padding to message list if message toolbar is enabled * fix(chat): Add timeout for cleaning up loaded message pages * fix(chat): Focused replied message cleanup * fix(chat): Loading UI when a new page is loaded * fix(chat): Align offline indicator on the left * fix(chat): Revert settings.yml changes * fix(chat): Message input focus style and text alignment - Use border instead of box shadow. - Use 1px border. - Fix bad text alignment.
antonbsa · Oct 30, 2024 · 1c66ff3 · 1c66ff3
1 parent 0dcb7b2
commit 1c66ff3
Show file tree

Hide file tree

Showing 14 changed files with 156 additions and 79 deletions.
diff --git a/bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-form/component.tsx b/bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-form/component.tsx
@@ -138,11 +138,12 @@ const ChatMessageForm: React.FC<ChatMessageFormProps> = ({
   const emojiPickerButtonRef = useRef(null);
   const [isTextAreaFocused, setIsTextAreaFocused] = React.useState(false);
   const [repliedMessageId, setRepliedMessageId] = React.useState<string | null>(null);
-  const [editingMessage, setEditingMessage] = React.useState<EditingMessage | null>(null);
+  const editingMessage = React.useRef<EditingMessage | null>(null);
   const textAreaRef: RefObject<TextareaAutosize> = useRef<TextareaAutosize>(null);
   const { isMobile } = deviceInfo;
   const prevChatId = usePreviousValue(chatId);
   const messageRef = useRef<string>('');
+  const messageBeforeEditingRef = useRef<string | null>(null);
   messageRef.current = message;
   const updateUnreadMessages = (chatId: string, message: string) => {
     const storedData = localStorage.getItem('unsentMessages') || '{}';
@@ -302,16 +303,24 @@ const ChatMessageForm: React.FC<ChatMessageFormProps> = ({
     const handleEditingMessage = (e: Event) => {
       if (e instanceof CustomEvent) {
         if (textAreaRef.current) {
+          if (messageBeforeEditingRef.current === null) {
+            messageBeforeEditingRef.current = messageRef.current;
+          }
           setMessage(e.detail.message);
-          setEditingMessage(e.detail);
+          editingMessage.current = e.detail;
         }
       }
     };
 
     const handleCancelEditingMessage = (e: Event) => {
       if (e instanceof CustomEvent) {
-        setMessage('');
-        setEditingMessage(null);
+        if (editingMessage.current) {
+          if (messageBeforeEditingRef.current !== null) {
+            setMessage(messageBeforeEditingRef.current);
+            messageBeforeEditingRef.current = null;
+          }
+          editingMessage.current = null;
+        }
       }
     };
 
@@ -363,11 +372,11 @@ const ChatMessageForm: React.FC<ChatMessageFormProps> = ({
         }
       };
 
-      if (editingMessage && !chatEditMessageLoading) {
+      if (editingMessage.current && !chatEditMessageLoading) {
         chatEditMessage({
           variables: {
-            chatId: editingMessage.chatId,
-            messageId: editingMessage.messageId,
+            chatId: editingMessage.current.chatId,
+            messageId: editingMessage.current.messageId,
             chatMessageInMarkdownFormat: msg,
           },
         }).then(() => {

diff --git a/bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-form/styles.ts b/bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-form/styles.ts
@@ -12,7 +12,6 @@ import {
   smPaddingY,
   borderRadius,
   borderSize,
-  xsPadding,
 } from '/imports/ui/stylesheets/styled-components/general';
 import { fontSizeBase } from '/imports/ui/stylesheets/styled-components/typography';
 import TextareaAutosize from 'react-autosize-textarea';
@@ -43,15 +42,13 @@ const Form = styled.form<FormProps>`
 const Wrapper = styled.div`
   display: flex;
   flex-direction: row;
-  box-shadow: inset 0 0 0 1px ${colorGrayLightest};
   border-radius: 0.75rem;
 `;
 
 const Input = styled(TextareaAutosize)`
   flex: 1;
   background: #fff;
   background-clip: padding-box;
-  margin: ${xsPadding} 0 ${xsPadding} ${xsPadding};
   color: ${colorText};
   -webkit-appearance: none;
   padding: calc(${smPaddingY} * 2.5) calc(${smPaddingX} * 1.25);
@@ -171,19 +168,19 @@ const InputWrapper = styled.div`
   flex-grow: 1;
   min-width: 0;
   z-index: 0;
+  padding: 1px 0 1px 1px;
+  border: 1px solid ${colorGrayLightest};
 
   [dir='ltr'] & {
     border-radius: 0.75rem 0 0 0.75rem;
-    margin-right: ${xsPadding};
   }
 
   [dir='rtl'] & {
     border-radius: 0 0.75rem 0.75rem 0;
-    margin-left: ${xsPadding};
   }
 
   &:focus-within {
-    box-shadow: 0 0 0 ${xsPadding} ${colorBlueLight};
+    border: 1px solid ${colorBlueLight};
   }
 `;
 

diff --git a/bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-list/component.tsx b/bigbluebutton-html5/imports/ui/components/chat/chat-graphql/chat-message-list/component.tsx
@@ -19,7 +19,6 @@ import { Message } from '/imports/ui/Types/message';
 import ChatListPage from './page/component';
 import LAST_SEEN_MUTATION from './queries';
 import {
-  ButtonLoadMore,
   MessageList,
   UnreadButton,
 } from './styles';
@@ -40,6 +39,7 @@ import { CHAT_DELETE_REACTION_MUTATION, CHAT_SEND_REACTION_MUTATION } from './pa
 import logger from '/imports/startup/client/logger';
 
 const PAGE_SIZE = 50;
+const CLEANUP_TIMEOUT = 3000;
 
 const intlMessages = defineMessages({
   loadMoreButtonLabel: {
@@ -180,21 +180,28 @@ const ChatMessageList: React.FC<ChatListProps> = ({
   const intl = useIntl();
   // I used a ref here because I don't want to re-render the component when the last sender changes
   const lastSenderPerPage = React.useRef<Map<number, string>>(new Map());
-  const sentinelRef = React.useRef<HTMLDivElement | null>(null);
+  const endSentinelRef = React.useRef<HTMLDivElement | null>(null);
+  const startSentinelRef = React.useRef<HTMLDivElement | null>(null);
   const {
     ref: messageListContainerRef,
     current: currentMessageListContainer,
   } = useReactiveRef<HTMLDivElement>(null);
   const messageListRef = React.useRef<HTMLDivElement>(null);
+  const cleanupTimeoutRef = React.useRef<ReturnType<typeof setTimeout> | null>(null);
   const [userLoadedBackUntilPage, setUserLoadedBackUntilPage] = useState<number | null>(null);
   const [lastMessageCreatedAt, setLastMessageCreatedAt] = useState<string>('');
   const [followingTail, setFollowingTail] = React.useState(true);
   const [selectedMessage, setSelectedMessage] = React.useState<HTMLElement | null>(null);
   const {
-    childRefProxy: sentinelRefProxy,
-    intersecting: isSentinelVisible,
-    parentRefProxy: messageListContainerRefProxy,
-  } = useIntersectionObserver(messageListContainerRef, sentinelRef);
+    childRefProxy: endSentinelRefProxy,
+    intersecting: isEndSentinelVisible,
+    parentRefProxy: endSentinelParentRefProxy,
+  } = useIntersectionObserver(messageListContainerRef, endSentinelRef);
+  const {
+    childRefProxy: startSentinelRefProxy,
+    intersecting: isStartSentinelVisible,
+    parentRefProxy: startSentinelParentRefProxy,
+  } = useIntersectionObserver(messageListContainerRef, startSentinelRef);
   const {
     startObserving: startObservingStickyScroll,
     stopObserving: stopObservingStickyScroll,
@@ -272,13 +279,27 @@ const ChatMessageList: React.FC<ChatListProps> = ({
   }, [chatDeleteReaction]);
 
   useEffect(() => {
-    if (isSentinelVisible) {
+    if (isEndSentinelVisible) {
       startObservingStickyScroll();
     } else {
       stopObservingStickyScroll();
     }
-    toggleFollowingTail(isSentinelVisible);
-  }, [isSentinelVisible]);
+    toggleFollowingTail(isEndSentinelVisible);
+  }, [isEndSentinelVisible]);
+
+  useEffect(() => {
+    if (isStartSentinelVisible) {
+      if (followingTail) {
+        toggleFollowingTail(false);
+      }
+      setUserLoadedBackUntilPage((prev) => {
+        if (typeof prev === 'number' && prev > 0) {
+          return prev - 1;
+        }
+        return prev;
+      });
+    }
+  }, [isStartSentinelVisible, followingTail]);
 
   useEffect(() => {
     setter({
@@ -330,7 +351,7 @@ const ChatMessageList: React.FC<ChatListProps> = ({
   }, [lastMessageCreatedAt, chatId]);
 
   const setScrollToTailEventHandler = () => {
-    toggleFollowingTail(isSentinelVisible);
+    toggleFollowingTail(isEndSentinelVisible);
   };
 
   const toggleFollowingTail = (toggle: boolean) => {
@@ -357,8 +378,8 @@ const ChatMessageList: React.FC<ChatListProps> = ({
           key="unread-messages"
           label={intl.formatMessage(intlMessages.moreMessages)}
           onClick={() => {
-            if (sentinelRef.current) {
-              sentinelRef.current.scrollIntoView({ behavior: 'smooth' });
+            if (endSentinelRef.current) {
+              endSentinelRef.current.scrollIntoView({ behavior: 'smooth' });
             }
           }}
         />
@@ -369,8 +390,8 @@ const ChatMessageList: React.FC<ChatListProps> = ({
 
   useEffect(() => {
     const scrollToTailEventHandler = () => {
-      if (isElement(sentinelRef.current)) {
-        sentinelRef.current.scrollIntoView();
+      if (isElement(endSentinelRef.current)) {
+        endSentinelRef.current.scrollIntoView();
       }
     };
 
@@ -383,13 +404,20 @@ const ChatMessageList: React.FC<ChatListProps> = ({
 
   useEffect(() => {
     if (followingTail) {
-      setUserLoadedBackUntilPage(null);
+      if (userLoadedBackUntilPage !== null) {
+        cleanupTimeoutRef.current = setTimeout(() => {
+          setUserLoadedBackUntilPage(null);
+        }, CLEANUP_TIMEOUT);
+      }
+    } else if (cleanupTimeoutRef.current !== null) {
+      clearTimeout(cleanupTimeoutRef.current);
+      cleanupTimeoutRef.current = null;
     }
-  }, [followingTail]);
+  }, [followingTail, userLoadedBackUntilPage]);
 
   useEffect(() => {
-    if (isElement(sentinelRef.current)) {
-      sentinelRef.current.scrollIntoView();
+    if (isElement(endSentinelRef.current)) {
+      endSentinelRef.current.scrollIntoView();
     }
   }, []);
 
@@ -409,6 +437,17 @@ const ChatMessageList: React.FC<ChatListProps> = ({
     }
   };
 
+  const hasMessageToolbar = CHAT_DELETE_ENABLED
+    || CHAT_EDIT_ENABLED
+    || CHAT_REPLY_ENABLED
+    || CHAT_REACTIONS_ENABLED;
+
+  const updateRefs = useCallback((el: HTMLDivElement | null) => {
+    messageListContainerRef.current = el;
+    startSentinelParentRefProxy.current = el;
+    endSentinelParentRefProxy.current = el;
+  }, []);
+
   return (
     <>
       {
@@ -424,30 +463,28 @@ const ChatMessageList: React.FC<ChatListProps> = ({
             }}
             data-test="chatMessages"
             isRTL={isRTL}
-            ref={messageListContainerRefProxy}
+            ref={updateRefs}
+            $hasMessageToolbar={hasMessageToolbar}
           >
             <div
               role="listbox"
               ref={messageListRef}
-              tabIndex={0}
+              tabIndex={hasMessageToolbar ? 0 : -1}
               onKeyDown={rove}
               onBlur={() => {
                 setSelectedMessage(null);
               }}
             >
-              {userLoadedBackUntilPage ? (
-                <ButtonLoadMore
-                  onClick={() => {
-                    if (followingTail) {
-                      toggleFollowingTail(false);
-                    }
-                    setUserLoadedBackUntilPage(userLoadedBackUntilPage - 1);
-                  }}
-                >
-                  {intl.formatMessage(intlMessages.loadMoreButtonLabel)}
-                </ButtonLoadMore>
-              ) : null}
-              <ChatPopupContainer />
+              <div
+                ref={startSentinelRefProxy}
+                style={{
+                  height: 1,
+                  background: 'none',
+                }}
+                tabIndex={-1}
+                aria-hidden
+              />
+              <ChatPopupContainer hasMessageToolbar={hasMessageToolbar} />
               {Array.from({ length: pagesToLoad }, (_v, k) => k + (firstPageToLoad)).map((page) => {
                 return (
                   <ChatListPage
@@ -459,7 +496,7 @@ const ChatMessageList: React.FC<ChatListProps> = ({
                     lastSenderPreviousPage={page ? lastSenderPerPage.current.get(page - 1) : undefined}
                     chatId={chatId}
                     markMessageAsSeen={markMessageAsSeen}
-                    scrollRef={messageListContainerRefProxy}
+                    scrollRef={messageListContainerRef}
                     focusedId={selectedMessage?.dataset.sequence
                       ? Number.parseInt(selectedMessage?.dataset.sequence, 10)
                       : null}
@@ -482,7 +519,7 @@ const ChatMessageList: React.FC<ChatListProps> = ({
               })}
             </div>
             <div
-              ref={sentinelRefProxy}
+              ref={endSentinelRefProxy}
               style={{
                 height: 1,
                 background: 'none',

diff --git a/...onents/chat/chat-graphql/chat-message-list/page/chat-message/message-header/component.tsx b/...onents/chat/chat-graphql/chat-message-list/page/chat-message/message-header/component.tsx
@@ -47,6 +47,7 @@ const ChatMessageHeader: React.FC<ChatMessageHeaderProps> = ({
             </Styled.ChatUserOffline>
           )
         }
+        <Styled.Center />
         {!deleteTime && editTime && (
           <Styled.EditLabel>
             <Icon iconName="pen_tool" />

diff --git a/...components/chat/chat-graphql/chat-message-list/page/chat-message/message-header/styles.ts b/...components/chat/chat-graphql/chat-message-list/page/chat-message/message-header/styles.ts
@@ -31,7 +31,7 @@ export const ChatUserName = styled.div<ChatUserNameProps>`
   white-space: nowrap;
   overflow: hidden;
   text-overflow: ellipsis;
-  flex-grow: 1;
+  flex-shrink: 1;
 
   ${({ currentlyInMeeting }) => currentlyInMeeting && `
     color: ${colorHeading};
@@ -102,11 +102,16 @@ export const EditLabel = styled.span`
   }
 `;
 
+export const Center = styled.div`
+  flex-grow: 1;
+`;
+
 export default {
   HeaderContent,
   ChatTime,
   ChatUserOffline,
   ChatUserName,
   ChatHeaderText,
   EditLabel,
+  Center,
 };
diff --git a/...nents/chat/chat-graphql/chat-message-list/page/chat-message/message-replied/component.tsx b/...nents/chat/chat-graphql/chat-message-list/page/chat-message/message-replied/component.tsx
@@ -37,7 +37,6 @@ const ChatMessageReplied: React.FC<MessageRepliedProps> = (props) => {
             },
           }),
         );
-        Storage.removeItem(ChatEvents.CHAT_FOCUS_MESSAGE_REQUEST);
         Storage.setItem(ChatEvents.CHAT_FOCUS_MESSAGE_REQUEST, sequence);
       }}
     >

diff --git a/...t/chat-graphql/chat-message-list/page/chat-message/message-toolbar/emoji-button/styles.ts b/...t/chat-graphql/chat-message-list/page/chat-message/message-toolbar/emoji-button/styles.ts
@@ -1,6 +1,6 @@
 import styled from 'styled-components';
-import { colorGray } from '/imports/ui/stylesheets/styled-components/palette';
-import { lgPadding } from '/imports/ui/stylesheets/styled-components/general';
+import { colorGray, colorGrayDark, colorOffWhite } from '/imports/ui/stylesheets/styled-components/palette';
+import { smPadding } from '/imports/ui/stylesheets/styled-components/general';
 import { fontSizeSmaller } from '/imports/ui/stylesheets/styled-components/typography';
 
 const EmojiButton = styled.button`
@@ -9,13 +9,15 @@ const EmojiButton = styled.button`
   background: none;
   border: none;
   outline: none;
-  padding: ${lgPadding};
+  padding: ${smPadding};
   color: ${colorGray};
   cursor: pointer;
+  border-radius: 0.25rem;
 
   &:focus,
   &:hover {
-    opacity: 0.5;
+    color: ${colorGrayDark};
+    background-color: ${colorOffWhite};
   }
 
   &:active {
-Original file line number
+Diff line change
@@ Expand Up @@
                 },
               }),
             );
-            Storage.removeItem(ChatEvents.CHAT_FOCUS_MESSAGE_REQUEST);
             Storage.setItem(ChatEvents.CHAT_FOCUS_MESSAGE_REQUEST, sequence);
           }}
         >
@@ Expand Down @@