fix: address PR reviews

stylesheets/_session_theme.scss

@@ -28,6 +28,7 @@
   flex-direction: row;
   align-items: center;
   max-width: 95%;
+  color: var(--color-sent-message-text);
 
   @media (min-width: 1200px) {
     max-width: 65%;
@@ -68,17 +69,14 @@
 
       a {
         text-decoration: underline;
-        color: inherit;
       }
     }
   }
-  color: var(--color-sent-message-text);
 
   &__container--outgoing {
     .module-message__text {
       a {
         text-decoration: underline;
-        color: inherit;
       }
     }
   }

ts/components/conversation/ImageGrid.tsx

@@ -20,48 +20,72 @@ type Props = {
   onClickAttachment?: (attachment: AttachmentTypeWithPath | AttachmentType) => void;
 };
 
-const StyledImageGrid = styled.div`
+const StyledImageGrid = styled.div<{ flexDirection: 'row' | 'column' }>`
   display: inline-flex;
-  flex-direction: row;
   align-items: center;
   gap: var(--margins-sm);
+  flex-direction: ${props => props.flexDirection};
 `;
-
-const StyledImageGridColumn = styled.div`
-  display: inline-flex;
-  flex-direction: column;
-  align-items: center;
-  gap: var(--margins-sm);
-`;
-
 // tslint:disable: cyclomatic-complexity max-func-body-length use-simple-attributes
+
+const Row = (
+  props: Props & { renderedSize: number; startIndex: number; totalAttachmentsCount: number }
+) => {
+  const {
+    attachments,
+    onError,
+    renderedSize,
+    startIndex,
+    onClickAttachment,
+    totalAttachmentsCount,
+  } = props;
+  const isMessageVisible = useContext(IsMessageVisibleContext);
+  const moreMessagesOverlay = totalAttachmentsCount > 3;
+  const moreMessagesOverlayText = moreMessagesOverlay ? `+${totalAttachmentsCount - 3}` : undefined;
+
+  return (
+    <>
+      {attachments.map((attachment, index) => {
+        const showOverlay = index === 1 && moreMessagesOverlay;
+        return (
+          <Image
+            alt={getAlt(attachment)}
+            attachment={attachment}
+            playIconOverlay={isVideoAttachment(attachment)}
+            height={renderedSize}
+            key={attachment.id}
+            width={renderedSize}
+            url={isMessageVisible ? getThumbnailUrl(attachment) : undefined}
+            attachmentIndex={startIndex + index}
+            onClick={onClickAttachment}
+            onError={onError}
+            softCorners={true}
+            darkOverlay={showOverlay}
+            overlayText={showOverlay ? moreMessagesOverlayText : undefined}
+          />
+        );
+      })}
+    </>
+  );
+};
+
 export const ImageGrid = (props: Props) => {
   const { attachments, onError, onClickAttachment } = props;
 
-  const isMessageVisible = useContext(IsMessageVisibleContext);
-
   if (!attachments || !attachments.length) {
     return null;
   }
 
-  const shared = {
-    onClick: onClickAttachment,
-    onError: onError,
-    softCorners: true,
-  };
-
   if (attachments.length === 1 || !areAllAttachmentsVisual(attachments)) {
     return (
-      <StyledImageGrid>
+      <StyledImageGrid flexDirection={'row'}>
-        <Image
+        <Row
-          alt={getAlt(attachments[0])}
+          attachments={attachments.slice(0, 1)}
-          attachment={attachments[0]}
+          onError={onError}
-          playIconOverlay={isVideoAttachment(attachments[0])}
+          onClickAttachment={onClickAttachment}
-          height={THUMBNAIL_SIDE}
+          renderedSize={THUMBNAIL_SIDE}
-          width={THUMBNAIL_SIDE}
+          startIndex={0}
-          url={isMessageVisible ? getThumbnailUrl(attachments[0]) : undefined}
+          totalAttachmentsCount={attachments.length}
-          attachmentIndex={0}
-          {...shared}
         />
       </StyledImageGrid>
     );
@@ -70,73 +94,43 @@ export const ImageGrid = (props: Props) => {
   if (attachments.length === 2) {
     // when we got 2 attachments we render them side by side with the full size of THUMBNAIL_SIDE
     return (
-      <StyledImageGrid>
+      <StyledImageGrid flexDirection={'row'}>
-        <Image
+        <Row
-          alt={getAlt(attachments[0])}
+          attachments={attachments.slice(0, 2)}
-          attachment={attachments[0]}
+          onError={onError}
-          playIconOverlay={isVideoAttachment(attachments[0])}
+          onClickAttachment={onClickAttachment}
-          height={THUMBNAIL_SIDE}
+          renderedSize={THUMBNAIL_SIDE}
-          width={THUMBNAIL_SIDE}
+          startIndex={0}
-          url={isMessageVisible ? getThumbnailUrl(attachments[0]) : undefined}
+          totalAttachmentsCount={attachments.length}
-          attachmentIndex={0}
-          {...shared}
-        />
-        <Image
-          alt={getAlt(attachments[1])}
-          playIconOverlay={isVideoAttachment(attachments[1])}
-          height={THUMBNAIL_SIDE}
-          width={THUMBNAIL_SIDE}
-          attachment={attachments[1]}
-          url={isMessageVisible ? getThumbnailUrl(attachments[1]) : undefined}
-          attachmentIndex={1}
-          {...shared}
         />
       </StyledImageGrid>
     );
   }
 
-  const moreMessagesOverlay = attachments.length > 3;
-  const moreMessagesOverlayText = moreMessagesOverlay ? `+${attachments.length - 3}` : undefined;
-
   const columnImageSide = THUMBNAIL_SIDE / 2 - 5;
 
-  // we know only support having 3 attachments displayed at most.
+  // we know only support having 3 attachments displayed at most, the rest are on the overlay
   return (
-    <StyledImageGrid>
+    <StyledImageGrid flexDirection={'row'}>
-      <Image
+      <Row
-        alt={getAlt(attachments[0])}
+        attachments={attachments.slice(0, 1)}
-        attachment={attachments[0]}
+        onError={onError}
-        playIconOverlay={isVideoAttachment(attachments[0])}
+        onClickAttachment={onClickAttachment}
-        height={THUMBNAIL_SIDE}
+        renderedSize={THUMBNAIL_SIDE}
-        width={THUMBNAIL_SIDE}
+        startIndex={0}
-        url={isMessageVisible ? getThumbnailUrl(attachments[0]) : undefined}
+        totalAttachmentsCount={attachments.length}
-        attachmentIndex={0}
-        {...shared}
       />
-      <StyledImageGridColumn>
+
-        <Image
+      <StyledImageGrid flexDirection={'column'}>
-          alt={getAlt(attachments[1])}
+        <Row
-          height={columnImageSide}
+          attachments={attachments.slice(1, 3)}
-          width={columnImageSide}
+          onError={onError}
-          attachment={attachments[1]}
+          onClickAttachment={onClickAttachment}
-          playIconOverlay={isVideoAttachment(attachments[1])}
+          renderedSize={columnImageSide}
-          url={isMessageVisible ? getThumbnailUrl(attachments[1]) : undefined}
+          startIndex={1}
-          attachmentIndex={1}
+          totalAttachmentsCount={attachments.length}
-          {...shared}
         />
-        <Image
+      </StyledImageGrid>
-          alt={getAlt(attachments[2])}
-          height={columnImageSide}
-          width={columnImageSide}
-          attachment={attachments[2]}
-          playIconOverlay={isVideoAttachment(attachments[2])}
-          url={isMessageVisible ? getThumbnailUrl(attachments[2]) : undefined}
-          attachmentIndex={2}
-          darkOverlay={moreMessagesOverlay}
-          overlayText={moreMessagesOverlayText}
-          {...shared}
-        />
-      </StyledImageGridColumn>
     </StyledImageGrid>
   );
 };

ts/components/conversation/message/message-content/MessageContent.tsx

@@ -50,8 +50,8 @@ const StyledMessageOpaqueContent = styled.div<{ messageDirection: MessageModelTy
       : 'var(--color-sent-message-background)'};
   align-self: ${props => (props.messageDirection === 'incoming' ? 'flex-start' : 'flex-end')};
 
-  padding: 7px 13px; // FIXME
+  padding: var(--padding-message-content);
-  border-radius: 16px; //FIXME
+  border-radius: var(--border-radius-message-box);
 `;
 
 export const IsMessageVisibleContext = createContext(false);
@@ -151,7 +151,7 @@ export const MessageContent = (props: Props) => {
         }}
       >
         <IsMessageVisibleContext.Provider value={isMessageVisible}>
-          {hasContentAfterAttachmentAndQuote ? (
+          {hasContentAfterAttachmentAndQuote && (
             <StyledMessageOpaqueContent messageDirection={direction}>
               {!isDeleted && (
                 <>
@@ -164,7 +164,7 @@ export const MessageContent = (props: Props) => {
               )}
               <MessageText messageId={props.messageId} />
             </StyledMessageOpaqueContent>
-          ) : null}
+          )}
           {!isDeleted && (
             <MessageAttachment
               messageId={props.messageId}