Formatting: Expand exceptions to multi-newline ops, multiple ops

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/quill/formatting/menu.tsx

@@ -3,11 +3,13 @@
 
 import type Quill from 'quill';
 import type { KeyboardContext } from 'quill';
+import type Op from 'quill-delta/dist/Op';
 import React from 'react';
 import classNames from 'classnames';
 import { Popper } from 'react-popper';
 import { createPortal } from 'react-dom';
 import type { VirtualElement } from '@popperjs/core';
+import { isString } from 'lodash';
 
 import * as log from '../../logging/log';
 import * as Errors from '../../types/errors';
@@ -54,6 +56,14 @@ function getMetaKey(platform: string, i18n: LocalizerType) {
   return i18n('icu:Keyboard--Key--ctrl');
 }
 
+function isAllNewlines(ops: Array<Op>): boolean {
+  return ops.every(isNewlineOnlyOp);
+}
+
+export function isNewlineOnlyOp(op: Op): boolean {
+  return isString(op.insert) && /^\n+$/gm.test(op.insert);
+}
+
 export class FormattingMenu {
   // Cache the results of our virtual elements's last rect calculation
   lastRect: DOMRect | undefined;
@@ -181,12 +191,12 @@ export class FormattingMenu {
       return;
     }
 
-    // Note: we special-case single \n ops because Quill doesn't apply formatting to them
+    // Note: we special-case all-newline ops because Quill doesn't apply styles to them
     const contents = this.quill.getContents(
       quillSelection.index,
       quillSelection.length
     );
-    if (contents.length() === 1 && contents.ops[0].insert === '\n') {
+    if (isAllNewlines(contents.ops)) {
       this.scheduleRemoval();
       return;
     }
@@ -261,13 +271,12 @@ export class FormattingMenu {
     const contents = this.quill.getContents(selection.index, selection.length);
 
     // Note: we special-case single \n ops because Quill doesn't apply formatting to them
-    if (contents.length() === 1 && contents.ops[0].insert === '\n') {
-      this.scheduleRemoval();
+    if (isAllNewlines(contents.ops)) {
       return false;
     }
 
     return contents.ops.every(
-      op => op.attributes?.[style] || op.insert === '\n'
+      op => op.attributes?.[style] || isNewlineOnlyOp(op)
     );
   }
 

ts/quill/util.ts

@@ -13,7 +13,7 @@ import type {
 } from '../types/BodyRange';
 import { BodyRange } from '../types/BodyRange';
 import type { MentionBlot } from './mentions/blot';
-import { QuillFormattingStyle } from './formatting/menu';
+import { isNewlineOnlyOp, QuillFormattingStyle } from './formatting/menu';
 import { isNotNil } from '../util/isNotNil';
 
 export type MentionBlotValue = {
@@ -165,8 +165,8 @@ export const getTextAndRangesFromOps = (
   };
 
   const preTrimText = ops.reduce((acc, op) => {
-    // We special-case single-newline ops because Quill doesn't apply styles to them
-    if (op.insert === '\n') {
+    // We special-case all-newline ops because Quill doesn't apply styles to them
+    if (isNewlineOnlyOp(op)) {
       return acc + op.insert;
     }
 

ts/test-node/quill/util_test.ts

@@ -125,6 +125,43 @@ describe('getTextAndRangesFromOps', () => {
       ]);
     });
 
+    it('handles trimming with no-formatting single- and multi-newline ops', () => {
+      const ops = [
+        {
+          insert: 'line1',
+          attributes: { bold: true },
+        },
+        // quill doesn't put formatting on all-newline ops
+        {
+          insert: '\n',
+        },
+        {
+          insert: 'line2',
+          attributes: { bold: true },
+        },
+        {
+          insert: '\n\n',
+        },
+        {
+          insert: 'line4',
+          attributes: { bold: true },
+        },
+        {
+          insert: '\n\n',
+        },
+      ];
+      const { text, bodyRanges } = getTextAndRangesFromOps(ops);
+      assert.equal(text, 'line1\nline2\n\nline4');
+      assert.equal(bodyRanges.length, 1);
+      assert.deepEqual(bodyRanges, [
+        {
+          start: 0,
+          length: 18,
+          style: BodyRange.Style.BOLD,
+        },
+      ]);
+    });
+
     it('handles trimming at the end of the message', () => {
       const ops = [
         {