Skip to content

Commit

Permalink
Fine-tune linkification technique for link previews
Browse files Browse the repository at this point in the history
  • Loading branch information
scottnonnenberg-signal committed Feb 12, 2019
1 parent 021e807 commit 858c7e6
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 3 deletions.
24 changes: 22 additions & 2 deletions js/modules/link_previews.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* global URL */

const { isNumber, compact } = require('lodash');
const he = require('he');
const LinkifyIt = require('linkify-it');

Expand Down Expand Up @@ -96,9 +97,28 @@ function getImageMetaTag(html) {
return _getMetaTag(html, META_IMAGE);
}

function findLinks(text) {
function findLinks(text, caretLocation) {
const haveCaretLocation = isNumber(caretLocation);
const textLength = text ? text.length : 0;

const matches = linkify.match(text || '') || [];
return matches.map(match => match.text);
return compact(
matches.map(match => {
if (!haveCaretLocation) {
return match.text;
}

if (match.lastIndex === textLength && caretLocation === textLength) {
return match.text;
}

if (match.index > caretLocation || match.lastIndex < caretLocation) {
return match.text;
}

return null;
})
);
}

function getDomain(url) {
Expand Down
6 changes: 5 additions & 1 deletion js/views/conversation_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,7 @@
}

const messageText = this.$messageField.val().trim();
const caretLocation = this.$messageField.get(0).selectionStart;

if (!messageText) {
this.resetLinkPreview();
Expand All @@ -1709,7 +1710,10 @@
return;
}

const links = window.Signal.LinkPreviews.findLinks(messageText);
const links = window.Signal.LinkPreviews.findLinks(
messageText,
caretLocation
);
const { currentlyMatchedLink } = this;
if (links.includes(currentlyMatchedLink)) {
return;
Expand Down
77 changes: 77 additions & 0 deletions test/modules/link_previews_test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { assert } = require('chai');

const {
findLinks,
getTitleMetaTag,
getImageMetaTag,
isLinkInWhitelist,
Expand Down Expand Up @@ -228,4 +229,80 @@ describe('Link previews', () => {
);
});
});

describe('#findLinks', () => {
it('returns all links if no caretLocation is provided', () => {
const text =
'Check out this link: https://github.com/signalapp/Signal-Desktop\nAnd this one too: https://github.com/signalapp/Signal-Android';

const expected = [
'https://github.com/signalapp/Signal-Desktop',
'https://github.com/signalapp/Signal-Android',
];

const actual = findLinks(text);
assert.deepEqual(expected, actual);
});

it('includes all links if cursor is not in a link', () => {
const text =
'Check out this link: https://github.com/signalapp/Signal-Desktop\nAnd this one too: https://github.com/signalapp/Signal-Android';
const caretLocation = 10;

const expected = [
'https://github.com/signalapp/Signal-Desktop',
'https://github.com/signalapp/Signal-Android',
];

const actual = findLinks(text, caretLocation);
assert.deepEqual(expected, actual);
});

it('excludes a link not at the end if the caret is inside of it', () => {
const text =
'Check out this link: https://github.com/signalapp/Signal-Desktop\nAnd this one too: https://github.com/signalapp/Signal-Android';
const caretLocation = 30;

const expected = ['https://github.com/signalapp/Signal-Android'];

const actual = findLinks(text, caretLocation);
assert.deepEqual(expected, actual);
});

it('excludes a link not at the end if the caret is at its end', () => {
const text =
'Check out this link: https://github.com/signalapp/Signal-Desktop\nAnd this one too: https://github.com/signalapp/Signal-Android';
const caretLocation = 64;

const expected = ['https://github.com/signalapp/Signal-Android'];

const actual = findLinks(text, caretLocation);
assert.deepEqual(expected, actual);
});

it('excludes a link at the end of the caret is inside of it', () => {
const text =
'Check out this link: https://github.com/signalapp/Signal-Desktop\nAnd this one too: https://github.com/signalapp/Signal-Android';
const caretLocation = 100;

const expected = ['https://github.com/signalapp/Signal-Desktop'];

const actual = findLinks(text, caretLocation);
assert.deepEqual(expected, actual);
});

it('includes link at the end if cursor is at its end', () => {
const text =
'Check out this link: https://github.com/signalapp/Signal-Desktop\nAnd this one too: https://github.com/signalapp/Signal-Android';
const caretLocation = text.length;

const expected = [
'https://github.com/signalapp/Signal-Desktop',
'https://github.com/signalapp/Signal-Android',
];

const actual = findLinks(text, caretLocation);
assert.deepEqual(expected, actual);
});
});
});

0 comments on commit 858c7e6

Please sign in to comment.