From 25de4e39d86a24b323d59d2a2c4fefde06ecc29c Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Thu, 16 Dec 2021 12:26:05 +0100 Subject: [PATCH 1/6] Finalize handling space in mention plugin --- packages/ckeditor5-mention/src/mentionui.js | 105 ++++++++++++++---- .../ckeditor5-mention/tests/manual/mention.js | 2 +- .../ckeditor5-mention/tests/manual/mention.md | 1 + packages/ckeditor5-mention/tests/mentionui.js | 79 +++++++++++-- 4 files changed, 157 insertions(+), 30 deletions(-) diff --git a/packages/ckeditor5-mention/src/mentionui.js b/packages/ckeditor5-mention/src/mentionui.js index 8e315b9438b..8a29cd282f7 100644 --- a/packages/ckeditor5-mention/src/mentionui.js +++ b/packages/ckeditor5-mention/src/mentionui.js @@ -160,16 +160,14 @@ export default class MentionUI extends Plugin { throw new CKEditorError( 'mentionconfig-incorrect-marker', null, { marker } ); } - const minimumCharacters = mentionDescription.minimumCharacters || 0; const feedCallback = typeof feed == 'function' ? feed.bind( this.editor ) : createFeedCallback( feed ); - const watcher = this._setupTextWatcherForFeed( marker, minimumCharacters ); const itemRenderer = mentionDescription.itemRenderer; - - const definition = { watcher, marker, feedCallback, itemRenderer }; + const definition = { marker, feedCallback, itemRenderer }; this._mentionsConfigurations.set( marker, definition ); } + this._setupTextWatcherForFeed( feeds ); this.listenTo( editor, 'change:isReadOnly', () => { this._hideUIAndRemoveMarker(); } ); @@ -377,12 +375,18 @@ export default class MentionUI extends Plugin { * @param {Number} minimumCharacters * @returns {module:typing/textwatcher~TextWatcher} */ - _setupTextWatcherForFeed( marker, minimumCharacters ) { + _setupTextWatcherForFeed( feeds ) { const editor = this.editor; - const watcher = new TextWatcher( editor.model, createTestCallback( marker, minimumCharacters ) ); + const feedsWithRegexp = feeds.map( feed => ( { + ...feed, + pattern: createRegExp( feed.marker, feed.minimumCharacters || 0 ) + } ) ); + + const watcher = new TextWatcher( editor.model, createTestCallback( feedsWithRegexp ) ); watcher.on( 'matched', ( evt, data ) => { + const markerDefinition = _getLastValidMarkerInText( feedsWithRegexp, data.text ); const selection = editor.model.document.selection; const focus = selection.focus; @@ -392,8 +396,8 @@ export default class MentionUI extends Plugin { return; } - const feedText = requestFeedText( marker, data.text ); - const matchedTextLength = marker.length + feedText.length; + const feedText = requestFeedText( markerDefinition, data.text ); + const matchedTextLength = markerDefinition.marker.length + feedText.length; // Create a marker range. const start = focus.getShiftedBy( -matchedTextLength ); @@ -414,7 +418,7 @@ export default class MentionUI extends Plugin { } ); } - this._requestFeedDebounced( marker, feedText ); + this._requestFeedDebounced( markerDefinition.marker, feedText ); } ); watcher.on( 'unmatched', () => { @@ -655,18 +659,56 @@ function getBalloonPanelPositions( preferredPosition ) { ]; } +/** + * Returns a marker definition of the last valid occuring marker in given string. + * If there is no valid marker in string it returns undefined. + * + * Example of returned object: + * + * { + * marker: '@', + * position: 4, + * minimumCharacters: 0 + * } + * + * @param {Array.} feedsWithRegexp Registered feeds in editor for mention plugin with created RegExp for matching marker. + * @param {String} text String to find marker in + * @returns {Object} Matched marker definition + */ +function _getLastValidMarkerInText( feedsWithRegexp, text ) { + let lastValidMarker; + + feedsWithRegexp.forEach( feed => { + const currentMarkerLastIndex = text.lastIndexOf( feed.marker ); + + if ( currentMarkerLastIndex > 0 && !text.substring( currentMarkerLastIndex - 1 ).match( feed.pattern ) ) { + return; + } + + if ( !lastValidMarker || currentMarkerLastIndex >= lastValidMarker.position ) { + lastValidMarker = { + marker: feed.marker, + position: currentMarkerLastIndex, + minimumCharacters: feed.minimumCharacters + }; + } + } ); + + return lastValidMarker; +} + // Creates a RegExp pattern for the marker. // // Function has to be exported to achieve 100% code coverage. // // @param {String} marker // @param {Number} minimumCharacters -// @returns {RegExp} +// @returns {Object} export function createRegExp( marker, minimumCharacters ) { const numberOfCharacters = minimumCharacters == 0 ? '*' : `{${ minimumCharacters },}`; const openAfterCharacters = env.features.isRegExpUnicodePropertySupported ? '\\p{Ps}\\p{Pi}"\'' : '\\(\\[{"\''; - const mentionCharacters = '\\S'; + const mentionCharacters = '.'; // The pattern consists of 3 groups: // - 0 (non-capturing): Opening sequence - start of the line, space or an opening punctuation character like "(" or "\"", @@ -674,9 +716,8 @@ export function createRegExp( marker, minimumCharacters ) { // - 2: Mention input (taking the minimal length into consideration to trigger the UI), // // The pattern matches up to the caret (end of string switch - $). - // (0: opening sequence )(1: marker )(2: typed mention )$ - const pattern = `(?:^|[ ${ openAfterCharacters }])([${ marker }])([${ mentionCharacters }]${ numberOfCharacters })$`; - + // (0: opening sequence )(1: marker )(2: typed mention )$ + const pattern = `(?:^|[ ${ openAfterCharacters }])([${ marker }])(${ mentionCharacters }${ numberOfCharacters })$`; return new RegExp( pattern, 'u' ); } @@ -685,20 +726,44 @@ export function createRegExp( marker, minimumCharacters ) { // @param {String} marker // @param {Number} minimumCharacters // @returns {Function} -function createTestCallback( marker, minimumCharacters ) { - const regExp = createRegExp( marker, minimumCharacters ); +function createTestCallback( feedsWithRegexp ) { + const textRegexp = text => { + const markerDefinition = _getLastValidMarkerInText( feedsWithRegexp, text ); + + if ( !markerDefinition ) { + return false; + } + + let splitStringFrom = 0; - return text => regExp.test( text ); + if ( markerDefinition.position !== 0 ) { + splitStringFrom = markerDefinition.position - 1; + } + + const minimumCharacters = markerDefinition.minimumCharacters || 0; + const regExp = createRegExp( markerDefinition.marker, minimumCharacters ); + const textToTest = text.substr( splitStringFrom ); + + return regExp.test( textToTest ); + }; + + return textRegexp; } // Creates a text matcher from the marker. // // @param {String} marker // @returns {Function} -function requestFeedText( marker, text ) { - const regExp = createRegExp( marker, 0 ); +function requestFeedText( markerDefinition, text ) { + let splitStringFrom = 0; + + if ( markerDefinition.position !== 0 ) { + splitStringFrom = markerDefinition.position - 1; + } - const match = text.match( regExp ); + const regExp = createRegExp( markerDefinition.marker, 0 ); + const textToMatch = text.substr( splitStringFrom ); + const match = textToMatch.match( regExp ); return match[ 2 ]; } diff --git a/packages/ckeditor5-mention/tests/manual/mention.js b/packages/ckeditor5-mention/tests/manual/mention.js index 6ebbd847891..6abd3fdd01e 100644 --- a/packages/ckeditor5-mention/tests/manual/mention.js +++ b/packages/ckeditor5-mention/tests/manual/mention.js @@ -157,7 +157,7 @@ ClassicEditor feeds: [ { marker: '@', - feed: [ '@Barney', '@Lily', '@Marshall', '@Robin', '@Ted' ] + feed: [ '@Barney', '@Lily', '@Marshall', '@Robin', '@Ted', '@Thomas Reeves' ] }, { marker: '#', diff --git a/packages/ckeditor5-mention/tests/manual/mention.md b/packages/ckeditor5-mention/tests/manual/mention.md index a98b0d3ac8f..23d153cdf05 100644 --- a/packages/ckeditor5-mention/tests/manual/mention.md +++ b/packages/ckeditor5-mention/tests/manual/mention.md @@ -13,6 +13,7 @@ The feeds: - Marshall - Robin - Ted + - Thomas Reeves 2. Static list of 20 items (`#` marker) diff --git a/packages/ckeditor5-mention/tests/mentionui.js b/packages/ckeditor5-mention/tests/mentionui.js index 8c5c1750817..6140318f6d2 100644 --- a/packages/ckeditor5-mention/tests/mentionui.js +++ b/packages/ckeditor5-mention/tests/mentionui.js @@ -525,14 +525,14 @@ describe( 'MentionUI', () => { env.features.isRegExpUnicodePropertySupported = false; createRegExp( '@', 2 ); sinon.assert.calledOnce( regExpStub ); - sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\(\\[{"\'])([@])([\\S]{2,})$', 'u' ); + sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\(\\[{"\'])([@])(.{2,})$', 'u' ); } ); it( 'returns a ES2018 RegExp for browsers supporting Unicode punctuation groups', () => { env.features.isRegExpUnicodePropertySupported = true; createRegExp( '@', 2 ); sinon.assert.calledOnce( regExpStub ); - sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])([@])([\\S]{2,})$', 'u' ); + sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])([@])(.{2,})$', 'u' ); } ); } ); @@ -1942,7 +1942,7 @@ describe( 'MentionUI', () => { feeds: [ { marker: '@', - feed: [ '@a1', '@a2', '@a3' ] + feed: [ '@a1', '@a2', '@a3', '@a4 xyz', '@a5 x y z', '@a6 x$z' ] }, { marker: '$', @@ -1967,7 +1967,7 @@ describe( 'MentionUI', () => { .then( () => { expect( panelView.isVisible ).to.be.true; expect( editor.model.markers.has( 'mention' ) ).to.be.true; - expect( mentionsView.items ).to.have.length( 3 ); + expect( mentionsView.items ).to.have.length( 6 ); mentionsView.items.get( 0 ).children.get( 0 ).fire( 'execute' ); } ) @@ -2002,7 +2002,7 @@ describe( 'MentionUI', () => { expect( panelView.isVisible ).to.be.true; expect( editor.model.markers.has( 'mention' ) ).to.be.true; - expect( mentionsView.items ).to.have.length( 3 ); + expect( mentionsView.items ).to.have.length( 6 ); } ); } ); @@ -2017,7 +2017,7 @@ describe( 'MentionUI', () => { .then( () => { expect( panelView.isVisible ).to.be.true; expect( editor.model.markers.has( 'mention' ) ).to.be.true; - expect( mentionsView.items ).to.have.length( 3 ); + expect( mentionsView.items ).to.have.length( 6 ); mentionsView.items.get( 0 ).children.get( 0 ).fire( 'execute' ); } ) @@ -2042,6 +2042,66 @@ describe( 'MentionUI', () => { expect( editor.model.markers.has( 'mention' ) ).to.be.true; } ); } ); + + it( 'should match a feed', () => { + setData( model, 'foo []' ); + + model.change( writer => { + writer.insertText( '@a3', doc.selection.getFirstPosition() ); + } ); + + return waitForDebounce() + .then( () => { + expect( panelView.isVisible ).to.be.true; + expect( editor.model.markers.has( 'mention' ) ).to.be.true; + expect( mentionsView.items ).to.have.length( 1 ); + } ); + } ); + + it( 'should match a feed with space', () => { + setData( model, 'foo []' ); + + model.change( writer => { + writer.insertText( '@a4 xyz', doc.selection.getFirstPosition() ); + } ); + + return waitForDebounce() + .then( () => { + expect( panelView.isVisible ).to.be.true; + expect( editor.model.markers.has( 'mention' ) ).to.be.true; + expect( mentionsView.items ).to.have.length( 1 ); + } ); + } ); + + it( 'should match a feed with multiple spaces', () => { + setData( model, 'foo []' ); + + model.change( writer => { + writer.insertText( '@a5 x y z', doc.selection.getFirstPosition() ); + } ); + + return waitForDebounce() + .then( () => { + expect( panelView.isVisible ).to.be.true; + expect( editor.model.markers.has( 'mention' ) ).to.be.true; + expect( mentionsView.items ).to.have.length( 1 ); + } ); + } ); + + it( 'should match a feed with spaces and other mention character', () => { + setData( model, 'foo []' ); + + model.change( writer => { + writer.insertText( '@a6 x$z', doc.selection.getFirstPosition() ); + } ); + + return waitForDebounce() + .then( () => { + expect( panelView.isVisible ).to.be.true; + expect( editor.model.markers.has( 'mention' ) ).to.be.true; + expect( mentionsView.items ).to.have.length( 1 ); + } ); + } ); } ); function testExecuteKey( name, keyCode, feedItems ) { @@ -2308,9 +2368,10 @@ describe( 'MentionUI', () => { return waitForDebounce() .then( () => { mentionsView.items.get( 0 ).children.get( 0 ).fire( 'execute' ); - - expect( panelView.isVisible ).to.be.false; - expect( editor.model.markers.has( 'mention' ) ).to.be.false; + return waitForDebounce().then( () => { + expect( panelView.isVisible ).to.be.false; + expect( editor.model.markers.has( 'mention' ) ).to.be.false; + } ); } ); } ); From 743a9102dd9890df3fa8185c714284030184a24b Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Thu, 16 Dec 2021 13:24:53 +0100 Subject: [PATCH 2/6] Reverse accidental change of return type for a function --- packages/ckeditor5-mention/src/mentionui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-mention/src/mentionui.js b/packages/ckeditor5-mention/src/mentionui.js index 8a29cd282f7..243ac235cfb 100644 --- a/packages/ckeditor5-mention/src/mentionui.js +++ b/packages/ckeditor5-mention/src/mentionui.js @@ -703,7 +703,7 @@ function _getLastValidMarkerInText( feedsWithRegexp, text ) { // // @param {String} marker // @param {Number} minimumCharacters -// @returns {Object} +// @returns {RegExp} export function createRegExp( marker, minimumCharacters ) { const numberOfCharacters = minimumCharacters == 0 ? '*' : `{${ minimumCharacters },}`; From 76fb73e971a1357c95a269b0c6d2b1addafc18d6 Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Tue, 21 Dec 2021 12:05:19 +0100 Subject: [PATCH 3/6] Fix for pull request --- packages/ckeditor5-mention/src/mentionui.js | 68 ++++++++++----------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/packages/ckeditor5-mention/src/mentionui.js b/packages/ckeditor5-mention/src/mentionui.js index 243ac235cfb..071b9e592ab 100644 --- a/packages/ckeditor5-mention/src/mentionui.js +++ b/packages/ckeditor5-mention/src/mentionui.js @@ -167,7 +167,7 @@ export default class MentionUI extends Plugin { this._mentionsConfigurations.set( marker, definition ); } - this._setupTextWatcherForFeed( feeds ); + this._setupTextWatcher( feeds ); this.listenTo( editor, 'change:isReadOnly', () => { this._hideUIAndRemoveMarker(); } ); @@ -371,22 +371,21 @@ export default class MentionUI extends Plugin { * Registers a text watcher for the marker. * * @private - * @param {String} marker - * @param {Number} minimumCharacters + * @param {Array.} feeds Feeds of mention plugin configured in editor * @returns {module:typing/textwatcher~TextWatcher} */ - _setupTextWatcherForFeed( feeds ) { + _setupTextWatcher( feeds ) { const editor = this.editor; - const feedsWithRegexp = feeds.map( feed => ( { + const feedsWithPattern = feeds.map( feed => ( { ...feed, pattern: createRegExp( feed.marker, feed.minimumCharacters || 0 ) } ) ); - const watcher = new TextWatcher( editor.model, createTestCallback( feedsWithRegexp ) ); + const watcher = new TextWatcher( editor.model, createTestCallback( feedsWithPattern ) ); watcher.on( 'matched', ( evt, data ) => { - const markerDefinition = _getLastValidMarkerInText( feedsWithRegexp, data.text ); + const markerDefinition = getLastValidMarkerInText( feedsWithPattern, data.text ); const selection = editor.model.document.selection; const focus = selection.focus; @@ -659,26 +658,24 @@ function getBalloonPanelPositions( preferredPosition ) { ]; } -/** - * Returns a marker definition of the last valid occuring marker in given string. - * If there is no valid marker in string it returns undefined. - * - * Example of returned object: - * - * { - * marker: '@', - * position: 4, - * minimumCharacters: 0 - * } - * - * @param {Array.} feedsWithRegexp Registered feeds in editor for mention plugin with created RegExp for matching marker. - * @param {String} text String to find marker in - * @returns {Object} Matched marker definition - */ -function _getLastValidMarkerInText( feedsWithRegexp, text ) { +// Returns a marker definition of the last valid occuring marker in given string. +// If there is no valid marker in string it returns undefined. +// +// Example of returned object: +// +// { +// marker: '@', +// position: 4, +// minimumCharacters: 0 +// } +// +// @param {Array.} feedsWithPattern Registered feeds in editor for mention plugin with created RegExp for matching marker. +// @param {String} text String to find marker in +// @returns {Object} Matched marker's definition +function getLastValidMarkerInText( feedsWithPattern, text ) { let lastValidMarker; - feedsWithRegexp.forEach( feed => { + for ( const feed of feedsWithPattern ) { const currentMarkerLastIndex = text.lastIndexOf( feed.marker ); if ( currentMarkerLastIndex > 0 && !text.substring( currentMarkerLastIndex - 1 ).match( feed.pattern ) ) { @@ -689,10 +686,11 @@ function _getLastValidMarkerInText( feedsWithRegexp, text ) { lastValidMarker = { marker: feed.marker, position: currentMarkerLastIndex, - minimumCharacters: feed.minimumCharacters + minimumCharacters: feed.minimumCharacters, + pattern: feed.pattern }; } - } ); + } return lastValidMarker; } @@ -723,12 +721,11 @@ export function createRegExp( marker, minimumCharacters ) { // Creates a test callback for the marker to be used in the text watcher instance. // -// @param {String} marker -// @param {Number} minimumCharacters +// @param {Array.} feedsWithPattern Feeds of mention plugin configured in editor with RegExp to match marker in text // @returns {Function} -function createTestCallback( feedsWithRegexp ) { - const textRegexp = text => { - const markerDefinition = _getLastValidMarkerInText( feedsWithRegexp, text ); +function createTestCallback( feedsWithPattern ) { + const textMatcher = text => { + const markerDefinition = getLastValidMarkerInText( feedsWithPattern, text ); if ( !markerDefinition ) { return false; @@ -747,12 +744,13 @@ function createTestCallback( feedsWithRegexp ) { return regExp.test( textToTest ); }; - return textRegexp; + return textMatcher; } // Creates a text matcher from the marker. // -// @param {String} marker +// @param {Object} markerDefinition +// @param {String} text // @returns {Function} function requestFeedText( markerDefinition, text ) { let splitStringFrom = 0; @@ -762,7 +760,7 @@ function requestFeedText( markerDefinition, text ) { } const regExp = createRegExp( markerDefinition.marker, 0 ); - const textToMatch = text.substr( splitStringFrom ); + const textToMatch = text.substring( splitStringFrom ); const match = textToMatch.match( regExp ); return match[ 2 ]; From ade73ecd68e965e9da450b4efa8127cbf1f960ae Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Tue, 21 Dec 2021 12:18:13 +0100 Subject: [PATCH 4/6] Fix for pull request --- packages/ckeditor5-mention/src/mentionui.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-mention/src/mentionui.js b/packages/ckeditor5-mention/src/mentionui.js index 071b9e592ab..b4d9d591c15 100644 --- a/packages/ckeditor5-mention/src/mentionui.js +++ b/packages/ckeditor5-mention/src/mentionui.js @@ -679,7 +679,7 @@ function getLastValidMarkerInText( feedsWithPattern, text ) { const currentMarkerLastIndex = text.lastIndexOf( feed.marker ); if ( currentMarkerLastIndex > 0 && !text.substring( currentMarkerLastIndex - 1 ).match( feed.pattern ) ) { - return; + continue; } if ( !lastValidMarker || currentMarkerLastIndex >= lastValidMarker.position ) { @@ -737,11 +737,9 @@ function createTestCallback( feedsWithPattern ) { splitStringFrom = markerDefinition.position - 1; } - const minimumCharacters = markerDefinition.minimumCharacters || 0; - const regExp = createRegExp( markerDefinition.marker, minimumCharacters ); - const textToTest = text.substr( splitStringFrom ); + const textToTest = text.substring( splitStringFrom ); - return regExp.test( textToTest ); + return markerDefinition.pattern.test( textToTest ); }; return textMatcher; From 51a1fca74591e8904418de37a493d9548b0193af Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Tue, 21 Dec 2021 12:37:17 +0100 Subject: [PATCH 5/6] Change names of feeds in mention plugin manual test --- packages/ckeditor5-mention/tests/manual/mention.js | 2 +- packages/ckeditor5-mention/tests/manual/mention.md | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-mention/tests/manual/mention.js b/packages/ckeditor5-mention/tests/manual/mention.js index 6abd3fdd01e..20c3aca0abc 100644 --- a/packages/ckeditor5-mention/tests/manual/mention.js +++ b/packages/ckeditor5-mention/tests/manual/mention.js @@ -157,7 +157,7 @@ ClassicEditor feeds: [ { marker: '@', - feed: [ '@Barney', '@Lily', '@Marshall', '@Robin', '@Ted', '@Thomas Reeves' ] + feed: [ '@Barney Stinson', '@Lily Aldrin', '@Marshall Eriksen', '@Robin Sherbatsky', '@Ted Mosby' ] }, { marker: '#', diff --git a/packages/ckeditor5-mention/tests/manual/mention.md b/packages/ckeditor5-mention/tests/manual/mention.md index 23d153cdf05..3ab3c86b807 100644 --- a/packages/ckeditor5-mention/tests/manual/mention.md +++ b/packages/ckeditor5-mention/tests/manual/mention.md @@ -8,12 +8,11 @@ The feeds: 1. Static list with `@` marker: - - Barney - - Lily - - Marshall - - Robin - - Ted - - Thomas Reeves + - Barney Stinson + - Lily Aldrin + - Marshall Eriksen + - Robin Sherbatsky + - Ted Mosby 2. Static list of 20 items (`#` marker) From a8a571efd444324fd26c2e889f1287d6deab4db6 Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Tue, 21 Dec 2021 12:41:12 +0100 Subject: [PATCH 6/6] Fix names --- packages/ckeditor5-mention/tests/manual/mention.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-mention/tests/manual/mention.html b/packages/ckeditor5-mention/tests/manual/mention.html index bdce5ac9ec1..a2ccf4cf31a 100644 --- a/packages/ckeditor5-mention/tests/manual/mention.html +++ b/packages/ckeditor5-mention/tests/manual/mention.html @@ -1,6 +1,6 @@
-

Hello @Ted.

-

Hello @Ted@Ted.

+

Hello @Ted Mosby.

+

Hello @Ted Mosby@Ted Mosby.