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.

Sign up for GitHub

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

Jump to bottom

Fixed emoji text matching with repeated symbols in a single line #2452

Merged
merged 6 commits into from
Oct 3, 2018
Merged

Fixed emoji text matching with repeated symbols in a single line #2452

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c98159e
Corrected emoji text matching.
jacekbogdanski Oct 3, 2018
0f90307
Refactored test so they can be run synchronously.
jacekbogdanski Oct 3, 2018
65cfb17
Added unit test.
jacekbogdanski Oct 3, 2018
0cfef02
Added manual test.
jacekbogdanski Oct 3, 2018
c05424e
Added changelog entry.
jacekbogdanski Oct 3, 2018
2850ae1
Tests: minor tests corrections, merged var statements, fixed typos etc.
mlewand Oct 3, 2018
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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ New Features:
Fixed Issues:

* [#1477](https://github.com/ckeditor/ckeditor-dev/issues/1477): Fixed: [Balloon Toolbar](https://ckeditor.com/cke4/addon/balloontoolbar) on destroy doesn't destroy its content.
* [#2394](https://github.com/ckeditor/ckeditor-dev/issues/2394): Fixed: [Emoji](https://ckeditor.com/cke4/addon/emoji) dropdown doesn't show up with repeated symbols in a single line.

API Changes:

Expand Down
4 changes: 2 additions & 2 deletions plugins/emoji/plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@
return null;
}

// In case of space preceding colon we need to return index of capturing grup.
return { start: text.indexOf( match[ 1 ] ), end: offset };
// In case of space preceding colon we need to return the last index (#2394) of capturing grup.
return { start: left.lastIndexOf( match[ 1 ] ), end: offset };
}

function dataCallback( matchInfo, callback ) {
Expand Down
67 changes: 31 additions & 36 deletions tests/plugins/emoji/basic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@

var stub = null;

// Disable autocomplete throttling so the tests can be run synchronously.
var throttle = CKEDITOR.tools.buffers.throttle;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible try to merge subsequent var statements.

sinon.stub( CKEDITOR.tools.buffers, 'throttle', function( minInterval, output, contextObj ) {
return new throttle( 0, output, contextObj );
} );

var singleTests = {
'test for custom emoji characters': function() {
var editor = this.editors.divarea,
Expand All @@ -46,18 +52,12 @@
emojiTools.assertIsNullOrUndefined( autocomplete.model.query );
emojiTools.assertIsNullOrUndefined( autocomplete.model.data );

// Handle throttle in autocomplete which by defualt is 20ms;
setTimeout( function() {
resume( function() {
bot.setHtmlWithSelection( '<p>foo :dagg^</p>' );
editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
assert.areSame( ':dagg', autocomplete.model.query, 'Model keeps wrong querry.' );
assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result.' );
objectAssert.areEqual( { id: ':dagger:', symbol: '🗡' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
autocomplete.close();
} );
}, 50 );
wait();
bot.setHtmlWithSelection( '<p>foo :dagg^</p>' );
editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
assert.areSame( ':dagg', autocomplete.model.query, 'Model keeps wrong querry.' );
assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result.' );
objectAssert.areEqual( { id: ':dagger:', symbol: '🗡' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
autocomplete.close();
} );
}
};
Expand Down Expand Up @@ -136,15 +136,11 @@
var autocomplete = editor._.emoji.autocomplete,
queries = [ ':OK_HAND', ':ok_hand', ':OK_hand', ':ok_HAND', ':Ok_hanD', 'oK_HANd' ];

CKEDITOR.tools.array.forEach( queries, function( query, index ) {
setTimeout( function() {
resume();
bot.setHtmlWithSelection( '<p>foo ' + query + '^</p>' );
editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
CKEDITOR.tools.array.forEach( queries, function( query ) {
bot.setHtmlWithSelection( '<p>foo ' + query + '^</p>' );
editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );

objectAssert.areEqual( { id: ':ok_hand:', symbol: '👌' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
}, 50 * ( index + 1 ) );
wait();
objectAssert.areEqual( { id: ':ok_hand:', symbol: '👌' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
} );
} );
},
Expand All @@ -157,16 +153,10 @@

bot.setHtmlWithSelection( '<p>foo:bug^</p>' );

// Delay assertions because of autocomplete throttle.
setTimeout( function() {
resume( function() {
emojiTools.assertIsNullOrUndefined( autocomplete.model.query );
emojiTools.assertIsNullOrUndefined( autocomplete.model.data );
} );
}, 50 );
emojiTools.assertIsNullOrUndefined( autocomplete.model.query );
emojiTools.assertIsNullOrUndefined( autocomplete.model.data );

editable.fire( 'keyup', new CKEDITOR.dom.event( {} ) );
wait();
} );
},

Expand All @@ -177,16 +167,21 @@
editable = editor.editable();

bot.setHtmlWithSelection( '<p>foo</p><p>:bug^</p>' );
editable.fire( 'keyup', new CKEDITOR.dom.event( {} ) );
objectAssert.areEqual( { id: ':bug:', symbol: '🐛' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
} );
},

// Delay assertions because of autocomplete throttle.
setTimeout( function() {
resume( function() {
objectAssert.areEqual( { id: ':bug:', symbol: '🐛' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
} );
}, 50 );
// (#2394)
'test emoji correctly matches repeated keywords': function( editor, bot ) {
emojiTools.runAfterInstanceReady( editor, bot, function( editor, bot ) {
var autocomplete = editor._.emoji.autocomplete;

editable.fire( 'keyup', new CKEDITOR.dom.event( {} ) );
wait();
bot.setHtmlWithSelection( '<p>foo :collision :collision^</p>' );
editor.editable().fire( 'keyup', new CKEDITOR.dom.event( {} ) );
assert.areSame( ':collision', autocomplete.model.query, 'Model keeps wrong querry.' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no dots at the end of assert msg. I can see that you might have take it from other asserts in this suite.

What's interesting is that objectAsserts have correct form 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo querry -> query

assert.areSame( 1, autocomplete.model.data.length, 'Emoji result contains more than one result.' );
objectAssert.areEqual( { id: ':collision:', symbol: '💥' }, autocomplete.model.data[ 0 ], 'Emoji result contains wrong result' );
} );
}
};
Expand Down
10 changes: 10 additions & 0 deletions tests/plugins/emoji/manual/repeated.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<h2>Classic editor</h2>
<div id="classic"></div>
<h2>Inline editor</h2>
<div id="inline" contenteditable="true">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a detail, but since you're closing all of the tags in a single line, it would be nice to close this one as well.

</div>

<script>
CKEDITOR.replace( 'classic' );
CKEDITOR.inline( 'inline' );
</script>
18 changes: 18 additions & 0 deletions tests/plugins/emoji/manual/repeated.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
@bender-tags: 4.11.0, bug, emoji, 2394
@bender-ckeditor-plugins: wysiwygarea, sourcearea, emoji
@bender-ui: collapsed
@bender-include: ../_helpers/tools.js

## For both editors:

1. Focus the editor.
1. Type `:bu :bu`.

### Expected

Dropdown shows up with `:bug` (🐛) suggestion symbol.

### Unexpected

* Dropdown doesn't appear.
* Dropdown contains different sugestion symbols.