One lightbox at a time; proper dismiss of "read more" screen; proper in-progress draft HEIC images

ts/components/Lightbox.tsx

@@ -22,6 +22,7 @@ import { IMAGE_PNG, isImage, isVideo } from '../types/MIME';
 import { LocalizerType } from '../types/Util';
 import { MediaItemType, MessageAttributesType } from '../types/MediaItem';
 import { formatDuration } from '../util/formatDuration';
+import { useRestoreFocus } from '../util/hooks/useRestoreFocus';
 
 export type PropsType = {
   children?: ReactNode;
@@ -55,20 +56,13 @@ export function Lightbox({
     initialSelectedIndex
   );
 
-  const [previousFocus, setPreviousFocus] = useState<HTMLElement | undefined>();
   const [videoElement, setVideoElement] = useState<HTMLVideoElement | null>(
     null
   );
   const [videoTime, setVideoTime] = useState<number | undefined>();
   const [zoomed, setZoomed] = useState(false);
   const containerRef = useRef<HTMLDivElement | null>(null);
-  const focusRef = useRef<HTMLDivElement | null>(null);
-
-  const restorePreviousFocus = useCallback(() => {
-    if (previousFocus && previousFocus.focus) {
-      previousFocus.focus();
-    }
-  }, [previousFocus]);
+  const [focusRef] = useRestoreFocus();
 
   const onPrevious = useCallback(() => {
     setSelectedIndex(prevSelectedIndex => Math.max(prevSelectedIndex - 1, 0));
@@ -167,18 +161,6 @@ export function Lightbox({
     };
   }, []);
 
-  useEffect(() => {
-    if (!previousFocus) {
-      setPreviousFocus(document.activeElement as HTMLElement);
-    }
-  }, [previousFocus]);
-
-  useEffect(() => {
-    return () => {
-      restorePreviousFocus();
-    };
-  }, [restorePreviousFocus]);
-
   useEffect(() => {
     const useCapture = true;
     document.addEventListener('keydown', onKeyDown, useCapture);
@@ -191,10 +173,6 @@ export function Lightbox({
   useEffect(() => {
     playVideo();
 
-    if (focusRef && focusRef.current) {
-      focusRef.current.focus();
-    }
-
     if (videoElement && isViewOnce) {
       videoElement.addEventListener('timeupdate', onTimeUpdate);
 

ts/components/Modal.tsx

@@ -56,6 +56,9 @@ export function Modal({
 
   return (
     <ModalHost noMouseClose={noMouseClose} onClose={onClose} theme={theme}>
+      {/* We don't want the click event to propagate to its container node. */}
+      {/* eslint-disable jsx-a11y/no-static-element-interactions */}
+      {/* eslint-disable jsx-a11y/click-events-have-key-events */}
       <div
         className={classNames(
           getClassName(''),
@@ -63,7 +66,12 @@ export function Modal({
           hasStickyButtons && getClassName('--sticky-buttons')
         )}
         ref={modalRef}
+        onClick={event => {
+          event.stopPropagation();
+        }}
       >
+        {/* eslint-enable jsx-a11y/no-static-element-interactions */}
+        {/* eslint-enable jsx-a11y/click-events-have-key-events */}
         {hasHeader && (
           <div className={getClassName('__header')}>
             {hasXButton && (

ts/components/conversation/AttachmentList.tsx

@@ -66,7 +66,7 @@ export const AttachmentList = ({
           const isImage = isImageAttachment(attachment);
           const isVideo = isVideoAttachment(attachment);
 
-          if (isImage || isVideo) {
+          if (isImage || isVideo || attachment.pending) {
             const clickCallback =
               attachments.length > 1 ? onClickAttachment : undefined;
 

ts/linkPreviews/shouldUseFullSizeLinkPreviewImage.ts

@@ -10,7 +10,7 @@ export function shouldUseFullSizeLinkPreviewImage({
   isStickerPack,
   image,
 }: Readonly<LinkPreviewType>): boolean {
-  if (isStickerPack || !isImageAttachment(image)) {
+  if (isStickerPack || !image || !isImageAttachment(image)) {
     return false;
   }
 

ts/types/Attachment.ts

@@ -626,9 +626,7 @@ export function isImage(attachments?: Array<AttachmentType>): boolean {
   );
 }
 
-export function isImageAttachment(
-  attachment?: AttachmentType
-): attachment is AttachmentType {
+export function isImageAttachment(attachment?: AttachmentType): boolean {
   return Boolean(
     attachment &&
       attachment.contentType &&
@@ -640,7 +638,9 @@ export function canBeTranscoded(
   attachment?: AttachmentType
 ): attachment is AttachmentType {
   return Boolean(
-    isImageAttachment(attachment) && !MIME.isGif(attachment.contentType)
+    attachment &&
+      isImageAttachment(attachment) &&
+      !MIME.isGif(attachment.contentType)
   );
 }
 

ts/util/lint/exceptions.json

@@ -13295,13 +13295,6 @@
     "reasonCategory": "usageTrusted",
     "updated": "2021-08-23T18:39:37.081Z"
   },
-  {
-    "rule": "React-useRef",
-    "path": "ts/components/Lightbox.tsx",
-    "line": "  const focusRef = useRef<HTMLDivElement | null>(null);",
-    "reasonCategory": "usageTrusted",
-    "updated": "2021-08-23T18:39:37.081Z"
-  },
   {
     "rule": "React-createRef",
     "path": "ts/components/MainHeader.js",

ts/views/conversation_view.ts

@@ -3084,6 +3084,11 @@ export class ConversationView extends window.Backbone.View<ConversationModel> {
       };
     };
 
+    if (this.lightboxView) {
+      this.lightboxView.remove();
+      this.lightboxView = undefined;
+    }
+
     this.lightboxView = new Whisper.ReactWrapperView({
       className: 'lightbox-wrapper',
       Component: window.Signal.Components.Lightbox,
@@ -3206,6 +3211,11 @@ export class ConversationView extends window.Backbone.View<ConversationModel> {
         mediaItem.attachment.path === selectedMediaItem.attachment.path
     );
 
+    if (this.lightboxView) {
+      this.lightboxView.remove();
+      this.lightboxView = undefined;
+    }
+
     this.lightboxView = new Whisper.ReactWrapperView({
       className: 'lightbox-wrapper',
       Component: window.Signal.Components.Lightbox,