From de14e29a130f6f5d96b5edc8af0fc65dd092e519 Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Mon, 30 Dec 2024 13:40:22 +0100 Subject: [PATCH 1/7] Not displaying unsupported emoji in the emoji picker and mention plugin. --- packages/ckeditor5-emoji/src/emojipicker.ts | 56 ++++++++++++++++++--- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-emoji/src/emojipicker.ts b/packages/ckeditor5-emoji/src/emojipicker.ts index bcf2cf560c8..d85a3263748 100644 --- a/packages/ckeditor5-emoji/src/emojipicker.ts +++ b/packages/ckeditor5-emoji/src/emojipicker.ts @@ -167,11 +167,20 @@ export default class EmojiPicker extends Plugin { databaseId: number; title: string; exampleEmoji: string; } ): Promise { const databaseGroup = await this._emojiDatabase.getEmojiByGroup( databaseId ); - - return { - title, - exampleEmoji, - items: databaseGroup.map( item => { + const container = this._createEmojiWidthTestingContainer(); + const baselineEmojiWidth = this._getNodeWidth( container, '🙂' ); + + const items = databaseGroup + .filter( item => { + const emojiWidth = this._getNodeWidth( container, item.unicode ); + + // On Windows, some supported emoji are ~50% bigger than the baseline emoji, but what we really want to guard + // against are the ones that are 2x the size, because those are truly broken (person with red hair = person with + // floating red wig, black cat = cat with black square, polar bear = bear with snowflake, etc.) + // So here we set the threshold at 1.8 times the size of the baseline emoji. + return ( emojiWidth / 1.8 < baselineEmojiWidth ) && ( emojiWidth >= baselineEmojiWidth ); + } ) + .map( item => { const name = item.annotation; const emojis = [ item.unicode ]; @@ -182,7 +191,15 @@ export default class EmojiPicker extends Plugin { this._emojis.set( name, emojis ); return { name, emojis }; - } ) + } ); + + // Clean up width testing container. + document.body.removeChild( container ); + + return { + title, + exampleEmoji, + items }; } @@ -432,6 +449,33 @@ export default class EmojiPicker extends Plugin { } ); } } + + /** + * Creates a div for emoji width testing purposes. + */ + private _createEmojiWidthTestingContainer(): HTMLDivElement { + const container = document.createElement( 'div' ); + container.style.position = 'absolute'; + container.style.left = '-9999px'; + container.style.whiteSpace = 'nowrap'; + container.style.fontSize = '24px'; + document.body.appendChild( container ); + + return container; + } + + /** + * Returns the width of the provided node. + */ + private _getNodeWidth( container: HTMLDivElement, node: string ): number { + const span = document.createElement( 'span' ); + span.textContent = node; + container.appendChild( span ); + const nodeWidth = span.offsetWidth; + container.removeChild( span ); + + return nodeWidth; + } } export interface DropdownPanelContent { From e817e8783cc9bd699ca0f8f8ab184d8489821bd1 Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Mon, 30 Dec 2024 15:42:35 +0100 Subject: [PATCH 2/7] Fixed tests. --- packages/ckeditor5-emoji/tests/emojimention.js | 16 ++++++++-------- packages/ckeditor5-emoji/tests/emojipicker.js | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-emoji/tests/emojimention.js b/packages/ckeditor5-emoji/tests/emojimention.js index 00e98dbf938..c90f2040cdd 100644 --- a/packages/ckeditor5-emoji/tests/emojimention.js +++ b/packages/ckeditor5-emoji/tests/emojimention.js @@ -288,11 +288,11 @@ describe( 'EmojiMention', () => { } ); it( 'should query single emoji properly properly', () => { - return queryEmoji( 'flag_poland' ).then( queryResult => { - expect( queryResult ).to.deep.equal( [ - { id: 'emoji:flag_poland:', text: '🇵🇱' }, - { id: 'emoji:__SHOW_ALL_EMOJI__:', text: 'flag_poland' } - ] ); + return queryEmoji( 'toothbrush' ).then( queryResult => { + expect( queryResult[ 0 ].id ).to.equal( 'emoji:toothbrush:' ); + expect( queryResult[ 0 ].text ).to.equal( '🪥' ); + expect( queryResult[ 1 ].id ).to.equal( 'emoji:__SHOW_ALL_EMOJI__:' ); + expect( queryResult[ 1 ].text ).to.equal( 'toothbrush' ); } ); } ); @@ -378,12 +378,12 @@ describe( 'EmojiMention', () => { it( 'should return emojis with the default skin tone when the skin tone is selected but the emoji does not have variants', () => { editor.plugins.get( EmojiPicker )._selectedSkinTone = 5; - return queryEmoji( 'flag_poland' ).then( queryResult => { + return queryEmoji( 'toothbrush' ).then( queryResult => { expect( queryResult.length ).to.equal( 2 ); expect( queryResult[ 0 ] ).to.deep.equal( { - id: 'emoji:flag_poland:', - text: '🇵🇱' + id: 'emoji:toothbrush:', + text: '🪥' } ); expect( queryResult[ 1 ].id ).to.equal( 'emoji:__SHOW_ALL_EMOJI__:' ); } ); diff --git a/packages/ckeditor5-emoji/tests/emojipicker.js b/packages/ckeditor5-emoji/tests/emojipicker.js index 8bc8d96e87b..473ef13f205 100644 --- a/packages/ckeditor5-emoji/tests/emojipicker.js +++ b/packages/ckeditor5-emoji/tests/emojipicker.js @@ -88,11 +88,11 @@ describe( 'EmojiPicker', () => { clickEmojiToolbarButton(); - const firstEmojiInGrid = document.querySelector( '.ck-emoji-grid__tiles > button' ); + const firstEmojiInGrid = document.querySelector( '.ck-emoji-grid__tiles [title="slightly smiling face"]' ); firstEmojiInGrid.click(); - expect( getModelData( editor.model ) ).to.equal( '😀[]' ); + expect( getModelData( editor.model ) ).to.equal( '🙂[]' ); } ); it( 'should close the picker when clicking outside of it', async () => { From 3e7b251cff17d3c8a4a345a2e5877103833ee35b Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Tue, 31 Dec 2024 11:38:09 +0100 Subject: [PATCH 3/7] Fixed baseline emoji width on Windows. --- packages/ckeditor5-emoji/src/emojipicker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-emoji/src/emojipicker.ts b/packages/ckeditor5-emoji/src/emojipicker.ts index d85a3263748..83b14213bc4 100644 --- a/packages/ckeditor5-emoji/src/emojipicker.ts +++ b/packages/ckeditor5-emoji/src/emojipicker.ts @@ -168,7 +168,7 @@ export default class EmojiPicker extends Plugin { } ): Promise { const databaseGroup = await this._emojiDatabase.getEmojiByGroup( databaseId ); const container = this._createEmojiWidthTestingContainer(); - const baselineEmojiWidth = this._getNodeWidth( container, '🙂' ); + const baselineEmojiWidth = 24; const items = databaseGroup .filter( item => { From 74fc0faa53247d1f8ffaa55b3b1b7b0cfa985c20 Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Tue, 31 Dec 2024 11:50:51 +0100 Subject: [PATCH 4/7] Extracted baseline emoji width to a common variable. --- packages/ckeditor5-emoji/src/emojipicker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-emoji/src/emojipicker.ts b/packages/ckeditor5-emoji/src/emojipicker.ts index 83b14213bc4..a7755671278 100644 --- a/packages/ckeditor5-emoji/src/emojipicker.ts +++ b/packages/ckeditor5-emoji/src/emojipicker.ts @@ -32,6 +32,7 @@ import { import EmojiToneView, { type SkinToneId } from './ui/emojitoneview.js'; const VISUAL_SELECTION_MARKER_NAME = 'emoji-picker'; +const BASELINE_EMOJI_WIDTH = 24; /** * The emoji picker plugin. @@ -168,7 +169,6 @@ export default class EmojiPicker extends Plugin { } ): Promise { const databaseGroup = await this._emojiDatabase.getEmojiByGroup( databaseId ); const container = this._createEmojiWidthTestingContainer(); - const baselineEmojiWidth = 24; const items = databaseGroup .filter( item => { @@ -178,7 +178,7 @@ export default class EmojiPicker extends Plugin { // against are the ones that are 2x the size, because those are truly broken (person with red hair = person with // floating red wig, black cat = cat with black square, polar bear = bear with snowflake, etc.) // So here we set the threshold at 1.8 times the size of the baseline emoji. - return ( emojiWidth / 1.8 < baselineEmojiWidth ) && ( emojiWidth >= baselineEmojiWidth ); + return ( emojiWidth / 1.8 < BASELINE_EMOJI_WIDTH ) && ( emojiWidth >= BASELINE_EMOJI_WIDTH ); } ) .map( item => { const name = item.annotation; @@ -458,7 +458,7 @@ export default class EmojiPicker extends Plugin { container.style.position = 'absolute'; container.style.left = '-9999px'; container.style.whiteSpace = 'nowrap'; - container.style.fontSize = '24px'; + container.style.fontSize = BASELINE_EMOJI_WIDTH + 'px'; document.body.appendChild( container ); return container; From d9482c49fa1a5402fc93e66696281b6cb16b9dc2 Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Tue, 7 Jan 2025 17:09:28 +0100 Subject: [PATCH 5/7] Installing newest emoji on CI to enable emoji picker testing. --- .circleci/template.yml | 6 ++++++ scripts/ci/generate-circleci-configuration.mjs | 1 + 2 files changed, 7 insertions(+) diff --git a/.circleci/template.yml b/.circleci/template.yml index e9f0501b7bb..510fec29aa9 100644 --- a/.circleci/template.yml +++ b/.circleci/template.yml @@ -14,6 +14,12 @@ parameters: default: false commands: + install_newest_emoji: + steps: + - run: + name: Install newest emoji to enable emoji picker testing + command: | + sudo apt install fonts-noto-color-emoji halt_if_short_flow: steps: - run: diff --git a/scripts/ci/generate-circleci-configuration.mjs b/scripts/ci/generate-circleci-configuration.mjs index 851d3b1ef59..9cbedd6d849 100755 --- a/scripts/ci/generate-circleci-configuration.mjs +++ b/scripts/ci/generate-circleci-configuration.mjs @@ -140,6 +140,7 @@ const persistToWorkspace = fileName => ( { machine: true, steps: [ ...bootstrapCommands(), + 'install_newest_emoji', prepareCodeCoverageDirectories(), listBatchPackages( batch ), ...generateTestSteps( batch, { From 4732368d43070fad3886186cc95dead6d32bd0f4 Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Tue, 7 Jan 2025 17:24:11 +0100 Subject: [PATCH 6/7] Revert "Fixed tests." This reverts commit e817e8783cc9bd699ca0f8f8ab184d8489821bd1. --- packages/ckeditor5-emoji/tests/emojimention.js | 16 ++++++++-------- packages/ckeditor5-emoji/tests/emojipicker.js | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-emoji/tests/emojimention.js b/packages/ckeditor5-emoji/tests/emojimention.js index c90f2040cdd..00e98dbf938 100644 --- a/packages/ckeditor5-emoji/tests/emojimention.js +++ b/packages/ckeditor5-emoji/tests/emojimention.js @@ -288,11 +288,11 @@ describe( 'EmojiMention', () => { } ); it( 'should query single emoji properly properly', () => { - return queryEmoji( 'toothbrush' ).then( queryResult => { - expect( queryResult[ 0 ].id ).to.equal( 'emoji:toothbrush:' ); - expect( queryResult[ 0 ].text ).to.equal( '🪥' ); - expect( queryResult[ 1 ].id ).to.equal( 'emoji:__SHOW_ALL_EMOJI__:' ); - expect( queryResult[ 1 ].text ).to.equal( 'toothbrush' ); + return queryEmoji( 'flag_poland' ).then( queryResult => { + expect( queryResult ).to.deep.equal( [ + { id: 'emoji:flag_poland:', text: '🇵🇱' }, + { id: 'emoji:__SHOW_ALL_EMOJI__:', text: 'flag_poland' } + ] ); } ); } ); @@ -378,12 +378,12 @@ describe( 'EmojiMention', () => { it( 'should return emojis with the default skin tone when the skin tone is selected but the emoji does not have variants', () => { editor.plugins.get( EmojiPicker )._selectedSkinTone = 5; - return queryEmoji( 'toothbrush' ).then( queryResult => { + return queryEmoji( 'flag_poland' ).then( queryResult => { expect( queryResult.length ).to.equal( 2 ); expect( queryResult[ 0 ] ).to.deep.equal( { - id: 'emoji:toothbrush:', - text: '🪥' + id: 'emoji:flag_poland:', + text: '🇵🇱' } ); expect( queryResult[ 1 ].id ).to.equal( 'emoji:__SHOW_ALL_EMOJI__:' ); } ); diff --git a/packages/ckeditor5-emoji/tests/emojipicker.js b/packages/ckeditor5-emoji/tests/emojipicker.js index 473ef13f205..8bc8d96e87b 100644 --- a/packages/ckeditor5-emoji/tests/emojipicker.js +++ b/packages/ckeditor5-emoji/tests/emojipicker.js @@ -88,11 +88,11 @@ describe( 'EmojiPicker', () => { clickEmojiToolbarButton(); - const firstEmojiInGrid = document.querySelector( '.ck-emoji-grid__tiles [title="slightly smiling face"]' ); + const firstEmojiInGrid = document.querySelector( '.ck-emoji-grid__tiles > button' ); firstEmojiInGrid.click(); - expect( getModelData( editor.model ) ).to.equal( '🙂[]' ); + expect( getModelData( editor.model ) ).to.equal( '😀[]' ); } ); it( 'should close the picker when clicking outside of it', async () => { From abad86a60f60e17dce635513e10d4378999c019a Mon Sep 17 00:00:00 2001 From: Marcin Panek Date: Tue, 7 Jan 2025 17:31:12 +0100 Subject: [PATCH 7/7] Setting aria-hidden to true for div created for testing emoji support. --- packages/ckeditor5-emoji/src/emojipicker.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ckeditor5-emoji/src/emojipicker.ts b/packages/ckeditor5-emoji/src/emojipicker.ts index a7755671278..1f9ca8fff01 100644 --- a/packages/ckeditor5-emoji/src/emojipicker.ts +++ b/packages/ckeditor5-emoji/src/emojipicker.ts @@ -455,6 +455,7 @@ export default class EmojiPicker extends Plugin { */ private _createEmojiWidthTestingContainer(): HTMLDivElement { const container = document.createElement( 'div' ); + container.setAttribute( 'aria-hidden', 'true' ); container.style.position = 'absolute'; container.style.left = '-9999px'; container.style.whiteSpace = 'nowrap';