Address beta feedback

* Use signal blue for search box focus outline * Reduce debounce for draft saves * Be less aggressive in our scrolling corrections * Lightbox: Ensure that a tall image is still fully visible * Fix spell checking after Electron API breaking changes * Fix link preview image generation * Message highlight: Move to border in signal blue
2019-08-22 14:11:36 -07:00 · 2019-08-22 14:11:36 -07:00 · b19659f5ac
parent eec0fce62a
commit b19659f5ac
9 changed files with 108 additions and 89 deletions
--- a/js/spell_check.js
+++ b/js/spell_check.js
@ -96,11 +96,12 @@ if (process.platform === 'linux') {
 }

 const simpleChecker = {
-  spellCheck(text) {
-    return !this.isMisspelled(text);
+  spellCheck(words, callback) {
+    const mispelled = words.filter(word => this.isMisspelled(word));
+    callback(mispelled);
  },
-  isMisspelled(text) {
-    const misspelled = spellchecker.isMisspelled(text);
+  isMisspelled(word) {
+    const misspelled = spellchecker.isMisspelled(word);

    // The idea is to make this as fast as possible. For the many, many calls which
    //   don't result in the red squiggly, we minimize the number of checks.
@ -109,7 +110,7 @@ const simpleChecker = {
    }

    // Only if we think we've found an error do we check the locale and skip list.
-    if (locale.match(EN_VARIANT) && _.contains(ENGLISH_SKIP_WORDS, text)) {
+    if (locale.match(EN_VARIANT) && _.contains(ENGLISH_SKIP_WORDS, word)) {
      return false;
    }

@ -118,14 +119,14 @@ const simpleChecker = {
  getSuggestions(text) {
    return spellchecker.getCorrectionsForMisspelling(text);
  },
-  add(text) {
-    spellchecker.add(text);
+  add(word) {
+    spellchecker.add(word);
  },
 };

 const dummyChecker = {
-  spellCheck() {
-    return true;
+  spellCheck(words, callback) {
+    callback([]);
  },
  isMisspelled() {
    return false;
--- 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), 2000);
+      this.debouncedSaveDraft = _.debounce(this.saveDraft.bind(this), 500);

      this.render();

@ -2668,18 +2668,18 @@
            );
          }

-          const data = await this.makeChunkedRequest(imageUrl);
+          const chunked = await this.makeChunkedRequest(imageUrl);

          // Ensure that this file is either small enough or is resized to meet our
          //   requirements for attachments
          const withBlob = await this.autoScale({
-            contentType: data.contentType,
-            file: new Blob([data.data], {
-              type: data.contentType,
+            contentType: chunked.contentType,
+            file: new Blob([chunked.data], {
+              type: chunked.contentType,
            }),
          });

-          const attachment = await this.arrayBufferFromFile(withBlob);
+          const data = await this.arrayBufferFromFile(withBlob.file);
          objectUrl = URL.createObjectURL(withBlob.file);

          const dimensions = await Signal.Types.VisualAttachment.getImageDimensions(
@ -2690,7 +2690,8 @@
          );

          image = {
-            ...attachment,
+            data,
+            size: data.byteLength,
            ...dimensions,
            contentType: withBlob.file.type,
          };
--- a/patches/react-virtualized+9.21.0.patch
+++ b/patches/react-virtualized+9.21.0.patch
@ -24,7 +24,7 @@ index d9716a0..e7a9f9f 100644
         }
       }
 diff --git a/node_modules/react-virtualized/dist/commonjs/Grid/Grid.js b/node_modules/react-virtualized/dist/commonjs/Grid/Grid.js
-index e1b959a..1e3a269 100644
+index e1b959a..18f8f1d 100644
 --- a/node_modules/react-virtualized/dist/commonjs/Grid/Grid.js
 +++ b/node_modules/react-virtualized/dist/commonjs/Grid/Grid.js
@@ -132,6 +132,9 @@ var Grid = function (_React$PureComponent) {
@ -104,11 +104,27 @@ index e1b959a..1e3a269 100644
       });
 
       this._maybeCallOnScrollbarPresenceChange();
-@@ -584,6 +611,51 @@ var Grid = function (_React$PureComponent) {
+@@ -584,6 +611,59 @@ var Grid = function (_React$PureComponent) {
         }
       }
 
-+      if (scrollToColumn >= 0) {
+      // We reset our hasScrolled flags if our target has changed, or if target is not longer set
+      if (scrollToColumn !== prevProps.scrollToColumn || scrollToColumn == null || scrollToColumn < 0) {
+        this._hasScrolledToColumnTarget = false;
+      }
+      if (scrollToRow !== prevProps.scrollToRow || scrollToRow == null || scrollToRow < 0) {
+        this._hasScrolledToRowTarget = false;
+      }
+
+      // We deactivate our forced scrolling if the user scrolls
+      if (scrollLeft !== prevState.scrollLeft && scrollToColumn >= 0 && scrollPositionChangeReason === SCROLL_POSITION_CHANGE_REASONS.OBSERVED) {
+        this._hasScrolledToColumnTarget = true;
+      }
+      if (scrollTop !== prevState.scrollTop && scrollToRow >= 0 && scrollPositionChangeReason === SCROLL_POSITION_CHANGE_REASONS.OBSERVED) {
+        this._hasScrolledToRowTarget = true;
+      }
+
+      if (scrollToColumn >= 0 && !this._hasScrolledToColumnTarget) {
 +        const scrollRight = scrollLeft + width;
 +        const targetColumn = instanceProps.columnSizeAndPositionManager.getSizeAndPositionOfCell(scrollToColumn);
 +
@ -124,13 +140,9 @@ index e1b959a..1e3a269 100644
 +          const totalColumnsWidth = instanceProps.columnSizeAndPositionManager.getTotalSize();
 +          const maxScroll = totalColumnsWidth - width;
 +          this._hasScrolledToColumnTarget = (scrollLeft >= maxScroll || targetColumn.offset === scrollLeft);
-+        } else if (scrollToColumn !== prevProps.scrollToColumn) {
-+          this._hasScrolledToColumnTarget = false;
 +        }
-+      } else {
-+        this._hasScrolledToColumnTarget = false;
 +      }
-+      if (scrollToRow >= 0) {
+      if (scrollToRow >= 0 && !this._hasScrolledToRowTarget) {
 +        const scrollBottom = scrollTop + height;
 +        const targetRow = instanceProps.rowSizeAndPositionManager.getSizeAndPositionOfCell(scrollToRow);
 +
@ -146,17 +158,13 @@ index e1b959a..1e3a269 100644
 +          const totalRowsHeight = instanceProps.rowSizeAndPositionManager.getTotalSize();
 +          const maxScroll = totalRowsHeight - height;
 +          this._hasScrolledToRowTarget = (scrollTop >= maxScroll || targetRow.offset === scrollTop);
-+        } else if (scrollToRow !== prevProps.scrollToRow) {
-+          this._hasScrolledToRowTarget = false;
 +        }
-+      } else {
-+        this._hasScrolledToRowTarget = false;
 +      }
 +
       // Special case where the previous size was 0:
       // In this case we don't show any windowed cells at all.
       // So we should always recalculate offset afterwards.
-@@ -594,6 +666,8 @@ var Grid = function (_React$PureComponent) {
+@@ -594,6 +674,8 @@ var Grid = function (_React$PureComponent) {
       if (this._recomputeScrollLeftFlag) {
         this._recomputeScrollLeftFlag = false;
         this._updateScrollLeftForScrollToColumn(this.props);
@ -165,7 +173,7 @@ index e1b959a..1e3a269 100644
       } else {
         (0, _updateScrollIndexHelper2.default)({
           cellSizeAndPositionManager: instanceProps.columnSizeAndPositionManager,
-@@ -616,6 +690,8 @@ var Grid = function (_React$PureComponent) {
+@@ -616,6 +698,8 @@ var Grid = function (_React$PureComponent) {
       if (this._recomputeScrollTopFlag) {
         this._recomputeScrollTopFlag = false;
         this._updateScrollTopForScrollToRow(this.props);
@ -174,7 +182,7 @@ index e1b959a..1e3a269 100644
       } else {
         (0, _updateScrollIndexHelper2.default)({
           cellSizeAndPositionManager: instanceProps.rowSizeAndPositionManager,
-@@ -630,11 +706,56 @@ var Grid = function (_React$PureComponent) {
+@@ -630,11 +714,56 @@ var Grid = function (_React$PureComponent) {
           size: height,
           sizeJustIncreasedFromZero: sizeJustIncreasedFromZero,
           updateScrollIndexCallback: function updateScrollIndexCallback() {
@ -232,7 +240,7 @@ index e1b959a..1e3a269 100644
       // Update onRowsRendered callback if start/stop indices have changed
       this._invokeOnGridRenderedHelper();
 
-@@ -647,7 +768,11 @@ var Grid = function (_React$PureComponent) {
+@@ -647,7 +776,11 @@ var Grid = function (_React$PureComponent) {
           scrollLeft: scrollLeft,
           scrollTop: scrollTop,
           totalColumnsWidth: totalColumnsWidth,
@ -245,7 +253,7 @@ index e1b959a..1e3a269 100644
         });
       }
 
-@@ -962,7 +1087,11 @@ var Grid = function (_React$PureComponent) {
+@@ -962,7 +1095,11 @@ var Grid = function (_React$PureComponent) {
       var scrollLeft = _ref6.scrollLeft,
           scrollTop = _ref6.scrollTop,
           totalColumnsWidth = _ref6.totalColumnsWidth,
@ -258,7 +266,7 @@ index e1b959a..1e3a269 100644
 
       this._onScrollMemoizer({
         callback: function callback(_ref7) {
-@@ -973,19 +1102,26 @@ var Grid = function (_React$PureComponent) {
+@@ -973,19 +1110,26 @@ var Grid = function (_React$PureComponent) {
               onScroll = _props7.onScroll,
               width = _props7.width;
 
@ -289,7 +297,7 @@ index e1b959a..1e3a269 100644
       });
     }
 diff --git a/node_modules/react-virtualized/dist/commonjs/Grid/accessibilityOverscanIndicesGetter.js b/node_modules/react-virtualized/dist/commonjs/Grid/accessibilityOverscanIndicesGetter.js
-index 70b0abe..2f04448 100644
+index 70b0abe..8e12ffc 100644
 --- a/node_modules/react-virtualized/dist/commonjs/Grid/accessibilityOverscanIndicesGetter.js
 +++ b/node_modules/react-virtualized/dist/commonjs/Grid/accessibilityOverscanIndicesGetter.js
@@ -32,15 +32,8 @@ function defaultOverscanIndicesGetter(_ref) {
@ -312,8 +320,9 @@ index 70b0abe..2f04448 100644
 +    overscanStopIndex: Math.min(cellCount - 1, stopIndex + overscanCellsCount),
 +  };
 }
+\ No newline at end of file
 diff --git a/node_modules/react-virtualized/dist/commonjs/Grid/defaultOverscanIndicesGetter.js b/node_modules/react-virtualized/dist/commonjs/Grid/defaultOverscanIndicesGetter.js
-index d5f6d04..e01e69a 100644
+index d5f6d04..c4b3d84 100644
 --- a/node_modules/react-virtualized/dist/commonjs/Grid/defaultOverscanIndicesGetter.js
 +++ b/node_modules/react-virtualized/dist/commonjs/Grid/defaultOverscanIndicesGetter.js
@@ -27,15 +27,8 @@ function defaultOverscanIndicesGetter(_ref) {
@ -336,6 +345,7 @@ index d5f6d04..e01e69a 100644
 +    overscanStopIndex: Math.min(cellCount - 1, stopIndex + overscanCellsCount),
 +  };
 }
+\ No newline at end of file
 diff --git a/node_modules/react-virtualized/dist/commonjs/List/List.js b/node_modules/react-virtualized/dist/commonjs/List/List.js
 index b5ad0eb..efb2cd7 100644
 --- a/node_modules/react-virtualized/dist/commonjs/List/List.js
--- a/stylesheets/_modules.scss
+++ b/stylesheets/_modules.scss
@ -12,7 +12,8 @@
 //   can be used outside tht Timeline contact more easily.
 .module-message-container {
  width: 100%;
-  margin-top: 10px;
+  margin-top: 4px;
+  margin-bottom: 4px;

  &::after {
    visibility: hidden;
@ -65,7 +66,7 @@
 }

 .module-message--incoming.module-message--group {
-  margin-left: 44px;
+  margin-left: 48px;
 }

 .module-message__buttons {
@ -175,6 +176,14 @@
  padding-top: 10px;
  padding-bottom: 10px;
  min-width: 0px;
+
+  @include light-theme {
+    border: 1px solid $color-white;
+  }
+
+  @include dark-theme {
+    border: 1px solid $color-gray-95;
+  }
 }

 .module-message__container--with-sticker {
@ -185,34 +194,23 @@
  background-color: $color-light-10;
 }

-.module-message__container__selection {
-  position: absolute;
-  display: block;
-  top: 0;
-  left: 0;
-  right: 0;
-  bottom: 0;
-
-  border-radius: 16px;
-
-  background-color: $color-black;
-  opacity: 0;
-
-  animation: message--selected 1s ease-in-out;
+.module-message__container--selected {
+  // cubic-bezier taken from https://easings.net/en#easeOutExpo
+  animation: message--selected 1s cubic-bezier(0.19, 1, 0.22, 1);
 }

@keyframes message--selected {
  0% {
-    opacity: 0;
+    box-shadow: 0 0 0 5px transparent;
  }
-  20% {
-    opacity: 0.3;
+  10% {
+    box-shadow: 0 0 0 5px $color-signal-blue;
  }
-  80% {
-    opacity: 0.3;
+  70% {
+    box-shadow: 0 0 0 5px $color-signal-blue;
  }
  100% {
-    opacity: 0;
+    box-shadow: 0 0 0 5px transparent;
  }
 }

@ -806,7 +804,7 @@
 .module-message__author-avatar {
  position: absolute;
  bottom: 0px;
-  right: calc(100% + 4px);
+  right: calc(100% + 8px);
 }

 .module-message__typing-container {
@ -2420,7 +2418,7 @@
  }

  &:focus {
-    border: solid 1px blue;
+    border: solid 1px $color-signal-blue;
    outline: none;
  }
 }
@ -2496,7 +2494,6 @@
 // Module: Image

 .module-image {
-  overflow: hidden;
  position: relative;
  display: inline-block;
  margin: 1px;
@ -2537,6 +2534,18 @@
  border-top-left-radius: 10px;
 }

+.module-image--selection--selected {
+  position: absolute;
+  top: 1px;
+  bottom: 1px;
+  left: 1px;
+  right: 1px;
+  border-radius: 10px;
+
+  // cubic-bezier taken from https://easings.net/en#easeOutExpo
+  animation: message--selected 1s cubic-bezier(0.19, 1, 0.22, 1);
+}
+
 .module-image__border-overlay {
  position: absolute;
  top: 0;
@ -2551,13 +2560,6 @@
  background-color: $color-black-02;
 }

-.module-image__border-overlay--selected {
-  background-color: $color-black;
-  opacity: 0;
-
-  animation: message--selected 1s ease-in-out;
-}
-
 .module-image__loading-placeholder {
  display: inline-flex;
  flex-direction: row;
@ -3504,7 +3506,8 @@
 }

 .module-timeline__message-container {
-  padding-top: 10px;
+  padding-top: 4px;
+  padding-bottom: 4px;
 }

 .ReactVirtualized__List {
--- a/test/spellcheck_test.js
+++ b/test/spellcheck_test.js
@ -1,6 +1,15 @@
 describe('spellChecker', () => {
  it('should work', () => {
-    assert(window.spellChecker.spellCheck('correct'));
-    assert(!window.spellChecker.spellCheck('fhqwgads'));
+    let result = null;
+
+    window.spellChecker.spellCheck(['correct'], answer => {
+      result = answer;
+    });
+    assert.deepEqual(result, []);
+
+    window.spellChecker.spellCheck(['fhqwgads'], answer => {
+      result = answer;
+    });
+    assert.deepEqual(result, ['fhqwgads']);
  });
 });
--- a/ts/components/Lightbox.tsx
+++ b/ts/components/Lightbox.tsx
@ -56,6 +56,8 @@ const styles = {
    paddingLeft: 40,
    paddingRight: 40,
    paddingBottom: 0,
+    // To ensure that a large image doesn't overflow the flex layout
+    minHeight: '50px',
  } as React.CSSProperties,
  objectContainer: {
    position: 'relative',
--- a/ts/components/conversation/Image.tsx
+++ b/ts/components/conversation/Image.tsx
@ -90,6 +90,9 @@ export class Image extends React.Component<Props> {
          softCorners ? 'module-image--soft-corners' : null
        )}
      >
+        {isSelected ? (
+          <div className="module-image--selection--selected" />
+        ) : null}
        {pending ? (
          <div
            className="module-image__loading-placeholder"
@ -120,7 +123,7 @@ export class Image extends React.Component<Props> {
            alt={i18n('imageCaptionIconAlt')}
          />
        ) : null}
-        {!noBorder || isSelected ? (
+        {!noBorder ? (
          <div
            className={classNames(
              'module-image__border-overlay',
@ -130,8 +133,7 @@ export class Image extends React.Component<Props> {
              curveBottomRight ? 'module-image--curved-bottom-right' : null,
              smallCurveTopLeft ? 'module-image--small-curved-top-left' : null,
              softCorners ? 'module-image--soft-corners' : null,
-              darkOverlay ? 'module-image__border-overlay--dark' : null,
-              isSelected ? 'module-image__border-overlay--selected' : null
+              darkOverlay ? 'module-image__border-overlay--dark' : null
            )}
          />
        ) : null}
--- a/ts/components/conversation/Message.tsx
+++ b/ts/components/conversation/Message.tsx
@ -1243,17 +1243,6 @@ export class Message extends React.PureComponent<Props, State> {
    );
  }

-  public renderSelectionHighlight() {
-    const { isSticker } = this.props;
-    const { isSelected } = this.state;
-
-    if (!isSelected || isSticker) {
-      return;
-    }
-
-    return <div className="module-message__container__selection" />;
-  }
-
  // tslint:disable-next-line cyclomatic-complexity
  public render() {
    const {
@ -1270,7 +1259,7 @@ export class Message extends React.PureComponent<Props, State> {
      isTapToViewError,
      timestamp,
    } = this.props;
-    const { expired, expiring, imageBroken } = this.state;
+    const { expired, expiring, imageBroken, isSelected } = this.state;

    const isAttachmentPending = this.isAttachmentPending();
    const isButton = isTapToView && !isTapToViewExpired && !isAttachmentPending;
@ -1306,6 +1295,9 @@ export class Message extends React.PureComponent<Props, State> {
        <div
          className={classNames(
            'module-message__container',
+            isSelected && !isSticker
+              ? 'module-message__container--selected'
+              : null,
            isSticker ? 'module-message__container--with-sticker' : null,
            !isSticker ? `module-message__container--${direction}` : null,
            isTapToView ? 'module-message__container--with-tap-to-view' : null,
@ -1333,7 +1325,6 @@ export class Message extends React.PureComponent<Props, State> {
        >
          {this.renderAuthor()}
          {this.renderContents()}
-          {this.renderSelectionHighlight()}
          {this.renderAvatar()}
        </div>
        {this.renderError(direction === 'outgoing')}
--- a/ts/util/lint/exceptions.json
+++ b/ts/util/lint/exceptions.json
@ -7484,7 +7484,7 @@
    "rule": "React-createRef",
    "path": "ts/components/Lightbox.js",
    "line": "        this.videoRef = react_1.default.createRef();",
-    "lineNumber": 183,
+    "lineNumber": 185,
    "reasonCategory": "usageTrusted",
    "updated": "2019-03-09T00:08:44.242Z",
    "reasonDetail": "Used to auto-start playback on videos"
@ -7493,7 +7493,7 @@
    "rule": "React-createRef",
    "path": "ts/components/Lightbox.tsx",
    "line": "    this.videoRef = React.createRef();",
-    "lineNumber": 179,
+    "lineNumber": 181,
    "reasonCategory": "usageTrusted",
    "updated": "2019-03-09T00:08:44.242Z",
    "reasonDetail": "Used to auto-start playback on videos"