From f5c2a4dbbdad03a4ce47feea029febe15adb5fe8 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 12 Feb 2025 15:58:30 +0000 Subject: [PATCH] DEV: Drop experimental `enable_diffhtml_preview` setting (#31306) This was intended to provide a better UX for interactive elements in the composer preview. However, the morphing strategy has irreconcilable conflicts with our `decorateCooked` API, and so we have been unable to enable this by default. Going forward, we're focussing efforts on the WYSIWYG composer to provide this kind of smooth UX, so we're dropping the `enable_diffhtml_preview` approach. --- .../discourse-markdown-it/src/setup.js | 1 - .../app/components/composer-editor.js | 18 ++++---- .../discourse/app/components/d-editor.hbs | 4 +- .../discourse/app/components/d-editor.js | 46 +------------------ .../pretty-text/addon/upload-short-url.js | 30 +++--------- config/locales/server.en.yml | 1 - config/site_settings.yml | 4 -- .../engine/allowlisted_generic_onebox.rb | 8 +--- lib/onebox/engine/audio_onebox.rb | 2 +- lib/onebox/engine/video_onebox.rb | 2 +- .../spec/system/morphed_preview_spec.rb | 30 ------------ spec/system/composer/morphed_preview_spec.rb | 41 ----------------- spec/system/composer_uploads_spec.rb | 20 -------- 13 files changed, 21 insertions(+), 186 deletions(-) delete mode 100644 plugins/discourse-details/spec/system/morphed_preview_spec.rb delete mode 100644 spec/system/composer/morphed_preview_spec.rb diff --git a/app/assets/javascripts/discourse-markdown-it/src/setup.js b/app/assets/javascripts/discourse-markdown-it/src/setup.js index 35d9ccf5a78..a26e40408b0 100644 --- a/app/assets/javascripts/discourse-markdown-it/src/setup.js +++ b/app/assets/javascripts/discourse-markdown-it/src/setup.js @@ -149,7 +149,6 @@ class Setup { discourse.limitedSiteSettings = { secureUploads: siteSettings.secure_uploads, - enableDiffhtmlPreview: siteSettings.enable_diffhtml_preview, traditionalMarkdownLinebreaks: siteSettings.traditional_markdown_linebreaks, enableMarkdownLinkify: siteSettings.enable_markdown_linkify, diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index d2887989cf3..aca1f7cd047 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -456,8 +456,8 @@ export default class ComposerEditor extends Component { $preview.scrollTop(desired + 50); } - _renderMentions(preview, unseen) { - unseen ||= linkSeenMentions(preview, this.siteSettings); + _renderMentions(preview) { + const unseen = linkSeenMentions(preview, this.siteSettings); if (unseen.length > 0) { this._renderUnseenMentions(preview, unseen); } else { @@ -480,9 +480,9 @@ export default class ComposerEditor extends Component { }); } - _renderHashtags(preview, unseen) { + _renderHashtags(preview) { const context = this.site.hashtag_configurations["topic-composer"]; - unseen ||= linkSeenHashtagsInContext(context, preview); + const unseen = linkSeenHashtagsInContext(context, preview); if (unseen.length > 0) { this._renderUnseenHashtags(preview, unseen, context); } @@ -896,15 +896,13 @@ export default class ComposerEditor extends Component { } @action - previewUpdated(preview, unseenMentions, unseenHashtags) { - this._renderMentions(preview, unseenMentions); - this._renderHashtags(preview, unseenHashtags); + previewUpdated(preview) { + this._renderMentions(preview); + this._renderHashtags(preview); this._refreshOneboxes(preview); this._expandShortUrls(preview); - if (!this.siteSettings.enable_diffhtml_preview) { - this._decorateCookedElement(preview); - } + this._decorateCookedElement(preview); this.composer.afterRefresh(preview); } diff --git a/app/assets/javascripts/discourse/app/components/d-editor.hbs b/app/assets/javascripts/discourse/app/components/d-editor.hbs index 8349bd32948..ae965622c54 100644 --- a/app/assets/javascripts/discourse/app/components/d-editor.hbs +++ b/app/assets/javascripts/discourse/app/components/d-editor.hbs @@ -75,9 +75,7 @@ class="d-editor-preview-wrapper {{if this.forcePreview 'force-preview'}}" >
- {{#unless this.siteSettings.enable_diffhtml_preview}} - {{html-safe this.preview}} - {{/unless}} + {{html-safe this.preview}}
{ if ( this._state !== "inDOM" || @@ -294,7 +250,7 @@ export default class DEditor extends Component { const previewElement = this.element.querySelector(".d-editor-preview"); if (previewElement && this.previewUpdated) { - this.previewUpdated(previewElement, unseenMentions, unseenHashtags); + this.previewUpdated(previewElement); } }); } diff --git a/app/assets/javascripts/pretty-text/addon/upload-short-url.js b/app/assets/javascripts/pretty-text/addon/upload-short-url.js index 81bd3810437..08854e4c5f7 100644 --- a/app/assets/javascripts/pretty-text/addon/upload-short-url.js +++ b/app/assets/javascripts/pretty-text/addon/upload-short-url.js @@ -159,31 +159,15 @@ function _loadCachedShortUrls(uploadElements, siteSettings, opts) { break; case "DIV": - if (siteSettings.enable_diffhtml_preview === true) { - retrieveCachedUrl(upload, siteSettings, "orig-src", opts, (url) => { - const videoHTML = ` - `; - upload.insertAdjacentHTML("beforeend", videoHTML); - upload.classList.add("video-container"); - }); - } else { - retrieveCachedUrl( - upload, - siteSettings, - "orig-src-id", - opts, - (url) => { - upload.style.backgroundImage = `url('${url}')`; + retrieveCachedUrl(upload, siteSettings, "orig-src-id", opts, (url) => { + upload.style.backgroundImage = `url('${url}')`; - const placeholderIcon = upload.querySelector( - ".placeholder-icon.video" - ); - placeholderIcon.style.backgroundColor = "rgba(0, 0, 0, 0.3)"; - } + const placeholderIcon = upload.querySelector( + ".placeholder-icon.video" ); - } + placeholderIcon.style.backgroundColor = "rgba(0, 0, 0, 0.3)"; + }); + break; } }); diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 48a6f8851d7..53c7f900a99 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2596,7 +2596,6 @@ en: disable_watched_word_checking_in_user_fields: "disable watched word checking in user fields" watched_words_regular_expressions: "Allows the use of regular expressions for filtering words. If enabled, this feature groups sensitive words by their case-sensitivity. It then compiles all selected words into a single regular expression, adding word boundaries for regular watched words. Consequently, this regex-based filtering method adds an extra layer of control over content moderation by supporting more sophisticated word patterns. The setting also allows to easily substitute the original text with the replacement of choice." - enable_diffhtml_preview: "Experimental feature which uses diffHTML to sync preview instead of full re-render" enable_fast_edit: "Adds a button to the post selection menu to edit a small selection inline." old_post_notice_days: "The number of days after which a post notice is considered old. This visually differentiates it from newer notices on the site." new_user_notice_tl: "Minimum trust level required to see new user post notices." diff --git a/config/site_settings.yml b/config/site_settings.yml index 42c1c52db72..65e11909ebf 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1310,10 +1310,6 @@ posting: watched_words_regular_expressions: client: true default: false - enable_diffhtml_preview: - hidden: true - default: false - client: true enable_fast_edit: default: true client: true diff --git a/lib/onebox/engine/allowlisted_generic_onebox.rb b/lib/onebox/engine/allowlisted_generic_onebox.rb index 235102591ae..a9f0be6d7aa 100644 --- a/lib/onebox/engine/allowlisted_generic_onebox.rb +++ b/lib/onebox/engine/allowlisted_generic_onebox.rb @@ -75,12 +75,8 @@ module Onebox def placeholder_html return article_html if (is_article? || force_article_html?) return image_html if is_image? - if !SiteSetting.enable_diffhtml_preview? && (is_video? || is_card?) - return Onebox::Helpers.video_placeholder_html - end - if !SiteSetting.enable_diffhtml_preview? && is_embedded? - return Onebox::Helpers.generic_placeholder_html - end + return Onebox::Helpers.video_placeholder_html if (is_video? || is_card?) + return Onebox::Helpers.generic_placeholder_html if is_embedded? to_html end diff --git a/lib/onebox/engine/audio_onebox.rb b/lib/onebox/engine/audio_onebox.rb index 8a41b100dd7..822923d8aec 100644 --- a/lib/onebox/engine/audio_onebox.rb +++ b/lib/onebox/engine/audio_onebox.rb @@ -23,7 +23,7 @@ module Onebox end def placeholder_html - SiteSetting.enable_diffhtml_preview ? to_html : ::Onebox::Helpers.audio_placeholder_html + ::Onebox::Helpers.audio_placeholder_html end end end diff --git a/lib/onebox/engine/video_onebox.rb b/lib/onebox/engine/video_onebox.rb index b0483bfbd40..3c810d7556c 100644 --- a/lib/onebox/engine/video_onebox.rb +++ b/lib/onebox/engine/video_onebox.rb @@ -29,7 +29,7 @@ module Onebox end def placeholder_html - SiteSetting.enable_diffhtml_preview ? to_html : ::Onebox::Helpers.video_placeholder_html + ::Onebox::Helpers.video_placeholder_html end end end diff --git a/plugins/discourse-details/spec/system/morphed_preview_spec.rb b/plugins/discourse-details/spec/system/morphed_preview_spec.rb deleted file mode 100644 index b379e4fe912..00000000000 --- a/plugins/discourse-details/spec/system/morphed_preview_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -describe "Morphed Composer Preview", type: :system do - fab!(:user) { Fabricate(:user, refresh_auto_groups: true) } - let(:composer) { PageObjects::Components::Composer.new } - - before do - SiteSetting.enable_diffhtml_preview = true - sign_in user - visit("/new-topic") - end - - it "keeps details element open" do - composer.type_content <<~MD - [details=Velcro] - What a rip-off! - [/details] - MD - - within(composer.preview) do - find("details").click - expect(page).to have_css("details[open]") - end - - composer.move_cursor_after("rip-off!") - composer.type_content(" :person_facepalming:") - - within(composer.preview) { expect(page).to have_css("details[open] img.emoji") } - end -end diff --git a/spec/system/composer/morphed_preview_spec.rb b/spec/system/composer/morphed_preview_spec.rb deleted file mode 100644 index 702da839f95..00000000000 --- a/spec/system/composer/morphed_preview_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -describe "Morphed Composer Preview", type: :system do - fab!(:user) { Fabricate(:user, username: "bob", refresh_auto_groups: true) } - let(:composer) { PageObjects::Components::Composer.new } - - before do - SiteSetting.enable_diffhtml_preview = true - sign_in user - visit("/new-topic") - end - - it "correctly morphs code blocks" do - composer.fill_content <<~MD - ```js - const = { - id: t.name, - text: t.name, - name: t.name, - ``` - MD - - within(composer.preview) { expect(find("code.lang-js")).to have_text("const = {") } - - composer.move_cursor_after("const") - composer.type_content("ant") - - within(composer.preview) { expect(find("code.lang-js")).to have_text("constant = {") } - end - - it "correctly morphs mentions" do - composer.fill_content("@bob text") - - within(composer.preview) { expect(find("a.mention")).to have_text("@bob") } - - composer.select_all - composer.type_content("@system") - - within(composer.preview) { expect(find("a.mention")).to have_text("@system") } - end -end diff --git a/spec/system/composer_uploads_spec.rb b/spec/system/composer_uploads_spec.rb index bbee973b819..d3a752566a6 100644 --- a/spec/system/composer_uploads_spec.rb +++ b/spec/system/composer_uploads_spec.rb @@ -137,26 +137,6 @@ describe "Uploading files in the composer", type: :system do expect(composer).to have_no_in_progress_uploads expect(composer.preview).to have_css(".onebox-placeholder-container") end - - it "shows video player in composer" do - SiteSetting.enable_diffhtml_preview = true - - visit "/new-topic" - expect(composer).to be_opened - topic.fill_in_composer_title("Video upload test") - - file_path_1 = file_from_fixtures("small.mp4", "media").path - attach_file(file_path_1) { composer.click_toolbar_button("upload") } - - expect(composer).to have_no_in_progress_uploads - expect(composer.preview).to have_css(".video-container video") - - expect(page).to have_css( - ".video-container video source[src]", - visible: false, - wait: Capybara.default_max_wait_time, - ) - end end context "when multiple images are uploaded" do