Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for mentions markers to be longer than 1 character #17335

Open
wants to merge 2 commits into
base: cc/image-merge-fields
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 2 additions & 20 deletions packages/ckeditor5-mention/src/mentioncommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,27 +113,9 @@ export default class MentionCommand extends Command {

const mention = _addMentionAttributes( { _text: mentionText, id: mentionID }, mentionData );

if ( options.marker.length != 1 ) {
if ( !mentionID.startsWith( options.marker ) ) {
/**
* The marker must be a single character.
*
* Correct markers: `'@'`, `'#'`.
*
* Incorrect markers: `'@@'`, `'[@'`.
*
* See {@link module:mention/mentionconfig~MentionConfig}.
*
* @error mentioncommand-incorrect-marker
*/
throw new CKEditorError(
'mentioncommand-incorrect-marker',
this
);
}

if ( mentionID.charAt( 0 ) != options.marker ) {
/**
* The feed item ID must start with the marker character.
* The feed item ID must start with the marker character(s).
*
* Correct mention feed setting:
*
Expand Down
10 changes: 5 additions & 5 deletions packages/ckeditor5-mention/src/mentionui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -722,12 +722,12 @@ export function createRegExp( marker: string, minimumCharacters: number ): RegEx
// The pattern consists of 3 groups:
//
// - 0 (non-capturing): Opening sequence - start of the line, space or an opening punctuation character like "(" or "\"",
// - 1: The marker character,
// - 1: The marker character(s),
// - 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' );
}
Expand Down Expand Up @@ -822,8 +822,8 @@ function isMarkerInExistingMention( markerPosition: Position ): boolean | null {
/**
* Checks if string is a valid mention marker.
*/
function isValidMentionMarker( marker: string ): boolean | string {
return marker && marker.length == 1;
function isValidMentionMarker( marker: string ): boolean {
return !!marker;
}

/**
Expand Down
13 changes: 0 additions & 13 deletions packages/ckeditor5-mention/tests/mentioncommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,6 @@ describe( 'MentionCommand', () => {
expect( textNode.hasAttribute( 'bold' ) ).to.be.true;
} );

it( 'should throw if marker is not one character', () => {
setData( model, '<paragraph>foo @Jo[]bar</paragraph>' );

const testCases = [
{ marker: '##', mention: '##foo' },
{ marker: '', mention: '@foo' }
];

for ( const options of testCases ) {
expectToThrowCKEditorError( () => command.execute( options ), /mentioncommand-incorrect-marker/, editor );
}
} );

it( 'should throw if marker does not match mention id', () => {
setData( model, '<paragraph>foo @Jo[]bar</paragraph>' );

Expand Down
60 changes: 52 additions & 8 deletions packages/ckeditor5-mention/tests/mentionui.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,8 @@ describe( 'MentionUI', () => {
} );
} );

it( 'should throw if marker is longer then 1 character', () => {
return createClassicTestEditor( { feeds: [ { marker: '$$', feed: [ 'a' ] } ] } ).catch( error => {
assertCKEditorError( error, /mentionconfig-incorrect-marker/, null, { marker: '$$' } );
} );
it( 'should not throw if marker is longer then 1 character', () => {
expect( () => createClassicTestEditor( { feeds: [ { marker: '$$', feed: [ 'a' ] } ] } ) ).to.not.throw();
} );
} );

Expand Down Expand Up @@ -401,28 +399,35 @@ describe( 'MentionUI', () => {
env.features.isRegExpUnicodePropertySupported = false;
createRegExp( '@', 2 );
sinon.assert.calledOnce( regExpStub );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\(\\[{"\'])([@])(.{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}"\'])([@])(.{2,})$', 'u' );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])(@)(.{2,})$', 'u' );
} );

it( 'returns a proper regexp for markers longer than 1 character', () => {
env.features.isRegExpUnicodePropertySupported = true;
createRegExp( '@@', 2 );
sinon.assert.calledOnce( regExpStub );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])(@@)(.{2,})$', 'u' );
} );

it( 'correctly escapes passed marker #1', () => {
env.features.isRegExpUnicodePropertySupported = true;
createRegExp( ']', 2 );
sinon.assert.calledOnce( regExpStub );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])([\\]])(.{2,})$', 'u' );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])(\\])(.{2,})$', 'u' );
} );

it( 'correctly escapes passed marker #2', () => {
env.features.isRegExpUnicodePropertySupported = true;
createRegExp( '\\', 2 );
sinon.assert.calledOnce( regExpStub );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])([\\\\])(.{2,})$', 'u' );
sinon.assert.calledWithExactly( regExpStub, '(?:^|[ \\p{Ps}\\p{Pi}"\'])(\\\\)(.{2,})$', 'u' );
} );
} );

Expand Down Expand Up @@ -470,6 +475,45 @@ describe( 'MentionUI', () => {
} );
} );

it( 'should show panel after the whole marker is matched', () => {
return createClassicTestEditor( {
feeds: [ { marker: '@@', feed: [ '@Barney', '@Lily', '@Marshall', '@Robin', '@Ted' ] } ]
} )
.then( () => {
setData( editor.model, '<paragraph>foo []</paragraph>' );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );
} )
.then( waitForDebounce )
.then( () => {
expect( panelView.isVisible ).to.be.false;
expect( editor.model.markers.has( 'mention' ) ).to.be.false;
} )
.then( () => {
model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );
} )
.then( waitForDebounce )
.then( () => {
expect( panelView.isVisible ).to.be.true;
expect( editor.model.markers.has( 'mention' ) ).to.be.true;
expect( mentionsView.items ).to.have.length( 5 );

model.change( writer => {
writer.insertText( 't', doc.selection.getFirstPosition() );
} );
} )
.then( 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 update the marker if the selection was moved from one valid position to another', () => {
const spy = sinon.spy();

Expand Down