Skip to content

Commit

Permalink
Replace #1647: Show more than first line in note list preview
Browse files Browse the repository at this point in the history
Replaces #1647

fixes: #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:**
<img width="331" alt="Screen Shot 2019-10-18 at 10 53 21 AM" src="https://user-images.githubusercontent.com/6817400/67104796-9690bd80-f195-11e9-959a-1edc18f58030.png">
**After:**
<img width="329" alt="Screen Shot 2019-10-18 at 10 53 29 AM" src="https://user-images.githubusercontent.com/6817400/67104797-9690bd80-f195-11e9-9c17-282747d9a7da.png">

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
  • Loading branch information
dmsnell committed Oct 28, 2019
1 parent 754a743 commit 192e3bf
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 61 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
96 changes: 46 additions & 50 deletions lib/utils/note-utils.js
Original file line number Diff line number Diff line change
@@ -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;
};

/**
Expand All @@ -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');
}

Expand Down
17 changes: 6 additions & 11 deletions lib/utils/note-utils.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import noteTitleAndPreview, {
defaults,
normalizeForSorting,
previewCharacterLimit,
titleCharacterLimit,
} from './note-utils';
import noteTitleAndPreview, { normalizeForSorting } from './note-utils';

describe('noteTitleAndPreview', () => {
let note;
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});

Expand Down

0 comments on commit 192e3bf

Please sign in to comment.