From 1403217ca4bbc4daba91a188d1b2757ed56d0a2d Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 12 Feb 2024 12:07:14 +0200 Subject: [PATCH] FEATURE: Async load of category and chat hashtags (#25526) This commit includes several changes to make hashtags work when "lazy load categories" is enabled. The previous hashtag implementation use the category colors CSS variables, but these are not defined when the site setting is enabled because categories are no longer preloaded. This commit implements two fundamental changes: 1. load colors together with the other hashtag information 2. load cooked hashtag data asynchronously The first change is implemented by adding "colors" to the HashtagItem model. It is a list because two colors are returned for subcategories: the color of the parent category and subcategory. The second change is implemented on the server-side in a new route /hashtags/by-ids and on the client side by loading previously unseen hashtags, generating the CSS on the fly and injecting it into the page. There have been minimal changes outside of these two fundamental ones, but a refactoring will be coming soon to reuse as much of the code and maybe favor use of `style` rather than injecting CSS into the page, which can lead to page rerenders and indefinite grow of the styles. --- .../hashtag-css-generator.js | 24 +--- .../discourse/app/lib/hashtag-autocomplete.js | 2 + .../discourse/app/lib/hashtag-decorator.js | 5 +- .../discourse/app/lib/hashtag-types/base.js | 63 ++++++++ .../app/lib/hashtag-types/category.js | 53 ++++--- .../tests/acceptance/css-generator-test.js | 15 +- .../acceptance/hashtag-autocomplete-test.js | 4 + .../common/components/hashtag.scss | 14 -- app/controllers/hashtags_controller.rb | 14 +- app/services/category_hashtag_data_source.rb | 7 +- app/services/hashtag_autocomplete_service.rb | 21 +++ config/routes.rb | 1 + .../discourse/lib/hashtag-types/channel.js | 21 ++- .../lib/chat/channel_hashtag_data_source.rb | 11 ++ .../chat/channel_hashtag_data_source_spec.rb | 48 +++++++ .../spec/system/hashtag_autocomplete_spec.rb | 6 +- .../acceptance/hashtag-css-generator-test.js | 9 +- spec/lib/pretty_text/helpers_spec.rb | 6 + spec/requests/hashtags_controller_spec.rb | 136 ++++++++++++++++++ .../category_hashtag_data_source_spec.rb | 14 ++ .../hashtag_autocomplete_service_spec.rb | 8 ++ spec/system/hashtag_autocomplete_spec.rb | 5 +- 22 files changed, 420 insertions(+), 67 deletions(-) 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 c31ed86b3f8..e14fd3e6949 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 @@ -9,33 +9,19 @@ export default { * cooked posts, and the sidebar. * * Each type has its own corresponding class, which is registered - * with the hastag type via api.registerHashtagType. The default + * with the hashtag type via api.registerHashtagType. The default * ones in core are CategoryHashtagType and TagHashtagType. */ initialize(owner) { this.site = owner.lookup("service:site"); - // If the site is login_required and the user is anon there will be no categories - // preloaded, so there will be no category color CSS variables generated by - // the category-color-css-generator initializer. - if (!this.site.categories?.length) { - return; - } - - let generatedCssClasses = []; - - Object.values(getHashtagTypeClasses()).forEach((hashtagType) => { - hashtagType.preloadedData.forEach((model) => { - generatedCssClasses = generatedCssClasses.concat( - hashtagType.generateColorCssClasses(model) - ); - }); - }); - const cssTag = document.createElement("style"); cssTag.type = "text/css"; cssTag.id = "hashtag-css-generator"; - cssTag.innerHTML = generatedCssClasses.join("\n"); + cssTag.innerHTML = Object.values(getHashtagTypeClasses()) + .map((hashtagType) => hashtagType.generatePreloadedCssClasses()) + .flat() + .join("\n"); document.head.appendChild(cssTag); }, }; diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js index 8f112a0c482..f36ba31a83b 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js @@ -228,6 +228,8 @@ function _searchRequest(term, contextualHashtagConfiguration, resultFunc) { const hashtagType = getHashtagTypeClassesNew()[result.type]; result.icon = hashtagType.generateIconHTML({ + preloaded: true, + colors: result.colors, icon: result.icon, id: result.id, }); diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js b/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js index c8584e653d4..5ab5df04b9f 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-decorator.js @@ -63,7 +63,10 @@ function _findAndReplaceSeenHashtagPlaceholder( // Replace raw span for the hashtag with a cooked one const matchingSeenHashtag = seenHashtags[type]?.[slugRef]; if (matchingSeenHashtag) { - generatePlaceholderHashtagHTML(type, hashtagSpan, matchingSeenHashtag); + generatePlaceholderHashtagHTML(type, hashtagSpan, { + preloaded: true, + ...matchingSeenHashtag, + }); } }); } diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-types/base.js b/app/assets/javascripts/discourse/app/lib/hashtag-types/base.js index 4de8a448250..f8fe5fc1ebd 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-types/base.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-types/base.js @@ -1,8 +1,37 @@ import { setOwner } from "@ember/application"; +import { debounce } from "@ember/runloop"; +import { ajax } from "discourse/lib/ajax"; +import { getHashtagTypeClasses } from "discourse/lib/hashtag-type-registry"; export default class HashtagTypeBase { + // Store a list of IDs that are currently being loaded globally to make it + // easier to batch requests for multiple types of hashtags + static loadingIds = {}; + + static async _load() { + const data = HashtagTypeBase.loadingIds; + HashtagTypeBase.loadingIds = {}; + + let hasData = false; + Object.keys(data).forEach((type) => { + hasData ||= data[type].size > 0; + data[type] = [...data[type]]; // Set to Array + }); + + if (!hasData) { + return; + } + + const hashtags = await ajax("/hashtags/by-ids", { data }); + const typeClasses = getHashtagTypeClasses(); + Object.entries(typeClasses).forEach(([type, typeClass]) => + hashtags[type]?.forEach((hashtag) => typeClass.onLoad(hashtag)) + ); + } + constructor(owner) { setOwner(this, owner); + this.loadedIds = new Set(); } get type() { @@ -13,6 +42,15 @@ export default class HashtagTypeBase { throw "not implemented"; } + generatePreloadedCssClasses() { + const cssClasses = []; + this.preloadedData.forEach((model) => { + this.loadedIds.add(model.id); + cssClasses.push(this.generateColorCssClasses(model)); + }); + return cssClasses.flat(); + } + generateColorCssClasses() { throw "not implemented"; } @@ -20,4 +58,29 @@ export default class HashtagTypeBase { generateIconHTML() { throw "not implemented"; } + + isLoaded(id) { + id = parseInt(id, 10); + return this.loadedIds.has(id); + } + + load(id) { + id = parseInt(id, 10); + if (!this.isLoaded(id)) { + (HashtagTypeBase.loadingIds[this.type] ||= new Set()).add(id); + debounce(HashtagTypeBase, HashtagTypeBase._load, 100, false); + } + } + + onLoad(hashtag) { + const hashtagId = parseInt(hashtag.id, 10); + if (!this.isLoaded(hashtagId)) { + this.loadedIds.add(hashtagId); + + // Append the styles for the loaded hashtag to the CSS generated by the + // `hashtag-css-generator` initializer for preloaded models + document.querySelector("#hashtag-css-generator").innerHTML += + "\n" + this.generateColorCssClasses(hashtag).join("\n"); + } + } } diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js b/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js index c362bf32564..24cac59c4a5 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js @@ -12,29 +12,50 @@ export default class CategoryHashtagType extends HashtagTypeBase { return this.site.categories || []; } - generateColorCssClasses(category) { - const generatedCssClasses = []; - const backgroundGradient = [`var(--category-${category.id}-color) 50%`]; - if (category.parentCategory) { - backgroundGradient.push( - `var(--category-${category.parentCategory.id}-color) 50%` - ); + generatePreloadedCssClasses() { + return [ + // Set a default color for category hashtags. This is added here instead + // of `hashtag.scss` because of the CSS precedence rules ( has a + // higher precedence than