From da635229bf397d4bef8c0f7afe490fbca78f58e2 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 10 Aug 2025 21:36:11 +0200 Subject: [PATCH 1/6] chore: simplify storing and sending custom emoji's - Using a map to store the CustomEmojis is unncessary, the values can be derived by the key easily. We still need to know if a certain name is custom emoji, so store the custom emojis in a `Set`. - Instead of constructing a map in `window.config`, construct a `Set` and reconstruct the value in `emoji.js`. This has the main benefit of reducing the amount of text being sent for each request, which is quite noticable if a Forgejo instance has many (>1000) custom emojis. - Remove the default value of `CustomEmojisMap`, it will be constructed anyway. --- modules/markup/html.go | 2 +- modules/markup/html_test.go | 2 +- modules/setting/ui.go | 9 +++------ modules/templates/helper.go | 4 ++-- templates/base/head_script.tmpl | 2 +- web_src/js/features/emoji.js | 4 ++-- 6 files changed, 10 insertions(+), 13 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 7961c5c930..aba287e0fd 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -1142,7 +1142,7 @@ func emojiShortCodeProcessor(ctx *RenderContext, node *html.Node) { converted := emoji.FromAlias(alias) if converted == nil { // check if this is a custom reaction - if _, exist := setting.UI.CustomEmojisMap[alias]; exist { + if setting.UI.CustomEmojisLookup.Contains(alias) { replaceContent(node, m[0], m[1], createCustomEmoji(alias)) node = node.NextSibling.NextSibling start = 0 diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 6bc0f7e56c..22c7e4ca5d 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -358,7 +358,7 @@ func TestRender_emoji(t *testing.T) { test( ":custom-emoji:", `

:custom-emoji:

`) - setting.UI.CustomEmojisMap["custom-emoji"] = ":custom-emoji:" + setting.UI.CustomEmojisLookup.Add("custom-emoji") test( ":custom-emoji:", `

:custom-emoji:

`) diff --git a/modules/setting/ui.go b/modules/setting/ui.go index 2e6a3df4c6..9dafe350eb 100644 --- a/modules/setting/ui.go +++ b/modules/setting/ui.go @@ -31,7 +31,7 @@ var UI = struct { Reactions []string ReactionsLookup container.Set[string] `ini:"-"` CustomEmojis []string - CustomEmojisMap map[string]string `ini:"-"` + CustomEmojisLookup container.Set[string] `ini:"-"` SearchRepoDescription bool OnlyShowRelevantRepos bool ExploreDefaultSort string `ini:"EXPLORE_PAGING_DEFAULT_SORT"` @@ -87,7 +87,6 @@ var UI = struct { Themes: []string{`forgejo-auto`, `forgejo-light`, `forgejo-dark`, `gitea-auto`, `gitea-light`, `gitea-dark`, `forgejo-auto-deuteranopia-protanopia`, `forgejo-light-deuteranopia-protanopia`, `forgejo-dark-deuteranopia-protanopia`, `forgejo-auto-tritanopia`, `forgejo-light-tritanopia`, `forgejo-dark-tritanopia`}, Reactions: []string{`+1`, `-1`, `laugh`, `hooray`, `confused`, `heart`, `rocket`, `eyes`}, CustomEmojis: []string{`git`, `gitea`, `codeberg`, `gitlab`, `github`, `gogs`, `forgejo`}, - CustomEmojisMap: map[string]string{"git": ":git:", "gitea": ":gitea:", "codeberg": ":codeberg:", "gitlab": ":gitlab:", "github": ":github:", "gogs": ":gogs:", "forgejo": ":forgejo:"}, ExploreDefaultSort: "recentupdate", PreferredTimestampTense: "mixed", @@ -163,8 +162,6 @@ func loadUIFrom(rootCfg ConfigProvider) { for _, reaction := range UI.Reactions { UI.ReactionsLookup.Add(reaction) } - UI.CustomEmojisMap = make(map[string]string) - for _, emoji := range UI.CustomEmojis { - UI.CustomEmojisMap[emoji] = ":" + emoji + ":" - } + UI.CustomEmojisLookup = make(container.Set[string]) + UI.CustomEmojisLookup.AddMultiple(UI.CustomEmojis...) } diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 42b4bad83c..8518290da0 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -128,8 +128,8 @@ func NewFuncMap() template.FuncMap { "AllowedReactions": func() []string { return setting.UI.Reactions }, - "CustomEmojis": func() map[string]string { - return setting.UI.CustomEmojisMap + "CustomEmojis": func() []string { + return setting.UI.CustomEmojis }, "MetaAuthor": func() string { return setting.UI.Meta.Author diff --git a/templates/base/head_script.tmpl b/templates/base/head_script.tmpl index d2774010b6..4bc274a73f 100644 --- a/templates/base/head_script.tmpl +++ b/templates/base/head_script.tmpl @@ -13,7 +13,7 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly. assetVersionEncoded: encodeURIComponent('{{AssetVersion}}'), // will be used in URL construction directly assetUrlPrefix: '{{AssetUrlPrefix}}', runModeIsProd: {{.RunModeIsProd}}, - customEmojis: {{CustomEmojis}}, + customEmojis: new Set({{CustomEmojis}}), csrfToken: '{{.CsrfToken}}', pageData: {{.PageData}}, notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}} diff --git a/web_src/js/features/emoji.js b/web_src/js/features/emoji.js index 032a3efe8a..76555bfe05 100644 --- a/web_src/js/features/emoji.js +++ b/web_src/js/features/emoji.js @@ -2,7 +2,7 @@ import emojis from '../../../assets/emoji.json'; const {assetUrlPrefix, customEmojis} = window.config; -const tempMap = {...customEmojis}; +const tempMap = Object.assign(...Array.from(customEmojis, (v) => ({[v]: `:${v}:`}))); for (const {emoji, aliases} of emojis) { for (const alias of aliases || []) { tempMap[alias] = emoji; @@ -23,7 +23,7 @@ for (const key of emojiKeys) { // retrieve HTML for given emoji name export function emojiHTML(name) { let inner; - if (Object.hasOwn(customEmojis, name)) { + if (customEmojis.has(name)) { inner = `:${name}:`; } else { inner = emojiString(name); From 1a466def75f18f6b50cf15621499ead33735d08b Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 10 Aug 2025 22:03:15 +0200 Subject: [PATCH 2/6] chore: add javascript unit test Verify that the operations in `emoji.js` still function correctly with normal, custom and non-existing emojis. --- web_src/js/emoji.test.js | 27 +++++++++++++++++++++++++++ web_src/js/vitest.setup.js | 3 ++- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 web_src/js/emoji.test.js diff --git a/web_src/js/emoji.test.js b/web_src/js/emoji.test.js new file mode 100644 index 0000000000..55e4787793 --- /dev/null +++ b/web_src/js/emoji.test.js @@ -0,0 +1,27 @@ +import {emojiString, emojiHTML} from './features/emoji.js'; + +test('emojiString', () => { + expect(emojiString('+1')).toEqual('👍'); + expect(emojiString('arrow_right')).toEqual('➡️'); + expect(emojiString('european_union')).toEqual('🇪🇺'); + expect(emojiString('eu')).toEqual('🇪🇺'); + + expect(emojiString('forgejo')).toEqual(':forgejo:'); + expect(emojiString('frogejo')).toEqual(':frogejo:'); + expect(emojiString('blobnom')).toEqual(':blobnom:'); + + expect(emojiString('not-a-emoji')).toEqual(':not-a-emoji:'); +}); + +test('emojiHTML', () => { + expect(emojiHTML('+1')).toEqual('👍'); + expect(emojiHTML('arrow_right')).toEqual('➡️'); + expect(emojiHTML('european_union')).toEqual('🇪🇺'); + expect(emojiHTML('eu')).toEqual('🇪🇺'); + + expect(emojiHTML('forgejo')).toEqual(':forgejo:'); + expect(emojiHTML('frogejo')).toEqual(':frogejo:'); + expect(emojiHTML('blobnom')).toEqual(':blobnom:'); + + expect(emojiHTML('not-a-emoji')).toEqual(':not-a-emoji:'); +}); diff --git a/web_src/js/vitest.setup.js b/web_src/js/vitest.setup.js index 5366958fb5..61a6453cf7 100644 --- a/web_src/js/vitest.setup.js +++ b/web_src/js/vitest.setup.js @@ -4,8 +4,9 @@ window.config = { csrfToken: 'test-csrf-token-123456', pageData: {}, i18n: {}, - customEmojis: {}, + customEmojis: new Set(['forgejo', 'frogejo', 'blobnom']), appSubUrl: '', + assetUrlPrefix: '/assets', mentionValues: [ {key: 'user1 User 1', value: 'user1', name: 'user1', fullname: 'User 1', avatar: 'https://avatar1.com'}, {key: 'user2 User 2', value: 'user2', name: 'user2', fullname: 'User 2', avatar: 'https://avatar2.com'}, From 9a8bdc6cbd295169113c9012ee4cc5ce95af783d Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 10 Aug 2025 22:08:59 +0200 Subject: [PATCH 3/6] chore: add integration test Check that the correct value ends up in `window.config`. --- tests/integration/window_config_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/integration/window_config_test.go diff --git a/tests/integration/window_config_test.go b/tests/integration/window_config_test.go new file mode 100644 index 0000000000..03fc2af52f --- /dev/null +++ b/tests/integration/window_config_test.go @@ -0,0 +1,20 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package integration + +import ( + "net/http" + "testing" + + "forgejo.org/tests" + + "github.com/stretchr/testify/assert" +) + +func TestWindowConfig(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + resp := MakeRequest(t, NewRequest(t, "GET", "/"), http.StatusOK) + assert.Contains(t, resp.Body.String(), `customEmojis: new Set(["git","gitea","codeberg","gitlab","github","gogs","forgejo"]),`) +} From fe5f16205f2209899b68a711888968f08a5034f2 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 10 Aug 2025 22:44:01 +0200 Subject: [PATCH 4/6] feat: make text expander aware of custom emojis The old MDE editor is aware of custom emojis and shows them as suggestions, but the new text expander is not aware of them and seems to re-implement some logic. Simplify it by using what `emoji.js` already provides. Custom emojis require a bit more work to get shown correctly (HTML and adding a `gap` to fake a space). --- web_src/js/features/comp/TextExpander.js | 14 ++++++++++---- web_src/js/features/emoji.js | 1 + web_src/js/utils/match.js | 17 ++++++----------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/web_src/js/features/comp/TextExpander.js b/web_src/js/features/comp/TextExpander.js index 128a2ddff0..8777f3a334 100644 --- a/web_src/js/features/comp/TextExpander.js +++ b/web_src/js/features/comp/TextExpander.js @@ -1,5 +1,6 @@ import {matchEmoji, matchMention} from '../../utils/match.js'; -import {emojiString} from '../emoji.js'; +import {emojiHTML, emojiString} from '../emoji.js'; +const {customEmojis} = window.config; export function initTextExpander(expander) { expander?.addEventListener('text-expander-change', ({detail: {key, provide, text}}) => { @@ -10,11 +11,16 @@ export function initTextExpander(expander) { const ul = document.createElement('ul'); ul.classList.add('suggestions'); for (const name of matches) { - const emoji = emojiString(name); const li = document.createElement('li'); li.setAttribute('role', 'option'); - li.setAttribute('data-value', emoji); - li.textContent = `${emoji} ${name}`; + li.setAttribute('data-value', emojiString(name)); + if (customEmojis.has(name)) { + li.style.gap = '0.25rem'; + li.innerHTML = emojiHTML(name); + li.append(name); + } else { + li.textContent = `${emojiString(name)} ${name}`; + } ul.append(li); } diff --git a/web_src/js/features/emoji.js b/web_src/js/features/emoji.js index 76555bfe05..c7a23ecf0f 100644 --- a/web_src/js/features/emoji.js +++ b/web_src/js/features/emoji.js @@ -10,6 +10,7 @@ for (const {emoji, aliases} of emojis) { } export const emojiKeys = Object.keys(tempMap).sort((a, b) => { + if (b === '+1' && a === '-1') return 1; if (a === '+1' || a === '-1') return -1; if (b === '+1' || b === '-1') return 1; return a.localeCompare(b); diff --git a/web_src/js/utils/match.js b/web_src/js/utils/match.js index 17fdfed113..aa53ad1435 100644 --- a/web_src/js/utils/match.js +++ b/web_src/js/utils/match.js @@ -1,4 +1,4 @@ -import emojis from '../../../assets/emoji.json'; +import {emojiKeys} from '../features/emoji.js'; const maxMatches = 6; @@ -9,19 +9,14 @@ function sortAndReduce(map) { export function matchEmoji(queryText) { const query = queryText.toLowerCase().replaceAll('_', ' '); - if (!query) return emojis.slice(0, maxMatches).map((e) => e.aliases[0]); + if (!query) return emojiKeys.slice(0, maxMatches); // results is a map of weights, lower is better const results = new Map(); - for (const {aliases} of emojis) { - const mainAlias = aliases[0]; - for (const [aliasIndex, alias] of aliases.entries()) { - const index = alias.replaceAll('_', ' ').indexOf(query); - if (index === -1) continue; - const existing = results.get(mainAlias); - const rankedIndex = index + aliasIndex; - results.set(mainAlias, existing ? existing - rankedIndex : rankedIndex); - } + for (const emojiKey of emojiKeys) { + const index = emojiKey.replaceAll('_', ' ').indexOf(query); + if (index === -1) continue; + results.set(emojiKey, index); } return sortAndReduce(results); From 4237603dd68e13c4e37f1614f1966a5f2a4bd369 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 10 Aug 2025 22:53:45 +0200 Subject: [PATCH 5/6] chore: add JS unit test Add a test to show that `matchEmoji` is aware of custom emojis. Also changes how aliases are handled (aliases are no longer correlated to each other and there's no preference for the main alias). --- web_src/js/utils/match.test.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/web_src/js/utils/match.test.js b/web_src/js/utils/match.test.js index 1e30b451d4..4910af1d3c 100644 --- a/web_src/js/utils/match.test.js +++ b/web_src/js/utils/match.test.js @@ -30,7 +30,7 @@ test('matchEmoji', () => { expect(matchEmoji('poo')).toEqual([ 'poodle', - 'hankey', + 'poop', 'spoon', 'bowl_with_spoon', ]); @@ -42,6 +42,19 @@ test('matchEmoji', () => { expect(matchEmoji('jellyfis')).toEqual([ 'jellyfish', ]); + + expect(matchEmoji('forge')).toEqual([ + 'forgejo', + ]); + + expect(matchEmoji('frog')).toEqual([ + 'frog', + 'frogejo', + ]); + + expect(matchEmoji('blob')).toEqual([ + 'blobnom', + ]); }); test('matchMention', () => { From b1a6d66cf571cc38f0033c194b220e019114db4a Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 10 Aug 2025 23:07:58 +0200 Subject: [PATCH 6/6] chore: add e2e test Check that emoji suggestion still work and that it is aware of custom emojis. --- tests/e2e/issue-comment.test.e2e.ts | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/e2e/issue-comment.test.e2e.ts b/tests/e2e/issue-comment.test.e2e.ts index 2017e4563e..bc2bc3d691 100644 --- a/tests/e2e/issue-comment.test.e2e.ts +++ b/tests/e2e/issue-comment.test.e2e.ts @@ -203,3 +203,36 @@ test('Pull quote reply', async ({page}, workerInfo) => { await editorTextarea.fill(''); }); + +test('Emoji suggestions', async ({page}) => { + const response = await page.goto('/user2/repo1/issues/1'); + expect(response?.status()).toBe(200); + + const textarea = page.locator('#comment-form textarea[name=content]'); + + await textarea.focus(); + await textarea.pressSequentially(':'); + + const suggestionList = page.locator('#comment-form .suggestions'); + await expect(suggestionList).toBeVisible(); + + const expectedSuggestions = [ + {emoji: '👍', name: '+1'}, + {emoji: '👎', name: '-1'}, + {emoji: '💯', name: '100'}, + {emoji: '🔢', name: '1234'}, + {emoji: '🥇', name: '1st_place_medal'}, + {emoji: '🥈', name: '2nd_place_medal'}, + ]; + + for (const {emoji, name} of expectedSuggestions) { + const item = suggestionList.locator(`li:has-text("${name}")`); + await expect(item).toContainText(`${emoji} ${name}`); + } + + await textarea.pressSequentially('forge'); + await expect(suggestionList).toBeVisible(); + + const item = suggestionList.locator(`li:has-text("forgejo")`); + await expect(item.locator('img')).toHaveAttribute('src', '/assets/img/emoji/forgejo.png'); +});