From 192e3bfbc69df346e3aadfdc5bb6ee777f01ed03 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Tue, 22 Oct 2019 11:36:04 -0700 Subject: [PATCH] Replace #1647: Show more than first line in note list preview Replaces #1647 fixes: https://github.com/Automattic/simplenote-electron/issues/235 The regex that captures the note list preview was only selecting the first line. This selects the remainder of the note. That string then gets sliced down to the `previewCharacterLimit`. This PR also decreases the performance cost of creating the title and preview from 1ms to 0.2ms. 1. Load production 2. Have note that has a few characters per line for a couple of lines 3. Have Settings -> Display -> Note Display set to Expanded 4. Observe only the first line of text is shown 5. Do the same on the this branch 6. Observe that multiple lines are shown up to 300 characters 7. This is the same as the Android app **Before:** Screen Shot 2019-10-18 at 10 53 21 AM **After:** Screen Shot 2019-10-18 at 10 53 29 AM Only one developer ais required to review these changes, but anyone can perform the review. `RELEASE-NOTES.txt` was updated with: - Fixed bug that only shows the first line of text in note list preview --- RELEASE-NOTES.txt | 1 + lib/utils/note-utils.js | 96 +++++++++++++++++------------------- lib/utils/note-utils.test.js | 17 +++---- 3 files changed, 53 insertions(+), 61 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index e9c5ee8e5..aecaa9cb5 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -12,6 +12,7 @@ ### Fixes - Rework WordPress.com signin to prevent infinite looping and login failures [#1627](https://github.com/Automattic/simplenote-electron/pull/1627) + - Fixed bug that only shows the first line of text in note list preview [#1647](https://github.com/Automattic/simplenote-electron/pull/1647) - Update link to release-notes in updater config: CHANGELOG -> RELEASE_NOTES ## [v1.9.1] diff --git a/lib/utils/note-utils.js b/lib/utils/note-utils.js index 2d8a25841..808c79345 100644 --- a/lib/utils/note-utils.js +++ b/lib/utils/note-utils.js @@ -1,37 +1,54 @@ import removeMarkdown from 'remove-markdown'; import { isFunction } from 'lodash'; -/** - * Matches the title and excerpt in a note's content - * - * Both the title and the excerpt are determined as - * content starting with something that isn't - * whitespace and leads up to a newline or line end - * - * @type {RegExp} matches a title and excerpt in note content - */ -const noteTitleAndPreviewRegExp = /\s*(\S[^\n]*)\s*(\S[^\n]*)?/; +const maxTitleChars = 64; +const maxPreviewChars = 200; +const maxPreviewLines = 4; // probably need to adjust if we're in the comfy view + +const matchUntilLimit = (pattern, source, preview = '', lines = 0) => { + const match = pattern.exec(source); + // must match, must have no more than four lines, must not be longer than N=10 characters + if (!match || lines > maxPreviewLines || preview.length > maxPreviewChars) { + return preview; + } + + const [, chunk] = match; + return matchUntilLimit(pattern, source, preview + chunk, lines + 1); +}; /** - * Matches the title and excerpt in a note's content skipping opening # from title - * - * Both the title and the excerpt are determined as - * content starting with something that isn't - * whitespace and leads up to a newline or line end + * Returns a string with markdown stripped * - * @type {RegExp} matches a title and excerpt in note content skipping opening #'s from title and preview + * @param {String} inputString string for which to remove markdown + * @returns {String} string with markdown removed */ -const noteTitleAndPreviewMdRegExp = /(?:#{0,6}\s+)?(\S[^\n]*)\s*(?:#{0,6}\s+)?(\S[^\n]*)?/; - -export const titleCharacterLimit = 200; +const removeMarkdownWithFix = inputString => { + // Workaround for a bug in `remove-markdown` + // See https://github.com/stiang/remove-markdown/issues/35 + return removeMarkdown(inputString.replace(/(\s)\s+/g, '$1'), { + stripListLeaders: false, + }); +}; -// Ample amount of characters for the 'Expanded' list view -export const previewCharacterLimit = 300; +const getTitle = (content, isNoteMarkdown) => { + const titlePattern = new RegExp(`\s*([^\n]{1,${maxTitleChars}})`, 'g'); + const titleMatch = titlePattern.exec(content); + if (!titleMatch) { + return 'New Note…'; + } + const [, rawTitle] = titleMatch; + return isNoteMarkdown + ? removeMarkdownWithFix(rawTitle) || rawTitle + : rawTitle; +}; -// Defaults for notes with empty content -export const defaults = { - title: 'New Note...', - preview: '', +const getPreview = (content, isNoteMarkdown) => { + const previewPattern = new RegExp( + `[^\n]*\n+[ \t]*([^]{0,${maxPreviewChars}})`, + 'g' + ); + let preview = matchUntilLimit(previewPattern, content); + return isNoteMarkdown ? removeMarkdownWithFix(preview) : preview; }; /** @@ -42,34 +59,13 @@ export const defaults = { */ export const noteTitleAndPreview = note => { const content = (note && note.data && note.data.content) || ''; - - const match = isMarkdown(note) - ? noteTitleAndPreviewMdRegExp.exec(content || '') - : noteTitleAndPreviewRegExp.exec(content || ''); - - if (!match) { - return defaults; - } - - let [, title = defaults.title, preview = defaults.preview] = match; - - title = title.slice(0, titleCharacterLimit); - preview = preview.slice(0, previewCharacterLimit); - - if (preview && isMarkdown(note)) { - // Workaround for a bug in `remove-markdown` - // See https://github.com/stiang/remove-markdown/issues/35 - const previewWithSpacesTrimmed = preview.replace(/(\s)\s+/g, '$1'); - - preview = removeMarkdown(previewWithSpacesTrimmed, { - stripListLeaders: false, - }); - } - + const isNoteMarkdown = isMarkdown(note); + const title = getTitle(content, isNoteMarkdown); + const preview = getPreview(content, isNoteMarkdown); return { title, preview }; }; -export function isMarkdown(note) { +function isMarkdown(note) { return note && note.data && note.data.systemTags.includes('markdown'); } diff --git a/lib/utils/note-utils.test.js b/lib/utils/note-utils.test.js index 615a9efb8..1388f8036 100644 --- a/lib/utils/note-utils.test.js +++ b/lib/utils/note-utils.test.js @@ -1,9 +1,4 @@ -import noteTitleAndPreview, { - defaults, - normalizeForSorting, - previewCharacterLimit, - titleCharacterLimit, -} from './note-utils'; +import noteTitleAndPreview, { normalizeForSorting } from './note-utils'; describe('noteTitleAndPreview', () => { let note; @@ -19,7 +14,7 @@ describe('noteTitleAndPreview', () => { it('should return default values if note content is empty', () => { const result = noteTitleAndPreview(note); - expect(result).toEqual(defaults); + expect(result).toEqual({ preview: '', title: 'New Note…' }); }); it('should return the title and preview when note is not Markdown', () => { @@ -55,13 +50,13 @@ describe('noteTitleAndPreview', () => { it('should have enough text for an Expanded preview, even if the title is very long', () => { // Longer than the char limits - const title = 'foo'.repeat(titleCharacterLimit); - const paragraph = 'foo'.repeat(previewCharacterLimit); + const title = 'foo'.repeat(64); + const paragraph = 'foo'.repeat(200); note.data.content = title + '\n' + paragraph; const result = noteTitleAndPreview(note); - expect(result.title).toHaveLength(titleCharacterLimit); - expect(result.preview).toHaveLength(previewCharacterLimit); + expect(result.title).toHaveLength(64); + expect(result.preview).toHaveLength(200); }); });