From d697fd5766d47c579a86dc67dac9b5b0bb33e766 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 1 May 2023 14:07:49 -0400 Subject: [PATCH] DEV: Do not rerender widgets on mouseup/mousedown (#21300) Ran into an issue with these hooks preventing click events on anchors from completing (because the triggered rerender cancels the click). See: https://github.com/discourse/discourse-header-search/pull/24 This change should have no effect on existing usage of these hooks. Current usage is limited to: - legacy navigation (should be a no-op) - reactions plugin (should be a no-op) - discourse-header-search (will fix the issue!) --- .../app/widgets/default-notification-item.js | 1 + .../javascripts/discourse/app/widgets/hooks.js | 15 +++++++++++---- .../widgets/default-notification-item-test.js | 6 +++++- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/app/widgets/default-notification-item.js b/app/assets/javascripts/discourse/app/widgets/default-notification-item.js index fa571a4e27f..7a2f3084b8c 100644 --- a/app/assets/javascripts/discourse/app/widgets/default-notification-item.js +++ b/app/assets/javascripts/discourse/app/widgets/default-notification-item.js @@ -192,6 +192,7 @@ export const DefaultNotificationItem = createWidget( method: "PUT", data: { id: this.attrs.id }, }); + this.scheduleRerender(); } }, } diff --git a/app/assets/javascripts/discourse/app/widgets/hooks.js b/app/assets/javascripts/discourse/app/widgets/hooks.js index a95de1c1817..9fc609fe3d3 100644 --- a/app/assets/javascripts/discourse/app/widgets/hooks.js +++ b/app/assets/javascripts/discourse/app/widgets/hooks.js @@ -263,13 +263,20 @@ WidgetClickHook.setupDocumentCallback = function () { }); $(document).on("mousedown.discourse-widget", (e) => { - nodeCallback(e.target, MOUSE_DOWN_ATTRIBUTE_NAME, (w) => { - w.mouseDown(e); - }); + nodeCallback( + e.target, + MOUSE_DOWN_ATTRIBUTE_NAME, + (w) => { + w.mouseDown(e); + }, + { rerender: false } + ); }); $(document).on("mouseup.discourse-widget", (e) => { - nodeCallback(e.target, MOUSE_UP_ATTRIBUTE_NAME, (w) => w.mouseUp(e)); + nodeCallback(e.target, MOUSE_UP_ATTRIBUTE_NAME, (w) => w.mouseUp(e), { + rerender: false, + }); }); $(document).on("mousemove.discourse-widget", (e) => { diff --git a/app/assets/javascripts/discourse/tests/integration/components/widgets/default-notification-item-test.js b/app/assets/javascripts/discourse/tests/integration/components/widgets/default-notification-item-test.js index 509f09bcd66..afd515e5de4 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/widgets/default-notification-item-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/widgets/default-notification-item-test.js @@ -55,7 +55,11 @@ module( assert.ok(!exists("li.read")); await triggerEvent("li", "mouseup", { button: 1, which: 2 }); - assert.strictEqual(count("li.read"), 1); + assert.strictEqual( + count("li.read"), + 1, + "only one item is marked as read" + ); assert.strictEqual(requests, 1); }); }