From b0b1e9b8e34db357d48a3397190a5248cedaa7a6 Mon Sep 17 00:00:00 2001
From: Scott Nonnenberg <scott@signal.org>
Date: Wed, 1 Aug 2018 15:40:13 -0700
Subject: [PATCH] Message: video/image attachment borders inline, on top of
 image

Otherwise, its transparency will result in a distracting line between it
and its attached message bubble
---
 stylesheets/_modules.scss              | 68 ++++++++++++++++++--------
 stylesheets/_theme_dark.scss           |  7 ++-
 ts/components/conversation/Message.tsx | 50 +++++++++++++------
 3 files changed, 88 insertions(+), 37 deletions(-)

diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss
index 605be3ad6..1371119a9 100644
--- a/stylesheets/_modules.scss
+++ b/stylesheets/_modules.scss
@@ -184,26 +184,64 @@
   text-align: center;
   position: relative;
   cursor: pointer;
-}
 
-.module-message__img-attachment {
-  max-width: 386px;
   margin-left: -12px;
   margin-right: -12px;
   margin-top: -10px;
-  // The padding on the bottom of the bubble produces three extra pixels of space at the
-  //   bottom, so this doesn't match up with the padding numbers above.
-  margin-bottom: -13px;
+  margin-bottom: -10px;
+
   border-radius: 16px;
-  border: 1px solid $color-black-02;
+  overflow: hidden;
+
   background-color: $color-white;
+}
+
+.module-message__attachment-container--with-content-below {
+  margin-bottom: 7px;
+  border-bottom-left-radius: 0px;
+  border-bottom-right-radius: 0px;
+}
+
+.module-message__attachment-container--with-content-above {
+  margin-top: 4px;
+  border-top-left-radius: 0px;
+  border-top-right-radius: 0px;
+}
+
+.module-message__img-border-overlay {
+  position: absolute;
+  top: 0;
+  bottom: 0;
+  z-index: 1;
+  left: 0;
+  right: 0;
+  border-radius: 16px;
+  box-shadow: inset 0px 0px 0px 1px $color-black-02;
+}
+
+.module-message__img-border-overlay--with-content-below {
+  border-bottom-left-radius: 0px;
+  border-bottom-right-radius: 0px;
+}
+
+.module-message__img-border-overlay--with-content-above {
+  border-top-left-radius: 0px;
+  border-top-right-radius: 0px;
+}
+
+.module-message__img-attachment {
+  max-width: 386px;
 
   object-fit: cover;
-  width: calc(100% + 24px);
+  width: 100%;
   min-width: 200px;
   min-height: 150px;
   max-height: 300px;
 
+  // The padding on the bottom of the bubble produces three extra pixels of space at the
+  //   bottom, so this doesn't match up with the padding numbers above.
+  margin-bottom: -3px;
+
   // redundant with attachment-container, but we get cursor flashing on move otherwise
   cursor: pointer;
 }
@@ -230,7 +268,7 @@
   );
   position: absolute;
   bottom: 0;
-  z-index: 1;
+  z-index: 2;
   left: 0;
   right: 0;
   margin-left: -12px;
@@ -240,18 +278,6 @@
   border-bottom-right-radius: 16px;
 }
 
-.module-message__img-attachment--with-content-below {
-  margin-bottom: 5px;
-  border-bottom-left-radius: 0px;
-  border-bottom-right-radius: 0px;
-}
-
-.module-message__img-attachment--with-content-above {
-  margin-top: 4px;
-  border-top-left-radius: 0px;
-  border-top-right-radius: 0px;
-}
-
 .module-message__broken-image {
   font-size: 11px;
   line-height: 16px;
diff --git a/stylesheets/_theme_dark.scss b/stylesheets/_theme_dark.scss
index b53e6e258..8961ea9fd 100644
--- a/stylesheets/_theme_dark.scss
+++ b/stylesheets/_theme_dark.scss
@@ -626,11 +626,14 @@ body.dark-theme {
     background-color: $color-conversation-teal;
   }
 
-  .module-message__img-attachment {
-    border: 1px solid $color-white-02;
+  .module-message__attachment-container {
     background-color: $color-black;
   }
 
+  .module-message__img-border-overlay {
+    box-shadow: inset 0px 0px 0px 1px rgba($color-dark-30, 0.25);
+  }
+
   .module-message__img-overlay {
     background-image: linear-gradient(
       to bottom,
diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx
index 477cafa5d..619fb8474 100644
--- a/ts/components/conversation/Message.tsx
+++ b/ts/components/conversation/Message.tsx
@@ -395,22 +395,33 @@ export class Message extends React.Component<Props, State> {
         <div
           onClick={onClickAttachment}
           role="button"
-          className="module-message__attachment-container"
+          className={classNames(
+            'module-message__attachment-container',
+            withCaption
+              ? 'module-message__attachment-container--with-content-below'
+              : null,
+            withContentAbove
+              ? 'module-message__attachment-container--with-content-above'
+              : null
+          )}
         >
           <img
             onError={this.handleImageErrorBound}
+            className="module-message__img-attachment"
+            height={Math.min(MAXIMUM_IMG_HEIGHT, height)}
+            src={attachment.url}
+            alt={i18n('imageAttachmentAlt')}
+          />
+          <div
             className={classNames(
-              'module-message__img-attachment',
+              'module-message__img-border-overlay',
               withCaption
-                ? 'module-message__img-attachment--with-content-below'
+                ? 'module-message__img-border-overlay--with-content-below'
                 : null,
               withContentAbove
-                ? 'module-message__img-attachment--with-content-above'
+                ? 'module-message__img-border-overlay--with-content-above'
                 : null
             )}
-            height={Math.min(MAXIMUM_IMG_HEIGHT, height)}
-            src={attachment.url}
-            alt={i18n('imageAttachmentAlt')}
           />
           {!withCaption && !collapseMetadata ? (
             <div className="module-message__img-overlay" />
@@ -441,22 +452,33 @@ export class Message extends React.Component<Props, State> {
         <div
           onClick={onClickAttachment}
           role="button"
-          className="module-message__attachment-container"
+          className={classNames(
+            'module-message__attachment-container',
+            withCaption
+              ? 'module-message__attachment-container--with-content-below'
+              : null,
+            withContentAbove
+              ? 'module-message__attachment-container--with-content-above'
+              : null
+          )}
         >
           <img
             onError={this.handleImageErrorBound}
+            className="module-message__img-attachment"
+            alt={i18n('videoAttachmentAlt')}
+            height={Math.min(MAXIMUM_IMG_HEIGHT, height)}
+            src={screenshot.url}
+          />
+          <div
             className={classNames(
-              'module-message__img-attachment',
+              'module-message__img-border-overlay',
               withCaption
-                ? 'module-message__img-attachment--with-content-below'
+                ? 'module-message__img-border-overlay--with-content-below'
                 : null,
               withContentAbove
-                ? 'module-message__img-attachment--with-content-above'
+                ? 'module-message__img-border-overlay--with-content-above'
                 : null
             )}
-            alt={i18n('videoAttachmentAlt')}
-            height={Math.min(MAXIMUM_IMG_HEIGHT, height)}
-            src={screenshot.url}
           />
           {!withCaption && !collapseMetadata ? (
             <div className="module-message__img-overlay" />