From 0b3cf83e3c7d03c2dcd54f86de60efe32f385a44 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 23 May 2023 09:33:55 +0200 Subject: [PATCH] FIX: Do not cook icon with hashtags (#21676) This commit makes some fundamental changes to how hashtag cooking and icon generation works in the new experimental hashtag autocomplete mode. Previously we cooked the appropriate SVG icon with the cooked hashtag, though this has proved inflexible especially for theming purposes. Instead, we now cook a data-ID attribute with the hashtag and add a new span as an icon placeholder. This is replaced on the client side with an icon (or a square span in the case of categories) on the client side via the decorateCooked API for posts and chat messages. This client side logic uses the generated hashtag, category, and channel CSS classes added in a previous commit. This is missing changes to the sidebar to use the new generated CSS classes and also colors and the split square for categories in the hashtag autocomplete menu -- I will tackle this in a separate PR so it is clearer. --- .../app/initializers/hashtag-css-generator.js | 3 +- .../initializers/hashtag-post-decorations.js | 24 +++++++ .../initializers/register-hashtag-types.js | 6 +- .../discourse/app/lib/click-track.js | 2 +- .../discourse/app/lib/hashtag-autocomplete.js | 37 +++++++++-- .../discourse/app/lib/hashtag-types/base.js | 4 ++ .../app/lib/hashtag-types/category.js | 16 +++-- .../discourse/app/lib/hashtag-types/tag.js | 7 ++ .../discourse/app/lib/plugin-api.js | 7 +- .../discourse/tests/fixtures/site-fixtures.js | 2 +- .../hashtag-autocomplete.js | 66 ++++++++----------- .../discourse-markdown/watched-words.js | 22 ++++++- .../common/components/hashtag.scss | 8 +++ app/serializers/site_serializer.rb | 2 +- app/services/category_hashtag_data_source.rb | 1 + app/services/hashtag_autocomplete_service.rb | 10 ++- app/services/tag_hashtag_data_source.rb | 7 +- lib/excerpt_parser.rb | 8 ++- lib/pretty_text.rb | 4 +- .../discourse/initializers/chat-decorators.js | 7 ++ .../discourse/initializers/chat-setup.js | 2 +- .../discourse/lib/hashtag-types/channel.js | 11 +++- .../lib/chat/channel_hashtag_data_source.rb | 1 + .../lib/chat/channel_archive_service_spec.rb | 2 +- .../chat/channel_hashtag_data_source_spec.rb | 5 ++ plugins/chat/spec/models/chat/message_spec.rb | 2 +- .../spec/system/hashtag_autocomplete_spec.rb | 6 +- .../acceptance/hashtag-css-generator-test.js | 2 +- spec/lib/email/sender_spec.rb | 2 +- spec/lib/oneboxer_spec.rb | 15 +++-- spec/lib/pretty_text/helpers_spec.rb | 6 ++ spec/lib/pretty_text_spec.rb | 15 +++-- .../api/schemas/json/site_response.json | 2 +- spec/requests/hashtags_controller_spec.rb | 9 +++ .../hashtag_autocomplete_service_spec.rb | 6 +- spec/system/hashtag_autocomplete_spec.rb | 8 +-- 36 files changed, 235 insertions(+), 102 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/initializers/hashtag-post-decorations.js diff --git a/app/assets/javascripts/discourse/app/initializers/hashtag-css-generator.js b/app/assets/javascripts/discourse/app/initializers/hashtag-css-generator.js index 44739229fbe..5555962647d 100644 --- a/app/assets/javascripts/discourse/app/initializers/hashtag-css-generator.js +++ b/app/assets/javascripts/discourse/app/initializers/hashtag-css-generator.js @@ -25,8 +25,7 @@ export default { let generatedCssClasses = []; - Object.values(getHashtagTypeClasses()).forEach((hashtagTypeClass) => { - const hashtagType = new hashtagTypeClass(container); + Object.values(getHashtagTypeClasses()).forEach((hashtagType) => { hashtagType.preloadedData.forEach((model) => { generatedCssClasses = generatedCssClasses.concat( hashtagType.generateColorCssClasses(model) diff --git a/app/assets/javascripts/discourse/app/initializers/hashtag-post-decorations.js b/app/assets/javascripts/discourse/app/initializers/hashtag-post-decorations.js new file mode 100644 index 00000000000..a14d6c7b4e5 --- /dev/null +++ b/app/assets/javascripts/discourse/app/initializers/hashtag-post-decorations.js @@ -0,0 +1,24 @@ +import { withPluginApi } from "discourse/lib/plugin-api"; +import { replaceHashtagIconPlaceholder } from "discourse/lib/hashtag-autocomplete"; + +export default { + name: "hashtag-post-decorations", + after: "hashtag-css-generator", + + initialize(container) { + const siteSettings = container.lookup("service:site-settings"); + const site = container.lookup("service:site"); + + withPluginApi("0.8.7", (api) => { + if (siteSettings.enable_experimental_hashtag_autocomplete) { + api.decorateCookedElement( + (post) => replaceHashtagIconPlaceholder(post, site), + { + onlyStream: true, + id: "hashtag-icons", + } + ); + } + }); + }, +}; diff --git a/app/assets/javascripts/discourse/app/initializers/register-hashtag-types.js b/app/assets/javascripts/discourse/app/initializers/register-hashtag-types.js index 990ce2dab60..ab67d25d539 100644 --- a/app/assets/javascripts/discourse/app/initializers/register-hashtag-types.js +++ b/app/assets/javascripts/discourse/app/initializers/register-hashtag-types.js @@ -6,10 +6,10 @@ export default { name: "register-hashtag-types", before: "hashtag-css-generator", - initialize() { + initialize(container) { withPluginApi("0.8.7", (api) => { - api.registerHashtagType("category", CategoryHashtagType); - api.registerHashtagType("tag", TagHashtagType); + api.registerHashtagType("category", new CategoryHashtagType(container)); + api.registerHashtagType("tag", new TagHashtagType(container)); }); }, }; diff --git a/app/assets/javascripts/discourse/app/lib/click-track.js b/app/assets/javascripts/discourse/app/lib/click-track.js index 04290714033..5e8cde5f431 100644 --- a/app/assets/javascripts/discourse/app/lib/click-track.js +++ b/app/assets/javascripts/discourse/app/lib/click-track.js @@ -46,7 +46,7 @@ export function isValidLink(link) { return ( link.classList.contains("track-link") || !link.closest( - ".hashtag, .hashtag-cooked, .badge-category, .onebox-result, .onebox-body" + ".hashtag, .hashtag-cooked, .hashtag-icon-placeholder, .badge-category, .onebox-result, .onebox-body" ) ); } diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js index d08e7646877..c3f02823d2b 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js @@ -1,4 +1,5 @@ import { findRawTemplate } from "discourse-common/lib/raw-templates"; +import domFromString from "discourse-common/lib/dom-from-string"; import discourseLater from "discourse-common/lib/later"; import { INPUT_DELAY, isTesting } from "discourse-common/config/environment"; import { cancel } from "@ember/runloop"; @@ -15,8 +16,8 @@ import { emojiUnescape } from "discourse/lib/text"; import { htmlSafe } from "@ember/template"; let hashtagTypeClasses = {}; -export function registerHashtagType(type, typeClass) { - hashtagTypeClasses[type] = typeClass; +export function registerHashtagType(type, typeClassInstance) { + hashtagTypeClasses[type] = typeClassInstance; } export function cleanUpHashtagTypeClasses() { hashtagTypeClasses = {}; @@ -24,6 +25,24 @@ export function cleanUpHashtagTypeClasses() { export function getHashtagTypeClasses() { return hashtagTypeClasses; } +export function replaceHashtagIconPlaceholder(element, site) { + element.querySelectorAll(".hashtag-cooked").forEach((hashtagEl) => { + 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]); + } + }); +} /** * Sets up a textarea using the jQuery autocomplete plugin, specifically @@ -216,12 +235,12 @@ function _searchRequest(term, contextualHashtagConfiguration, resultFunc) { data: { term, order: contextualHashtagConfiguration }, }); currentSearch - .then((r) => { - r.results?.forEach((result) => { + .then((response) => { + response.results?.forEach((result) => { // Convert :emoji: in the result text to HTML safely. result.text = htmlSafe(emojiUnescape(escapeExpression(result.text))); }); - resultFunc(r.results || CANCELLED_STATUS); + resultFunc(response.results || CANCELLED_STATUS); }) .finally(() => { currentSearch = null; @@ -235,7 +254,7 @@ function _findAndReplaceSeenHashtagPlaceholder( hashtagSpan ) { contextualHashtagConfiguration.forEach((type) => { - // replace raw span for the hashtag with a cooked one + // Replace raw span for the hashtag with a cooked one const matchingSeenHashtag = seenHashtags[type]?.[slugRef]; if (matchingSeenHashtag) { // NOTE: When changing the HTML structure here, you must also change @@ -244,8 +263,12 @@ function _findAndReplaceSeenHashtagPlaceholder( link.classList.add("hashtag-cooked"); link.href = matchingSeenHashtag.relative_url; link.dataset.type = type; + link.dataset.id = matchingSeenHashtag.id; link.dataset.slug = matchingSeenHashtag.slug; - link.innerHTML = `${matchingSeenHashtag.text}`; + const hashtagType = new getHashtagTypeClasses()[type]; + link.innerHTML = `${hashtagType.generateIconHTML( + matchingSeenHashtag + )}${emojiUnescape(matchingSeenHashtag.text)}`; hashtagSpan.replaceWith(link); } }); 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 17be5a0a022..2d9850ef7eb 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-types/base.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-types/base.js @@ -16,4 +16,8 @@ export default class HashtagTypeBase { generateColorCssClasses() { throw "not implemented"; } + + generateIconHTML() { + throw "not implemented"; + } } 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 e167da8c11d..ffb8f3001d4 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-types/category.js @@ -12,21 +12,25 @@ export default class CategoryHashtagType extends HashtagTypeBase { return this.site.categories || []; } - generateColorCssClasses(model) { + generateColorCssClasses(category) { const generatedCssClasses = []; - const backgroundGradient = [`var(--category-${model.id}-color) 50%`]; - if (model.parentCategory) { + const backgroundGradient = [`var(--category-${category.id}-color) 50%`]; + if (category.parentCategory) { backgroundGradient.push( - `var(--category-${model.parentCategory.id}-color) 50%` + `var(--category-${category.parentCategory.id}-color) 50%` ); } else { - backgroundGradient.push(`var(--category-${model.id}-color) 50%`); + backgroundGradient.push(`var(--category-${category.id}-color) 50%`); } - generatedCssClasses.push(`.hashtag-color--category-${model.id} { + generatedCssClasses.push(`.hashtag-color--category-${category.id} { background: linear-gradient(90deg, ${backgroundGradient.join(", ")}); }`); return generatedCssClasses; } + + generateIconHTML(hashtag) { + return ``; + } } diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-types/tag.js b/app/assets/javascripts/discourse/app/lib/hashtag-types/tag.js index 9161f864adf..42cd657f210 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-types/tag.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-types/tag.js @@ -1,4 +1,5 @@ import HashtagTypeBase from "./base"; +import { iconHTML } from "discourse-common/lib/icon-library"; export default class TagHashtagType extends HashtagTypeBase { get type() { @@ -12,4 +13,10 @@ export default class TagHashtagType extends HashtagTypeBase { generateColorCssClasses() { return []; } + + generateIconHTML(hashtag) { + return iconHTML(hashtag.icon, { + class: `hashtag-color--${this.type}-${hashtag.id}`, + }); + } } diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 63f47195629..4da8c699a14 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -2226,10 +2226,11 @@ class PluginApi { * This is used when generating CSS classes in the hashtag-css-generator. * * @param {string} type - The type of the hashtag. - * @param {Class} typeClass - The class of the hashtag type. + * @param {Class} typeClassInstance - The initialized class of the hashtag type, which + * needs the `container`. */ - registerHashtagType(type, typeClass) { - registerHashtagType(type, typeClass); + registerHashtagType(type, typeClassInstance) { + registerHashtagType(type, typeClassInstance); } } diff --git a/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js index d7706f45802..c7c05895436 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js @@ -699,7 +699,7 @@ export default { ], displayed_about_plugin_stat_groups: ["chat_messages"], hashtag_configurations: { "topic-composer": ["category", "tag"] }, - hashtag_icons: ["folder", "tag"], + hashtag_icons: { "category": "folder", "tag": "tag" }, anonymous_sidebar_sections: [ { id: 111, diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js index 65e2fca64bb..42a14b3d2d2 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js @@ -29,6 +29,7 @@ function addHashtag(buffer, matches, state) { ["href", result.relative_url], ["data-type", result.type], ["data-slug", result.slug], + ["data-id", result.id], ]; // Most cases these will be the exact same, one standout is categories @@ -40,20 +41,7 @@ function addHashtag(buffer, matches, state) { token.block = false; buffer.push(token); - token = new state.Token("svg_open", "svg", 1); - token.block = false; - token.attrs = [ - ["class", `fa d-icon d-icon-${result.icon} svg-icon svg-node`], - ]; - buffer.push(token); - - token = new state.Token("use_open", "use", 1); - token.block = false; - token.attrs = [["href", `#${result.icon}`]]; - buffer.push(token); - - buffer.push(new state.Token("use_close", "use", -1)); - buffer.push(new state.Token("svg_close", "svg", -1)); + addIconPlaceholder(buffer, state); token = new state.Token("span_open", "span", 1); token.block = false; @@ -82,24 +70,22 @@ function addHashtag(buffer, matches, state) { } } +// The svg icon is not baked into the HTML because we want +// to be able to use icon replacement via renderIcon, and +// because different hashtag types may render icons/CSS +// classes differently. +// +// Instead, the UI will dynamically replace these where hashtags +// are rendered, like within posts, using decorateCooked* APIs. +function addIconPlaceholder(buffer, state) { + const token = new state.Token("span_open", "span", 1); + token.block = false; + token.attrs = [["class", "hashtag-icon-placeholder"]]; + buffer.push(token); + buffer.push(new state.Token("span_close", "span", -1)); +} + export function setup(helper) { - const opts = helper.getOptions(); - - // we do this because plugins can register their own hashtag data - // sources which specify an icon, and each icon must be allowlisted - // or it will not render in the markdown pipeline - const hashtagIconAllowList = opts.hashtagIcons - ? opts.hashtagIcons - .concat(["hashtag"]) - .map((icon) => { - return [ - `svg[class=fa d-icon d-icon-${icon} svg-icon svg-node]`, - `use[href=#${icon}]`, - ]; - }) - .flat() - : []; - helper.registerPlugin((md) => { if ( md.options.discourse.limitedSiteSettings @@ -114,13 +100,13 @@ export function setup(helper) { } }); - helper.allowList( - hashtagIconAllowList.concat([ - "a.hashtag-cooked", - "span.hashtag-raw", - "a[data-type]", - "a[data-slug]", - "a[data-ref]", - ]) - ); + helper.allowList([ + "a.hashtag-cooked", + "span.hashtag-raw", + "span.hashtag-icon-placeholder", + "a[data-type]", + "a[data-slug]", + "a[data-ref]", + "a[data-id]", + ]); } diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/watched-words.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/watched-words.js index 6b5228cb2c0..ac5ef7b7b40 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/watched-words.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/watched-words.js @@ -43,6 +43,7 @@ const NONE = 0; const MENTION = 1; const HASHTAG_LINK = 2; const HASHTAG_SPAN = 3; +const HASHTAG_ICON_SPAN = 4; export function setup(helper) { const opts = helper.getOptions(); @@ -97,6 +98,7 @@ export function setup(helper) { // We scan once to mark tokens that must be skipped because they are // mentions or hashtags let lastType = NONE; + let currentType = NONE; for (let i = 0; i < tokens.length; ++i) { const currentToken = tokens[i]; @@ -109,14 +111,26 @@ export function setup(helper) { currentToken.attrs.some( (attr) => attr[0] === "class" && - (attr[1].includes("hashtag") || - attr[1].includes("hashtag-cooked")) + (attr[1] === "hashtag" || + attr[1] === "hashtag-cooked" || + attr[1] === "hashtag-raw") ) ) { lastType = currentToken.type === "link_open" ? HASHTAG_LINK : HASHTAG_SPAN; } + if ( + currentToken.type === "span_open" && + currentToken.attrs && + currentToken.attrs.some( + (attr) => + attr[0] === "class" && attr[1] === "hashtag-icon-placeholder" + ) + ) { + currentType = HASHTAG_ICON_SPAN; + } + if (lastType !== NONE) { currentToken.skipReplace = true; } @@ -124,7 +138,9 @@ export function setup(helper) { if ( (lastType === MENTION && currentToken.type === "mention_close") || (lastType === HASHTAG_LINK && currentToken.type === "link_close") || - (lastType === HASHTAG_SPAN && currentToken.type === "span_close") + (lastType === HASHTAG_SPAN && + currentToken.type === "span_close" && + currentType !== HASHTAG_ICON_SPAN) ) { lastType = NONE; } diff --git a/app/assets/stylesheets/common/components/hashtag.scss b/app/assets/stylesheets/common/components/hashtag.scss index caa7aed2d6b..5d9e1a3c581 100644 --- a/app/assets/stylesheets/common/components/hashtag.scss +++ b/app/assets/stylesheets/common/components/hashtag.scss @@ -37,6 +37,14 @@ a.hashtag-cooked { svg { display: inline; } + + .hashtag-category-badge { + flex: 0 0 auto; + width: 9px; + height: 9px; + margin-right: 5px; + display: inline-block; + } } .hashtag-autocomplete { diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 140d7657c71..941b7f17690 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -241,7 +241,7 @@ class SiteSerializer < ApplicationSerializer end def hashtag_icons - HashtagAutocompleteService.data_source_icons + HashtagAutocompleteService.data_source_icon_map end def displayed_about_plugin_stat_groups diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb index 85403cf5b16..0e47ec76d5e 100644 --- a/app/services/category_hashtag_data_source.rb +++ b/app/services/category_hashtag_data_source.rb @@ -19,6 +19,7 @@ class CategoryHashtagDataSource item.description = category.description_text item.icon = icon item.relative_url = category.url + item.id = category.id # Single-level category heirarchy should be enough to distinguish between # categories here. diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index 6e92b2ee272..be60056adda 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -34,8 +34,8 @@ class HashtagAutocompleteService data_sources.map(&:type) end - def self.data_source_icons - data_sources.map(&:icon) + def self.data_source_icon_map + data_sources.map { |ds| [ds.type, ds.icon] }.to_h end def self.data_source_from_type(type) @@ -88,6 +88,10 @@ class HashtagAutocompleteService # item, used for the cooked hashtags, e.g. /c/2/staff attr_accessor :relative_url + # The ID of the resource that is represented by the autocomplete item, + # e.g. category.id, tag.id + attr_accessor :id + def initialize(params = {}) @relative_url = params[:relative_url] @text = params[:text] @@ -96,6 +100,7 @@ class HashtagAutocompleteService @type = params[:type] @ref = params[:ref] @slug = params[:slug] + @id = params[:id] end def to_h @@ -107,6 +112,7 @@ class HashtagAutocompleteService type: self.type, ref: self.ref, slug: self.slug, + id: self.id, } end end diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb index b186ca12c42..7ec1b20ba7e 100644 --- a/app/services/tag_hashtag_data_source.rb +++ b/app/services/tag_hashtag_data_source.rb @@ -27,6 +27,7 @@ class TagHashtagDataSource item.slug = tag.name item.relative_url = tag.url item.icon = icon + item.id = tag.id end end private_class_method :tag_to_hashtag_item @@ -66,7 +67,11 @@ class TagHashtagDataSource TagsController .tag_counts_json(tags_with_counts, guardian) .take(limit) - .map { |tag| tag_to_hashtag_item(tag, guardian) } + .map do |tag| + # We want the actual ID here not the `name` as tag_counts_json gives us. + tag[:id] = tags_with_counts.find { |t| t.name == tag[:name] }.id + tag_to_hashtag_item(tag, guardian) + end end def self.search_sort(search_results, _) diff --git a/lib/excerpt_parser.rb b/lib/excerpt_parser.rb index 0e23f29758e..5125135fccf 100644 --- a/lib/excerpt_parser.rb +++ b/lib/excerpt_parser.rb @@ -22,6 +22,7 @@ class ExcerptParser < Nokogiri::XML::SAX::Document @keep_svg = options[:keep_svg] == true @remap_emoji = options[:remap_emoji] == true @start_excerpt = false + @start_hashtag_icon = false @in_details_depth = 0 @summary_contents = +"" @detail_contents = +"" @@ -112,10 +113,14 @@ class ExcerptParser < Nokogiri::XML::SAX::Document when "header" @in_quote = !@keep_onebox_source if attributes.include?(%w[class source]) when "div", "span" - if attributes.include?(%w[class excerpt]) + attributes = Hash[*attributes.flatten] + if attributes["class"]&.include?("excerpt") @excerpt = +"" @current_length = 0 @start_excerpt = true + elsif attributes["class"]&.include?("hashtag-icon-placeholder") + @start_hashtag_icon = true + include_tag(name, attributes) end when "details" @detail_contents = +"" if @in_details_depth == 0 @@ -180,6 +185,7 @@ class ExcerptParser < Nokogiri::XML::SAX::Document @in_summary = false if @in_details_depth == 1 when "div", "span" throw :done if @start_excerpt + characters("", truncate: false, count_it: false, encode: false) if @start_hashtag_icon when "svg" characters("", truncate: false, count_it: false, encode: false) if @keep_svg @in_svg = false diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 871a9f431cc..91d9b304791 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -224,10 +224,8 @@ module PrettyText .ordered_types_for_context(opts[:hashtag_context]) .map { |t| "'#{t}'" } .join(",") - hashtag_icons_as_js = - HashtagAutocompleteService.data_source_icons.map { |i| "'#{i}'" }.join(",") buffer << "__optInput.hashtagTypesInPriorityOrder = [#{hashtag_types_as_js}];\n" - buffer << "__optInput.hashtagIcons = [#{hashtag_icons_as_js}];\n" + buffer << "__optInput.hashtagIcons = #{HashtagAutocompleteService.data_source_icon_map.to_json};\n" buffer << "__textOptions = __buildOptions(__optInput);\n" buffer << ("__pt = new __PrettyText(__textOptions);") diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js index 360820ac785..88d4137a410 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-decorators.js @@ -1,4 +1,5 @@ import { decorateGithubOneboxBody } from "discourse/initializers/onebox-decorators"; +import { replaceHashtagIconPlaceholder } from "discourse/lib/hashtag-autocomplete"; import { withPluginApi } from "discourse/lib/plugin-api"; import highlightSyntax from "discourse/lib/highlight-syntax"; import I18n from "I18n"; @@ -12,6 +13,7 @@ export default { initializeWithPluginApi(api, container) { const siteSettings = container.lookup("service:site-settings"); + const site = container.lookup("service:site"); api.decorateChatMessage((element) => decorateGithubOneboxBody(element), { id: "onebox-github-body", }); @@ -70,6 +72,11 @@ export default { id: "lightbox", } ); + + api.decorateChatMessage( + (element) => replaceHashtagIconPlaceholder(element, site), + { id: "hashtagIcons" } + ); }, _getScrollParent(node, maxParentSelector) { diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js index 5acabfedb7c..7189d72016f 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js @@ -28,7 +28,7 @@ export default { } withPluginApi("0.12.1", (api) => { - api.registerHashtagType("channel", ChannelHashtagType); + api.registerHashtagType("channel", new ChannelHashtagType(container)); api.registerChatComposerButton({ id: "chat-upload-btn", diff --git a/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js b/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js index ba679704b96..2a879b199af 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js +++ b/plugins/chat/assets/javascripts/discourse/lib/hashtag-types/channel.js @@ -1,4 +1,5 @@ import HashtagTypeBase from "discourse/lib/hashtag-types/base"; +import { iconHTML } from "discourse-common/lib/icon-library"; import { inject as service } from "@ember/service"; export default class ChannelHashtagType extends HashtagTypeBase { @@ -17,9 +18,15 @@ export default class ChannelHashtagType extends HashtagTypeBase { } } - generateColorCssClasses(model) { + generateColorCssClasses(channel) { return [ - `.hashtag-color--${this.type}-${model.id} { color: var(--category-${model.chatable.id}-color); }`, + `.hashtag-cooked .d-icon.hashtag-color--${this.type}-${channel.id} { color: var(--category-${channel.chatable.id}-color); }`, ]; } + + generateIconHTML(hashtag) { + return iconHTML(hashtag.icon, { + class: `hashtag-color--${this.type}-${hashtag.id}`, + }); + } } diff --git a/plugins/chat/lib/chat/channel_hashtag_data_source.rb b/plugins/chat/lib/chat/channel_hashtag_data_source.rb index f97b1e7334c..e31e96d0b22 100644 --- a/plugins/chat/lib/chat/channel_hashtag_data_source.rb +++ b/plugins/chat/lib/chat/channel_hashtag_data_source.rb @@ -18,6 +18,7 @@ module Chat item.icon = icon item.relative_url = channel.relative_url item.type = "channel" + item.id = channel.id end end diff --git a/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb b/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb index 44a6d4c4db4..3657f458735 100644 --- a/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb +++ b/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb @@ -195,7 +195,7 @@ describe Chat::ChannelArchiveService do expect(@channel_archive.reload.complete?).to eq(true) pm_topic = Topic.private_messages.last expect(pm_topic.first_post.cooked).to include( - "#{channel.title(user)}", + "#{channel.title(user)}", ) end end diff --git a/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb b/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb index 0a93c2b5e0a..4aff20fb3f4 100644 --- a/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb +++ b/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb @@ -41,6 +41,7 @@ RSpec.describe Chat::ChannelHashtagDataSource do text: "Zany Things", description: "Just weird stuff", icon: "comment", + id: channel1.id, type: "channel", ref: nil, slug: "random", @@ -60,6 +61,7 @@ RSpec.describe Chat::ChannelHashtagDataSource do text: "Secret Stuff", description: nil, icon: "comment", + id: channel2.id, type: "channel", ref: nil, slug: "secret", @@ -94,6 +96,7 @@ RSpec.describe Chat::ChannelHashtagDataSource do text: "Zany Things", description: "Just weird stuff", icon: "comment", + id: channel1.id, type: "channel", ref: nil, slug: "random", @@ -109,6 +112,7 @@ RSpec.describe Chat::ChannelHashtagDataSource do text: "Zany Things", description: "Just weird stuff", icon: "comment", + id: channel1.id, type: "channel", ref: nil, slug: "random", @@ -127,6 +131,7 @@ RSpec.describe Chat::ChannelHashtagDataSource do text: "Secret Stuff", description: nil, icon: "comment", + id: channel2.id, type: "channel", ref: nil, slug: "secret", diff --git a/plugins/chat/spec/models/chat/message_spec.rb b/plugins/chat/spec/models/chat/message_spec.rb index 53e4ae48270..0af043f6b9d 100644 --- a/plugins/chat/spec/models/chat/message_spec.rb +++ b/plugins/chat/spec/models/chat/message_spec.rb @@ -259,7 +259,7 @@ describe Chat::Message do cooked = described_class.cook("##{category.slug}", user_id: user.id) expect(cooked).to eq( - "

#{category.name}

", + "

#{category.name}

", ) end diff --git a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb index b847cfb4d72..f3dfa9ba18a 100644 --- a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb +++ b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb @@ -71,13 +71,13 @@ describe "Using #hashtag autocompletion to search for and lookup channels", cooked_hashtags = page.all(".hashtag-cooked", count: 3) expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp) - Random + Random HTML expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp) - Raspberry + Raspberry HTML expect(cooked_hashtags[2]["outerHTML"]).to eq(<<~HTML.chomp) - razed + razed HTML end end diff --git a/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js b/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js index cc39cd47a22..a2aeeee9c87 100644 --- a/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js +++ b/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js @@ -59,7 +59,7 @@ acceptance("Chat | Hashtag CSS Generator", function (needs) { assert.equal( cssTag.innerHTML, - ".hashtag-color--category-1 {\n background: linear-gradient(90deg, var(--category-1-color) 50%, var(--category-1-color) 50%);\n}\n.hashtag-color--category-2 {\n background: linear-gradient(90deg, var(--category-2-color) 50%, var(--category-2-color) 50%);\n}\n.hashtag-color--category-4 {\n background: linear-gradient(90deg, var(--category-4-color) 50%, var(--category-1-color) 50%);\n}\n.hashtag-color--channel-44 { color: var(--category-1-color); }\n.hashtag-color--channel-74 { color: var(--category-2-color); }\n.hashtag-color--channel-88 { color: var(--category-4-color); }" + ".hashtag-color--category-1 {\n background: linear-gradient(90deg, var(--category-1-color) 50%, var(--category-1-color) 50%);\n}\n.hashtag-color--category-2 {\n background: linear-gradient(90deg, var(--category-2-color) 50%, var(--category-2-color) 50%);\n}\n.hashtag-color--category-4 {\n background: linear-gradient(90deg, var(--category-4-color) 50%, var(--category-1-color) 50%);\n}\n.hashtag-cooked .d-icon.hashtag-color--channel-44 { color: var(--category-1-color); }\n.hashtag-cooked .d-icon.hashtag-color--channel-74 { color: var(--category-2-color); }\n.hashtag-cooked .d-icon.hashtag-color--channel-88 { color: var(--category-4-color); }" ); }); }); diff --git a/spec/lib/email/sender_spec.rb b/spec/lib/email/sender_spec.rb index a3619e28c25..2194b379ad0 100644 --- a/spec/lib/email/sender_spec.rb +++ b/spec/lib/email/sender_spec.rb @@ -572,7 +572,7 @@ RSpec.describe Email::Sender do reply.rebake! Email::Sender.new(message, :valid_type).send expected = <<~HTML - #dev + #dev HTML expect(message.html_part.body.to_s).to include(expected.chomp) end diff --git a/spec/lib/oneboxer_spec.rb b/spec/lib/oneboxer_spec.rb index 7e23517425a..ed7cf50c2e9 100644 --- a/spec/lib/oneboxer_spec.rb +++ b/spec/lib/oneboxer_spec.rb @@ -178,13 +178,20 @@ RSpec.describe Oneboxer do expect(preview("/u/#{user.username}")).to include("Thunderland") end - it "includes hashtag HTML and icons" do + it "includes hashtag HTML" do SiteSetting.enable_experimental_hashtag_autocomplete = true category = Fabricate(:category, slug: "random") - Fabricate(:tag, name: "bug") + tag = Fabricate(:tag, name: "bug") public_post = Fabricate(:post, raw: "This post has some hashtags, #random and #bug") - expect(preview(public_post.url).chomp).to include(<<~HTML.chomp) - #{category.name} and bug + preview = + Nokogiri::HTML5 + .fragment(preview(public_post.url).chomp) + .css("blockquote") + .inner_html + .chomp + .strip + expect(preview).to eq(<<~HTML.chomp.strip) + This post has some hashtags, #{category.name} and #{tag.name} HTML end end diff --git a/spec/lib/pretty_text/helpers_spec.rb b/spec/lib/pretty_text/helpers_spec.rb index c8612c9cc4b..ff14ee704e0 100644 --- a/spec/lib/pretty_text/helpers_spec.rb +++ b/spec/lib/pretty_text/helpers_spec.rb @@ -68,6 +68,7 @@ RSpec.describe PrettyText::Helpers do text: "somecooltag", description: "Coolest things ever", icon: "tag", + id: tag.id, slug: "somecooltag", ref: "somecooltag::tag", type: "tag", @@ -85,6 +86,7 @@ RSpec.describe PrettyText::Helpers do text: "Some Awesome Category", description: "Really great stuff here", icon: "folder", + id: category.id, slug: "someawesomecategory", ref: "someawesomecategory::category", type: "category", @@ -101,6 +103,7 @@ RSpec.describe PrettyText::Helpers do text: "Some Awesome Category", description: "Really great stuff here", icon: "folder", + id: category.id, slug: "someawesomecategory", ref: "someawesomecategory", type: "category", @@ -115,6 +118,7 @@ RSpec.describe PrettyText::Helpers do text: "somecooltag", description: "Coolest things ever", icon: "tag", + id: tag.id, slug: "somecooltag", ref: "somecooltag", type: "tag", @@ -128,6 +132,7 @@ RSpec.describe PrettyText::Helpers do text: "Some Awesome Category", description: "Really great stuff here", icon: "folder", + id: category.id, slug: "someawesomecategory", ref: "someawesomecategory", type: "category", @@ -150,6 +155,7 @@ RSpec.describe PrettyText::Helpers do text: "Manager Hideout", description: nil, icon: "folder", + id: private_category.id, slug: "secretcategory", ref: "secretcategory", type: "category", diff --git a/spec/lib/pretty_text_spec.rb b/spec/lib/pretty_text_spec.rb index 30fb439ffa7..d64cd9a2a11 100644 --- a/spec/lib/pretty_text_spec.rb +++ b/spec/lib/pretty_text_spec.rb @@ -1715,19 +1715,20 @@ RSpec.describe PrettyText do category2 = Fabricate(:category, name: "known", slug: "known") group = Fabricate(:group) private_category = Fabricate(:private_category, name: "secret", group: group, slug: "secret") - Fabricate(:topic, tags: [Fabricate(:tag, name: "known")]) + tag = Fabricate(:tag, name: "known") + Fabricate(:topic, tags: [tag]) cooked = PrettyText.cook(" #unknown::tag #known #known::tag #testing #secret", user_id: user.id) expect(cooked).to include("#unknown::tag") expect(cooked).to include( - "known", + "known", ) expect(cooked).to include( - "known", + "known", ) expect(cooked).to include( - "testing", + "testing", ) expect(cooked).to include("#secret") @@ -1735,7 +1736,7 @@ RSpec.describe PrettyText do group.add(user) cooked = PrettyText.cook(" #unknown::tag #known #known::tag #testing #secret", user_id: user.id) expect(cooked).to include( - "secret", + "secret", ) cooked = PrettyText.cook("[`a` #known::tag here](http://example.com)", user_id: user.id) @@ -1753,7 +1754,7 @@ RSpec.describe PrettyText do cooked = PrettyText.cook("test #known::tag", user_id: user.id) html = <<~HTML -

test known

+

test known

HTML expect(cooked).to eq(html.strip) @@ -2006,7 +2007,7 @@ HTML expect(PrettyText.cook("@test #test test")).to match_html(<<~HTML)

@test - test + test tdiscourset

HTML diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index 516b2bc2722..99cabace440 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -524,7 +524,7 @@ "type": "object" }, "hashtag_icons" : { - "type": "array" + "type": "object" }, "displayed_about_plugin_stat_groups" : { "type": "array" diff --git a/spec/requests/hashtags_controller_spec.rb b/spec/requests/hashtags_controller_spec.rb index 968465bd0a0..98b37199a10 100644 --- a/spec/requests/hashtags_controller_spec.rb +++ b/spec/requests/hashtags_controller_spec.rb @@ -163,6 +163,7 @@ RSpec.describe HashtagsController do "type" => "category", "ref" => category.slug, "slug" => category.slug, + "id" => category.id, }, ], "tag" => [ @@ -175,6 +176,7 @@ RSpec.describe HashtagsController do "ref" => tag.name, "slug" => tag.name, "secondary_text" => "x0", + "id" => tag.id, }, ], }, @@ -198,6 +200,7 @@ RSpec.describe HashtagsController do "ref" => "#{tag.name}::tag", "slug" => tag.name, "secondary_text" => "x0", + "id" => tag.id, }, ], }, @@ -242,6 +245,7 @@ RSpec.describe HashtagsController do "type" => "category", "ref" => private_category.slug, "slug" => private_category.slug, + "id" => private_category.id, }, ], "tag" => [ @@ -254,6 +258,7 @@ RSpec.describe HashtagsController do "ref" => hidden_tag.name, "slug" => hidden_tag.name, "secondary_text" => "x0", + "id" => hidden_tag.id, }, ], }, @@ -337,6 +342,7 @@ RSpec.describe HashtagsController do "type" => "category", "ref" => category.slug, "slug" => category.slug, + "id" => category.id, }, { "relative_url" => tag_2.url, @@ -347,6 +353,7 @@ RSpec.describe HashtagsController do "ref" => "#{tag_2.name}::tag", "slug" => tag_2.name, "secondary_text" => "x0", + "id" => tag_2.id, }, ], ) @@ -379,6 +386,7 @@ RSpec.describe HashtagsController do "type" => "category", "ref" => private_category.slug, "slug" => private_category.slug, + "id" => private_category.id, }, ], ) @@ -396,6 +404,7 @@ RSpec.describe HashtagsController do "ref" => "#{hidden_tag.name}", "slug" => hidden_tag.name, "secondary_text" => "x0", + "id" => hidden_tag.id, }, ], ) diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index 9c2382816ba..fce479967df 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -32,9 +32,11 @@ RSpec.describe HashtagAutocompleteService do end end - describe ".data_source_icons" do + describe ".data_source_icon_map" do it "gets an array for all icons defined by data sources so they can be used for markdown allowlisting" do - expect(HashtagAutocompleteService.data_source_icons).to eq(%w[folder tag]) + expect(HashtagAutocompleteService.data_source_icon_map).to eq( + { "category" => "folder", "tag" => "tag" }, + ) end end diff --git a/spec/system/hashtag_autocomplete_spec.rb b/spec/system/hashtag_autocomplete_spec.rb index 254f6a686c7..1c8c0bc79f2 100644 --- a/spec/system/hashtag_autocomplete_spec.rb +++ b/spec/system/hashtag_autocomplete_spec.rb @@ -61,7 +61,7 @@ describe "Using #hashtag autocompletion to search for and lookup categories and expect(page).to have_css(".hashtag-cooked") cooked_hashtag = page.find(".hashtag-cooked") expected = <<~HTML.chomp - Cool Category + Cool Category HTML expect(cooked_hashtag["outerHTML"].squish).to eq(expected) @@ -71,7 +71,7 @@ describe "Using #hashtag autocompletion to search for and lookup categories and expect(page).to have_css(".hashtag-cooked") cooked_hashtag = page.find(".hashtag-cooked") expect(cooked_hashtag["outerHTML"].squish).to eq(<<~HTML.chomp) - cooltag + cooltag HTML end @@ -84,10 +84,10 @@ describe "Using #hashtag autocompletion to search for and lookup categories and cooked_hashtags = page.all(".hashtag-cooked", count: 2) expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp) - Cool Category + Cool Category HTML expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp) - cooltag + cooltag HTML end end