From c7860173c1683fcbe33600d29b5294c0bb228253 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 29 Jan 2024 09:48:56 +1000 Subject: [PATCH] DEV: Clean up hashtag code (#25397) * Delete dead code * Split up hashtag-autocomplete into more logical modules --- .../app/components/composer-editor.js | 2 +- .../discourse/app/components/d-editor.js | 6 +- .../hashtag-css-generator.js | 2 +- .../hashtag-post-decorations.js | 2 +- .../discourse/app/lib/hashtag-autocomplete.js | 196 ++++++++---------- .../discourse/app/lib/hashtag-decorator.js | 109 ++++++++++ .../app/lib/hashtag-type-registry.js | 10 + .../discourse/app/lib/link-hashtags.js | 66 ------ .../discourse/app/lib/plugin-api.js | 2 +- .../discourse/tests/helpers/qunit-helpers.js | 2 +- .../discourse/initializers/chat-decorators.js | 2 +- .../discourse/lib/transform-auto-links.js | 2 +- 12 files changed, 214 insertions(+), 187 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/hashtag-decorator.js create mode 100644 app/assets/javascripts/discourse/app/lib/hashtag-type-registry.js delete mode 100644 app/assets/javascripts/discourse/app/lib/link-hashtags.js diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 3d883afa0fd..41918025cc5 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -10,7 +10,7 @@ import { ajax } from "discourse/lib/ajax"; import { fetchUnseenHashtagsInContext, linkSeenHashtagsInContext, -} from "discourse/lib/hashtag-autocomplete"; +} from "discourse/lib/hashtag-decorator"; import { fetchUnseenMentions, linkSeenMentions, diff --git a/app/assets/javascripts/discourse/app/components/d-editor.js b/app/assets/javascripts/discourse/app/components/d-editor.js index 26c22f1b5c8..2ecf3ea20b6 100644 --- a/app/assets/javascripts/discourse/app/components/d-editor.js +++ b/app/assets/javascripts/discourse/app/components/d-editor.js @@ -11,10 +11,8 @@ import { Promise } from "rsvp"; import InsertHyperlink from "discourse/components/modal/insert-hyperlink"; import { ajax } from "discourse/lib/ajax"; import { SKIP } from "discourse/lib/autocomplete"; -import { - linkSeenHashtagsInContext, - setupHashtagAutocomplete, -} from "discourse/lib/hashtag-autocomplete"; +import { setupHashtagAutocomplete } from "discourse/lib/hashtag-autocomplete"; +import { linkSeenHashtagsInContext } from "discourse/lib/hashtag-decorator"; import { wantsNewWindow } from "discourse/lib/intercept-click"; import { PLATFORM_KEY_MODIFIER } from "discourse/lib/keyboard-shortcuts"; import { linkSeenMentions } from "discourse/lib/link-mentions"; diff --git a/app/assets/javascripts/discourse/app/instance-initializers/hashtag-css-generator.js b/app/assets/javascripts/discourse/app/instance-initializers/hashtag-css-generator.js index a5b584ce961..c31ed86b3f8 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/hashtag-css-generator.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/hashtag-css-generator.js @@ -1,4 +1,4 @@ -import { getHashtagTypeClasses } from "discourse/lib/hashtag-autocomplete"; +import { getHashtagTypeClasses } from "discourse/lib/hashtag-type-registry"; export default { after: "category-color-css-generator", diff --git a/app/assets/javascripts/discourse/app/instance-initializers/hashtag-post-decorations.js b/app/assets/javascripts/discourse/app/instance-initializers/hashtag-post-decorations.js index d869ca2cccc..dd54a16d073 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/hashtag-post-decorations.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/hashtag-post-decorations.js @@ -1,4 +1,4 @@ -import { decorateHashtags } from "discourse/lib/hashtag-autocomplete"; +import { decorateHashtags } from "discourse/lib/hashtag-decorator"; import { withPluginApi } from "discourse/lib/plugin-api"; export default { diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js index 7a6175d7d0b..8f112a0c482 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js @@ -2,6 +2,17 @@ import { cancel } from "@ember/runloop"; import { htmlSafe } from "@ember/template"; import { ajax } from "discourse/lib/ajax"; import { CANCELLED_STATUS } from "discourse/lib/autocomplete"; +import { + decorateHashtags as decorateHashtagsNew, + fetchUnseenHashtagsInContext as fetchUnseenHashtagsInContextNew, + generatePlaceholderHashtagHTML as generatePlaceholderHashtagHTMLNew, + linkSeenHashtagsInContext as linkSeenHashtagsInContextNew, +} from "discourse/lib/hashtag-decorator"; +import { + cleanUpHashtagTypeClasses as cleanUpHashtagTypeClassesNew, + getHashtagTypeClasses as getHashtagTypeClassesNew, + registerHashtagType as registerHashtagTypeNew, +} from "discourse/lib/hashtag-type-registry"; import { emojiUnescape } from "discourse/lib/text"; import { caretPosition, @@ -10,58 +21,88 @@ import { } from "discourse/lib/utilities"; import { INPUT_DELAY, isTesting } from "discourse-common/config/environment"; import discourseDebounce from "discourse-common/lib/debounce"; -import domFromString from "discourse-common/lib/dom-from-string"; +import deprecated from "discourse-common/lib/deprecated"; import discourseLater from "discourse-common/lib/later"; import { findRawTemplate } from "discourse-common/lib/raw-templates"; -let hashtagTypeClasses = {}; -export function registerHashtagType(type, typeClassInstance) { - hashtagTypeClasses[type] = typeClassInstance; +// TODO (martin) Remove this once plugins have changed to use hashtag-decorator and +// hashtag-type-registry imports +export function fetchUnseenHashtagsInContext() { + deprecated( + `fetchUnseenHashtagsInContext is has been moved to the module 'discourse/lib/hashtag-decorator'`, + { + id: "discourse.hashtag.fetchUnseenHashtagsInContext", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", + } + ); + return fetchUnseenHashtagsInContextNew(...arguments); } -export function cleanUpHashtagTypeClasses() { - hashtagTypeClasses = {}; +export function linkSeenHashtagsInContext() { + deprecated( + `linkSeenHashtagsInContext is has been moved to the module 'discourse/lib/hashtag-decorator'`, + { + id: "discourse.hashtag.linkSeenHashtagsInContext", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", + } + ); + return linkSeenHashtagsInContextNew(...arguments); +} +export function generatePlaceholderHashtagHTML() { + deprecated( + `generatePlaceholderHashtagHTML is has been moved to the module 'discourse/lib/hashtag-decorator'`, + { + id: "discourse.hashtag.generatePlaceholderHashtagHTML", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", + } + ); + return generatePlaceholderHashtagHTMLNew(...arguments); +} +export function decorateHashtags() { + deprecated( + `decorateHashtags is has been moved to the module 'discourse/lib/hashtag-decorator'`, + { + id: "discourse.hashtag.decorateHashtags", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", + } + ); + return decorateHashtagsNew(...arguments); } export function getHashtagTypeClasses() { - return hashtagTypeClasses; -} -export function decorateHashtags(element, site) { - element.querySelectorAll(".hashtag-cooked").forEach((hashtagEl) => { - // Replace the empty icon placeholder span with actual icon HTML. - const iconPlaceholderEl = hashtagEl.querySelector( - ".hashtag-icon-placeholder" - ); - const hashtagType = hashtagEl.dataset.type; - const hashtagTypeClass = getHashtagTypeClasses()[hashtagType]; - if (iconPlaceholderEl && hashtagTypeClass) { - const hashtagIconHTML = hashtagTypeClass - .generateIconHTML({ - icon: site.hashtag_icons[hashtagType], - id: hashtagEl.dataset.id, - }) - .trim(); - iconPlaceholderEl.replaceWith(domFromString(hashtagIconHTML)[0]); + deprecated( + `getHashtagTypeClasses is has been moved to the module 'discourse/lib/hashtag-type-registry'`, + { + id: "discourse.hashtag.getHashtagTypeClasses", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", } - - // Add an aria-label to the hashtag element so that screen readers - // can read the hashtag text. - hashtagEl.setAttribute("aria-label", `${hashtagEl.innerText.trim()}`); - }); + ); + return getHashtagTypeClassesNew(...arguments); } - -export function generatePlaceholderHashtagHTML(type, spanEl, data) { - // NOTE: When changing the HTML structure here, you must also change - // it in the hashtag-autocomplete markdown rule, and vice-versa. - const link = document.createElement("a"); - link.classList.add("hashtag-cooked"); - link.href = data.relative_url; - link.dataset.type = type; - link.dataset.id = data.id; - link.dataset.slug = data.slug; - const hashtagTypeClass = new getHashtagTypeClasses()[type]; - link.innerHTML = `${hashtagTypeClass.generateIconHTML( - data - )}${emojiUnescape(data.text)}`; - spanEl.replaceWith(link); +export function registerHashtagType() { + deprecated( + `registerHashtagType is has been moved to the module 'discourse/lib/hashtag-type-registry'`, + { + id: "discourse.hashtag.registerHashtagType", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", + } + ); + return registerHashtagTypeNew(...arguments); +} +export function cleanUpHashtagTypeClasses() { + deprecated( + `cleanUpHashtagTypeClasses is has been moved to the module 'discourse/lib/hashtag-type-registry'`, + { + id: "discourse.hashtag.cleanUpHashtagTypeClasses", + since: "3.2.0.beta5-dev", + dropFrom: "3.2.1", + } + ); + return cleanUpHashtagTypeClassesNew(...arguments); } /** @@ -107,57 +148,6 @@ export function hashtagTriggerRule(textarea) { return true; } -const checkedHashtags = new Set(); -let seenHashtags = {}; - -// NOTE: For future maintainers, the hashtag lookup here does not take -// into account mixed contexts -- for instance, a chat quote inside a post -// or a post quote inside a chat message, so this may -// not provide an accurate priority lookup for hashtags without a ::type suffix in those -// cases. -export function fetchUnseenHashtagsInContext( - contextualHashtagConfiguration, - slugs -) { - return ajax("/hashtags", { - data: { slugs, order: contextualHashtagConfiguration }, - }).then((response) => { - Object.keys(response).forEach((type) => { - seenHashtags[type] = seenHashtags[type] || {}; - response[type].forEach((item) => { - seenHashtags[type][item.ref] = seenHashtags[type][item.ref] || item; - }); - }); - slugs.forEach(checkedHashtags.add, checkedHashtags); - }); -} - -export function linkSeenHashtagsInContext( - contextualHashtagConfiguration, - elem -) { - const hashtagSpans = [...(elem?.querySelectorAll("span.hashtag-raw") || [])]; - if (hashtagSpans.length === 0) { - return []; - } - const slugs = [ - ...hashtagSpans.map((span) => span.innerText.replace("#", "")), - ]; - - hashtagSpans.forEach((hashtagSpan, index) => { - _findAndReplaceSeenHashtagPlaceholder( - slugs[index], - contextualHashtagConfiguration, - hashtagSpan - ); - }); - - return slugs - .map((slug) => slug.toLowerCase()) - .uniq() - .filter((slug) => !checkedHashtags.has(slug)); -} - function _setup( contextualHashtagConfiguration, $textArea, @@ -236,7 +226,7 @@ function _searchRequest(term, contextualHashtagConfiguration, resultFunc) { // Convert :emoji: in the result text to HTML safely. result.text = htmlSafe(emojiUnescape(escapeExpression(result.text))); - const hashtagType = getHashtagTypeClasses()[result.type]; + const hashtagType = getHashtagTypeClassesNew()[result.type]; result.icon = hashtagType.generateIconHTML({ icon: result.icon, id: result.id, @@ -249,17 +239,3 @@ function _searchRequest(term, contextualHashtagConfiguration, resultFunc) { }); return currentSearch; } - -function _findAndReplaceSeenHashtagPlaceholder( - slugRef, - contextualHashtagConfiguration, - hashtagSpan -) { - contextualHashtagConfiguration.forEach((type) => { - // Replace raw span for the hashtag with a cooked one - const matchingSeenHashtag = seenHashtags[type]?.[slugRef]; - if (matchingSeenHashtag) { - generatePlaceholderHashtagHTML(type, hashtagSpan, matchingSeenHashtag); - } - }); -} diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js b/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js new file mode 100644 index 00000000000..c8584e653d4 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js @@ -0,0 +1,109 @@ +import { ajax } from "discourse/lib/ajax"; +import { getHashtagTypeClasses } from "discourse/lib/hashtag-type-registry"; +import { emojiUnescape } from "discourse/lib/text"; +import domFromString from "discourse-common/lib/dom-from-string"; + +const checkedHashtags = new Set(); +let seenHashtags = {}; + +// NOTE: For future maintainers, the hashtag lookup here does not take +// into account mixed contexts -- for instance, a chat quote inside a post +// or a post quote inside a chat message, so this may +// not provide an accurate priority lookup for hashtags without a ::type suffix in those +// cases. +export function fetchUnseenHashtagsInContext( + contextualHashtagConfiguration, + slugs +) { + return ajax("/hashtags", { + data: { slugs, order: contextualHashtagConfiguration }, + }).then((response) => { + Object.keys(response).forEach((type) => { + seenHashtags[type] = seenHashtags[type] || {}; + response[type].forEach((item) => { + seenHashtags[type][item.ref] = seenHashtags[type][item.ref] || item; + }); + }); + slugs.forEach(checkedHashtags.add, checkedHashtags); + }); +} + +export function linkSeenHashtagsInContext( + contextualHashtagConfiguration, + elem +) { + const hashtagSpans = [...(elem?.querySelectorAll("span.hashtag-raw") || [])]; + if (hashtagSpans.length === 0) { + return []; + } + const slugs = [ + ...hashtagSpans.map((span) => span.innerText.replace("#", "")), + ]; + + hashtagSpans.forEach((hashtagSpan, index) => { + _findAndReplaceSeenHashtagPlaceholder( + slugs[index], + contextualHashtagConfiguration, + hashtagSpan + ); + }); + + return slugs + .map((slug) => slug.toLowerCase()) + .uniq() + .filter((slug) => !checkedHashtags.has(slug)); +} + +function _findAndReplaceSeenHashtagPlaceholder( + slugRef, + contextualHashtagConfiguration, + hashtagSpan +) { + contextualHashtagConfiguration.forEach((type) => { + // Replace raw span for the hashtag with a cooked one + const matchingSeenHashtag = seenHashtags[type]?.[slugRef]; + if (matchingSeenHashtag) { + generatePlaceholderHashtagHTML(type, hashtagSpan, matchingSeenHashtag); + } + }); +} + +export function generatePlaceholderHashtagHTML(type, spanEl, data) { + // NOTE: When changing the HTML structure here, you must also change + // it in the hashtag-autocomplete markdown rule, and vice-versa. + const link = document.createElement("a"); + link.classList.add("hashtag-cooked"); + link.href = data.relative_url; + link.dataset.type = type; + link.dataset.id = data.id; + link.dataset.slug = data.slug; + const hashtagTypeClass = new getHashtagTypeClasses()[type]; + link.innerHTML = `${hashtagTypeClass.generateIconHTML( + data + )}${emojiUnescape(data.text)}`; + spanEl.replaceWith(link); +} + +export function decorateHashtags(element, site) { + element.querySelectorAll(".hashtag-cooked").forEach((hashtagEl) => { + // Replace the empty icon placeholder span with actual icon HTML. + const iconPlaceholderEl = hashtagEl.querySelector( + ".hashtag-icon-placeholder" + ); + const hashtagType = hashtagEl.dataset.type; + const hashtagTypeClass = getHashtagTypeClasses()[hashtagType]; + if (iconPlaceholderEl && hashtagTypeClass) { + const hashtagIconHTML = hashtagTypeClass + .generateIconHTML({ + icon: site.hashtag_icons[hashtagType], + id: hashtagEl.dataset.id, + }) + .trim(); + iconPlaceholderEl.replaceWith(domFromString(hashtagIconHTML)[0]); + } + + // Add an aria-label to the hashtag element so that screen readers + // can read the hashtag text. + hashtagEl.setAttribute("aria-label", `${hashtagEl.innerText.trim()}`); + }); +} diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-type-registry.js b/app/assets/javascripts/discourse/app/lib/hashtag-type-registry.js new file mode 100644 index 00000000000..24d6bc773cd --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/hashtag-type-registry.js @@ -0,0 +1,10 @@ +let hashtagTypeClasses = {}; +export function registerHashtagType(type, typeClassInstance) { + hashtagTypeClasses[type] = typeClassInstance; +} +export function cleanUpHashtagTypeClasses() { + hashtagTypeClasses = {}; +} +export function getHashtagTypeClasses() { + return hashtagTypeClasses; +} diff --git a/app/assets/javascripts/discourse/app/lib/link-hashtags.js b/app/assets/javascripts/discourse/app/lib/link-hashtags.js deleted file mode 100644 index 05d4cd9b60c..00000000000 --- a/app/assets/javascripts/discourse/app/lib/link-hashtags.js +++ /dev/null @@ -1,66 +0,0 @@ -// TODO (martin) Delete this after core PR and any other PRs that depend -// on this file (e.g. discourse-encrypt) are merged. - -import $ from "jquery"; -import { ajax } from "discourse/lib/ajax"; -import { replaceSpan } from "discourse/lib/category-hashtags"; -import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags"; -import deprecated from "discourse-common/lib/deprecated"; - -const categoryHashtags = {}; -const tagHashtags = {}; -const checkedHashtags = new Set(); - -export function linkSeenHashtags(elem) { - if (elem instanceof $) { - elem = elem[0]; - - deprecated("linkSeenHashtags now expects a DOM node as first parameter", { - since: "2.8.0.beta7", - dropFrom: "2.9.0.beta1", - id: "discourse.link-hashtags.dom-node", - }); - } - - const hashtags = [...(elem?.querySelectorAll("span.hashtag") || [])]; - if (hashtags.length === 0) { - return []; - } - const slugs = [...hashtags.map((hashtag) => hashtag.innerText.slice(1))]; - - hashtags.forEach((hashtag, index) => { - let slug = slugs[index]; - const hasTagSuffix = slug.endsWith(TAG_HASHTAG_POSTFIX); - if (hasTagSuffix) { - slug = slug.slice(0, slug.length - TAG_HASHTAG_POSTFIX.length); - } - - const lowerSlug = slug.toLowerCase(); - if (categoryHashtags[lowerSlug] && !hasTagSuffix) { - replaceSpan($(hashtag), slug, categoryHashtags[lowerSlug]); - } else if (tagHashtags[lowerSlug]) { - replaceSpan($(hashtag), slug, tagHashtags[lowerSlug]); - } - }); - - return slugs - .map((slug) => slug.toLowerCase()) - .uniq() - .filter((slug) => !checkedHashtags.has(slug)); -} - -export function fetchUnseenHashtags(slugs) { - return ajax("/hashtags", { - data: { slugs }, - }).then((response) => { - Object.keys(response.categories).forEach((slug) => { - categoryHashtags[slug] = response.categories[slug]; - }); - - Object.keys(response.tags).forEach((slug) => { - tagHashtags[slug] = response.tags[slug]; - }); - - slugs.forEach(checkedHashtags.add, checkedHashtags); - }); -} diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index f1607820323..06afcdd3e83 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -46,7 +46,7 @@ import { addBeforeAuthCompleteCallback } from "discourse/instance-initializers/a import { addPopupMenuOption } from "discourse/lib/composer/custom-popup-menu-options"; import { registerDesktopNotificationHandler } from "discourse/lib/desktop-notifications"; import { downloadCalendar } from "discourse/lib/download-calendar"; -import { registerHashtagType } from "discourse/lib/hashtag-autocomplete"; +import { registerHashtagType } from "discourse/lib/hashtag-type-registry"; import { registerHighlightJSLanguage, registerHighlightJSPlugin, diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 3e995b7231a..8beee715bd8 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -31,7 +31,7 @@ import { resetUsernameDecorators } from "discourse/helpers/decorate-username-sel import { resetBeforeAuthCompleteCallbacks } from "discourse/instance-initializers/auth-complete"; import { clearPopupMenuOptions } from "discourse/lib/composer/custom-popup-menu-options"; import { clearDesktopNotificationHandlers } from "discourse/lib/desktop-notifications"; -import { cleanUpHashtagTypeClasses } from "discourse/lib/hashtag-autocomplete"; +import { cleanUpHashtagTypeClasses } from "discourse/lib/hashtag-type-registry"; import { clearExtraKeyboardShortcutHelp, PLATFORM_KEY_MODIFIER, diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js index 974feee21c3..34fbb3977d7 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js @@ -1,7 +1,7 @@ import $ from "jquery"; import { spinnerHTML } from "discourse/helpers/loading-spinner"; import { decorateGithubOneboxBody } from "discourse/instance-initializers/onebox-decorators"; -import { decorateHashtags } from "discourse/lib/hashtag-autocomplete"; +import { decorateHashtags } from "discourse/lib/hashtag-decorator"; import highlightSyntax from "discourse/lib/highlight-syntax"; import loadScript from "discourse/lib/load-script"; import { withPluginApi } from "discourse/lib/plugin-api"; diff --git a/plugins/chat/assets/javascripts/discourse/lib/transform-auto-links.js b/plugins/chat/assets/javascripts/discourse/lib/transform-auto-links.js index e6696fc480f..08f45a9de93 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/transform-auto-links.js +++ b/plugins/chat/assets/javascripts/discourse/lib/transform-auto-links.js @@ -1,4 +1,4 @@ -import { generatePlaceholderHashtagHTML } from "discourse/lib/hashtag-autocomplete"; +import { generatePlaceholderHashtagHTML } from "discourse/lib/hashtag-decorator"; import getURL from "discourse-common/lib/get-url"; const domParser = new DOMParser();