From 90c2a97aa7139fb59464039f61e27fae23971edd Mon Sep 17 00:00:00 2001
From: Scott Nonnenberg <scott@signal.org>
Date: Tue, 20 Aug 2019 12:34:52 -0700
Subject: [PATCH] Address alpha feedback

---
 js/views/conversation_view.js           | 11 +++++-----
 js/views/inbox_view.js                  |  8 +++++++
 package.json                            |  3 ++-
 stylesheets/_modules.scss               | 10 ++++++++-
 ts/components/MainHeader.tsx            | 15 +++++++------
 ts/components/conversation/Timeline.tsx | 15 ++++++-------
 ts/state/ducks/conversations.ts         | 24 ++++++++++++++++++---
 ts/state/selectors/search.ts            |  6 +++---
 ts/util/lint/exceptions.json            | 28 ++++++++++++-------------
 9 files changed, 78 insertions(+), 42 deletions(-)

diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js
index 387c300bff..cc1c833bf7 100644
--- a/js/views/conversation_view.js
+++ b/js/views/conversation_view.js
@@ -186,7 +186,7 @@
         this.maybeGrabLinkPreview.bind(this),
         200
       );
-      this.debouncedSaveDraft = _.debounce(this.saveDraft.bind(this), 200);
+      this.debouncedSaveDraft = _.debounce(this.saveDraft.bind(this), 2000);
 
       this.render();
 
@@ -2149,6 +2149,7 @@
         try {
           this.unload('delete messages');
           await this.model.destroyMessages();
+          Whisper.events.trigger('unloadConversation', this.model.id);
           this.model.updateLastMessage();
         } catch (error) {
           window.log.error(
@@ -2421,10 +2422,10 @@
     },
 
     async saveDraft(messageText) {
-      if (
-        (this.model.get('draft') && !messageText) ||
-        messageText.length === 0
-      ) {
+      const trimmed =
+        messageText && messageText.length > 0 ? messageText.trim() : '';
+
+      if ((this.model.get('draft') && !messageText) || trimmed.length === 0) {
         this.model.set({
           draft: null,
         });
diff --git a/js/views/inbox_view.js b/js/views/inbox_view.js
index b2f47b5ecb..e39ec58754 100644
--- a/js/views/inbox_view.js
+++ b/js/views/inbox_view.js
@@ -47,6 +47,11 @@
       // Make sure poppers are positioned properly
       window.dispatchEvent(new Event('resize'));
     },
+    onUnload(conversationId) {
+      if (this.lastConversation.id === conversationId) {
+        this.lastConversation = null;
+      }
+    },
   });
 
   Whisper.AppLoadingScreen = Whisper.View.extend({
@@ -75,6 +80,9 @@
         el: this.$('.conversation-stack'),
         model: { window: options.window },
       });
+      Whisper.events.on('unloadConversation', conversationId => {
+        this.conversation_stack.onUnload(conversationId);
+      });
 
       if (!options.initialLoadComplete) {
         this.appLoadingScreen = new Whisper.AppLoadingScreen();
diff --git a/package.json b/package.json
index fdf3ebff34..71ce341ba2 100644
--- a/package.json
+++ b/package.json
@@ -202,7 +202,8 @@
         }
       ],
       "target": [
-        "zip", "dmg"
+        "zip",
+        "dmg"
       ],
       "bundleVersion": "1"
     },
diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss
index dcb3a37779..9e52d4af95 100644
--- a/stylesheets/_modules.scss
+++ b/stylesheets/_modules.scss
@@ -2410,7 +2410,7 @@
   border: solid 1px $color-gray-15;
 
   padding-left: 30px;
-  padding-right: 30px;
+  padding-right: 5px;
 
   color: $color-gray-90;
   font-size: 14px;
@@ -2425,6 +2425,10 @@
   }
 }
 
+.module-main-header__search__input--with-text {
+  padding-right: 30px;
+}
+
 .module-main-header__search__input--in-conversation {
   padding-left: 50px;
 }
@@ -2453,6 +2457,10 @@
   display: flex;
   flex-direction: row;
   align-items: center;
+
+  // Overriding some default button styling
+  border: none;
+  padding: 0;
 }
 .module-main-header__search__in-conversation-pill__avatar-container {
   margin-left: 4px;
diff --git a/ts/components/MainHeader.tsx b/ts/components/MainHeader.tsx
index f9a1cc4976..fc7a8d4c53 100644
--- a/ts/components/MainHeader.tsx
+++ b/ts/components/MainHeader.tsx
@@ -197,15 +197,15 @@ export class MainHeader extends React.Component<PropsType> {
         />
         <div className="module-main-header__search">
           {searchConversationId ? (
-            <div className="module-main-header__search__in-conversation-pill">
+            <button
+              className="module-main-header__search__in-conversation-pill"
+              onClick={this.clearSearch}
+            >
               <div className="module-main-header__search__in-conversation-pill__avatar-container">
                 <div className="module-main-header__search__in-conversation-pill__avatar" />
               </div>
-              <button
-                className="module-main-header__search__in-conversation-pill__x-button"
-                onClick={this.clearSearch}
-              />
-            </div>
+              <div className="module-main-header__search__in-conversation-pill__x-button" />
+            </button>
           ) : (
             <button
               className="module-main-header__search__icon"
@@ -217,6 +217,9 @@ export class MainHeader extends React.Component<PropsType> {
             ref={this.inputRef}
             className={classNames(
               'module-main-header__search__input',
+              searchTerm
+                ? 'module-main-header__search__input--with-text'
+                : null,
               searchConversationId
                 ? 'module-main-header__search__input--in-conversation'
                 : null
diff --git a/ts/components/conversation/Timeline.tsx b/ts/components/conversation/Timeline.tsx
index c0211a3e29..eba70f23ee 100644
--- a/ts/components/conversation/Timeline.tsx
+++ b/ts/components/conversation/Timeline.tsx
@@ -578,10 +578,7 @@ export class Timeline extends React.PureComponent<Props, State> {
   public getRowCount() {
     const { haveOldest, oldestUnreadIndex, typingContact } = this.props;
     const { items } = this.props;
-
-    if (!items || items.length < 1) {
-      return 0;
-    }
+    const itemsCount = items && items.length ? items.length : 0;
 
     let extraRows = 0;
 
@@ -597,7 +594,7 @@ export class Timeline extends React.PureComponent<Props, State> {
       extraRows += 1;
     }
 
-    return items.length + extraRows;
+    return itemsCount + extraRows;
   }
 
   public fromRowToItemIndex(row: number): number | undefined {
@@ -848,13 +845,13 @@ export class Timeline extends React.PureComponent<Props, State> {
       areUnreadBelowCurrentPosition,
     } = this.state;
 
-    if (!items || items.length < 1) {
-      return null;
-    }
-
     const rowCount = this.getRowCount();
     const scrollToIndex = this.getScrollTarget();
 
+    if (!items || rowCount === 0) {
+      return null;
+    }
+
     return (
       <div className="module-timeline">
         <AutoSizer>
diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts
index 898eadba5e..69f761133d 100644
--- a/ts/state/ducks/conversations.ts
+++ b/ts/state/ducks/conversations.ts
@@ -11,6 +11,11 @@ import {
 } from 'lodash';
 import { trigger } from '../../shims/events';
 import { NoopActionType } from './noop';
+import {
+  AttachmentType,
+  isImageAttachment,
+  isVideoAttachment,
+} from '../../types/attachment';
 
 // State
 
@@ -53,6 +58,7 @@ export type MessageType = {
   quote?: { author: string };
   received_at: number;
   hasSignalAccount?: boolean;
+  attachments: Array<AttachmentType>;
 
   // No need to go beyond this; unused at this stage, since this goes into
   //   a reducer still in plain JavaScript and comes out well-formed
@@ -511,10 +517,22 @@ function hasMessageHeightChanged(
   message: MessageType,
   previous: MessageType
 ): Boolean {
-  return (
+  const visualAttachmentNoLongerPending =
+    previous.attachments &&
+    previous.attachments[0] &&
+    previous.attachments[0].pending &&
+    message.attachments &&
+    message.attachments.length === 1 &&
+    message.attachments[0] &&
+    (isImageAttachment(message.attachments[0]) ||
+      isVideoAttachment(message.attachments[0])) &&
+    !message.attachments[0].pending;
+
+  const signalAccountChanged =
     Boolean(message.hasSignalAccount || previous.hasSignalAccount) &&
-    message.hasSignalAccount !== previous.hasSignalAccount
-  );
+    message.hasSignalAccount !== previous.hasSignalAccount;
+
+  return visualAttachmentNoLongerPending || signalAccountChanged;
 }
 
 // tslint:disable-next-line cyclomatic-complexity max-func-body-length
diff --git a/ts/state/selectors/search.ts b/ts/state/selectors/search.ts
index e014a5691c..a288694409 100644
--- a/ts/state/selectors/search.ts
+++ b/ts/state/selectors/search.ts
@@ -53,9 +53,9 @@ export const getSearchConversationName = createSelector(
 export const isSearching = createSelector(
   getSearch,
   (state: SearchStateType) => {
-    const { query, searchConversationId } = state;
+    const { query } = state;
 
-    return (query && query.trim().length > 1) || searchConversationId;
+    return query && query.trim().length > 1;
   }
 );
 
@@ -71,7 +71,7 @@ export const getSearchResults = createSelector(
     regionCode: string,
     lookup: ConversationLookupType,
     selectedConversation?: string
-  ): SearchResultsPropsType => {
+  ): SearchResultsPropsType | undefined => {
     const {
       contacts,
       conversations,
diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json
index 4775cc5744..3bb19dffb1 100644
--- a/ts/util/lint/exceptions.json
+++ b/ts/util/lint/exceptions.json
@@ -486,7 +486,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "        this.$('.message').text(message);",
-    "lineNumber": 58,
+    "lineNumber": 63,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Hardcoded selector"
@@ -495,7 +495,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "        el: this.$('.conversation-stack'),",
-    "lineNumber": 75,
+    "lineNumber": 80,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Hardcoded selector"
@@ -504,7 +504,7 @@
     "rule": "jQuery-prependTo(",
     "path": "js/views/inbox_view.js",
     "line": "        this.appLoadingScreen.$el.prependTo(this.el);",
-    "lineNumber": 82,
+    "lineNumber": 90,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -513,7 +513,7 @@
     "rule": "jQuery-append(",
     "path": "js/views/inbox_view.js",
     "line": "        .append(this.networkStatusView.render().el);",
-    "lineNumber": 99,
+    "lineNumber": 107,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -522,7 +522,7 @@
     "rule": "jQuery-prependTo(",
     "path": "js/views/inbox_view.js",
     "line": "        banner.$el.prependTo(this.$el);",
-    "lineNumber": 103,
+    "lineNumber": 111,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -531,7 +531,7 @@
     "rule": "jQuery-appendTo(",
     "path": "js/views/inbox_view.js",
     "line": "        toast.$el.appendTo(this.$el);",
-    "lineNumber": 109,
+    "lineNumber": 117,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -540,7 +540,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      this.$('.left-pane-placeholder').append(this.leftPaneView.el);",
-    "lineNumber": 129,
+    "lineNumber": 137,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -549,7 +549,7 @@
     "rule": "jQuery-append(",
     "path": "js/views/inbox_view.js",
     "line": "      this.$('.left-pane-placeholder').append(this.leftPaneView.el);",
-    "lineNumber": 129,
+    "lineNumber": 137,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -558,7 +558,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      if (e && this.$(e.target).closest('.placeholder').length) {",
-    "lineNumber": 172,
+    "lineNumber": 180,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -567,7 +567,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      this.$('#header, .gutter').addClass('inactive');",
-    "lineNumber": 176,
+    "lineNumber": 184,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Hardcoded selector"
@@ -576,7 +576,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      this.$('.conversation-stack').addClass('inactive');",
-    "lineNumber": 180,
+    "lineNumber": 188,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Hardcoded selector"
@@ -585,7 +585,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      this.$('.conversation:first .menu').trigger('close');",
-    "lineNumber": 182,
+    "lineNumber": 190,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Hardcoded selector"
@@ -594,7 +594,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      if (e && this.$(e.target).closest('.capture-audio').length > 0) {",
-    "lineNumber": 202,
+    "lineNumber": 210,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Known DOM elements"
@@ -603,7 +603,7 @@
     "rule": "jQuery-$(",
     "path": "js/views/inbox_view.js",
     "line": "      this.$('.conversation:first .recorder').trigger('close');",
-    "lineNumber": 205,
+    "lineNumber": 213,
     "reasonCategory": "usageTrusted",
     "updated": "2019-07-31T00:19:18.696Z",
     "reasonDetail": "Hardcoded selector"