From 9c0e50e1cc0b493d8405d3266e491b29b8efe0b7 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Wed, 23 Aug 2023 15:39:58 +0400 Subject: [PATCH] FIX: hide tooltips when scrolling on mobile (#23098) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes the problem reported in https://meta.discourse.org/t/custom-status-message-in-front-of-by-header-on-scroll/273320. This problem can be reproduced with any tooltip created using the DTooltip component or the createDTooltip function. The problem happens because the trigger for tooltip on mobile is click, and for tooltip to disappear the user has to click outside the tooltip. This is the default behavior of tippy.js – the library we use under the hood. Note that this PR fixes the problem in topics, but not in chat. I'm going to investigate and address it in chat in a following PR. To fix it for tooltips created with the createDTooltip function, I had to make a refactoring. We had a somewhat not ideal solution there, we were leaking an implementation detail by passing tippy instances to calling sides, so they could then destroy them. With this fix, I would have to make it more complex, because now we need to also remove onScrool handlers, and I would need to leak this implementation detail too. So, I firstly refactored the current solution in 5a4af05 and then added onScroll handlers in fb4aabe. When refactoring this, I was running locally some temporarily skipped flaky tests. Turned out they got a bit outdated, so I fixed them. Note that I'm not unskipping them in this commit, we'll address them separately later. --- .../app/components/composer-editor.js | 6 +- .../discourse/app/components/d-tooltip.gjs | 17 +++ .../discourse/app/lib/d-tooltip.js | 62 +++++++--- .../app/lib/update-user-status-on-mention.js | 18 ++- .../discourse/app/lib/user-status-message.js | 115 ++++++++++-------- .../app/lib/user-status-on-autocomplete.js | 16 +-- .../discourse/app/widgets/post-cooked.js | 18 ++- .../discourse/components/chat-composer.js | 6 +- .../user-status-on-mentions-test.js | 19 +-- 9 files changed, 168 insertions(+), 109 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index b74a0c5ec0d..fefde08feb3 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -39,7 +39,7 @@ import { loadOneboxes } from "discourse/lib/load-oneboxes"; import putCursorAtEnd from "discourse/lib/put-cursor-at-end"; import userSearch from "discourse/lib/user-search"; import { - destroyTippyInstances, + destroyUserStatuses, initUserStatusHtml, renderUserStatusHtml, } from "discourse/lib/user-status-on-autocomplete"; @@ -222,7 +222,7 @@ export default Component.extend( $input.autocomplete({ template: findRawTemplate("user-selector-autocomplete"), dataSource: (term) => { - destroyTippyInstances(); + destroyUserStatuses(); return userSearch({ term, topicId: this.topic?.id, @@ -241,7 +241,7 @@ export default Component.extend( afterComplete: this._afterMentionComplete, triggerRule: (textarea) => !inCodeBlock(textarea.value, caretPosition(textarea)), - onClose: destroyTippyInstances, + onClose: destroyUserStatuses, }); } diff --git a/app/assets/javascripts/discourse/app/components/d-tooltip.gjs b/app/assets/javascripts/discourse/app/components/d-tooltip.gjs index 1f0a9719f59..9eae5e19f7c 100644 --- a/app/assets/javascripts/discourse/app/components/d-tooltip.gjs +++ b/app/assets/javascripts/discourse/app/components/d-tooltip.gjs @@ -3,6 +3,8 @@ import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import { inject as service } from "@ember/service"; import { action } from "@ember/object"; import { iconHTML } from "discourse-common/lib/icon-library"; +import discourseDebounce from "discourse-common/lib/debounce"; +import { bind } from "discourse-common/utils/decorators"; import tippy from "tippy.js"; export default class DiscourseTooltip extends Component { @@ -15,11 +17,26 @@ export default class DiscourseTooltip extends Component { #tippyInstance; + constructor() { + super(...arguments); + if (this.capabilities.touch) { + window.addEventListener("scroll", this.onScroll); + } + } + willDestroy() { super.willDestroy(...arguments); + if (this.capabilities.touch) { + window.removeEventListener("scroll", this.onScroll); + } this.#tippyInstance.destroy(); } + @bind + onScroll() { + discourseDebounce(() => this.#tippyInstance.hide(), 10); + } + stopPropagation(instance, event) { event.preventDefault(); event.stopPropagation(); diff --git a/app/assets/javascripts/discourse/app/lib/d-tooltip.js b/app/assets/javascripts/discourse/app/lib/d-tooltip.js index b217b54a44a..5ef2a65022b 100644 --- a/app/assets/javascripts/discourse/app/lib/d-tooltip.js +++ b/app/assets/javascripts/discourse/app/lib/d-tooltip.js @@ -1,22 +1,48 @@ import tippy from "tippy.js"; +import { bind } from "discourse-common/utils/decorators"; +import discourseDebounce from "discourse-common/lib/debounce"; -function stopPropagation(instance, event) { - event.preventDefault(); - event.stopPropagation(); -} -function hasTouchCapabilities() { - return navigator.maxTouchPoints > 1 || "ontouchstart" in window; -} +export class DTooltip { + #tippyInstance; -export default function createDTooltip(target, content) { - return tippy(target, { - interactive: false, - content, - trigger: hasTouchCapabilities() ? "click" : "mouseenter", - theme: "d-tooltip", - arrow: false, - placement: "bottom-start", - onTrigger: stopPropagation, - onUntrigger: stopPropagation, - }); + constructor(target, content) { + this.#tippyInstance = this.#initTippy(target, content); + if (this.#hasTouchCapabilities()) { + window.addEventListener("scroll", this.onScroll); + } + } + + destroy() { + if (this.#hasTouchCapabilities()) { + window.removeEventListener("scroll", this.onScroll); + } + this.#tippyInstance.destroy(); + } + + @bind + onScroll() { + discourseDebounce(() => this.#tippyInstance.hide(), 10); + } + + #initTippy(target, content) { + return tippy(target, { + interactive: false, + content, + trigger: this.#hasTouchCapabilities() ? "click" : "mouseenter", + theme: "d-tooltip", + arrow: false, + placement: "bottom-start", + onTrigger: this.#stopPropagation, + onUntrigger: this.#stopPropagation, + }); + } + + #hasTouchCapabilities() { + return navigator.maxTouchPoints > 1 || "ontouchstart" in window; + } + + #stopPropagation(instance, event) { + event.preventDefault(); + event.stopPropagation(); + } } diff --git a/app/assets/javascripts/discourse/app/lib/update-user-status-on-mention.js b/app/assets/javascripts/discourse/app/lib/update-user-status-on-mention.js index eb95e52d34a..a52afb06fb3 100644 --- a/app/assets/javascripts/discourse/app/lib/update-user-status-on-mention.js +++ b/app/assets/javascripts/discourse/app/lib/update-user-status-on-mention.js @@ -1,14 +1,22 @@ -import createUserStatusMessage from "discourse/lib/user-status-message"; +import { UserStatusMessage } from "discourse/lib/user-status-message"; -export function updateUserStatusOnMention(mention, status, tippyInstances) { +let userStatusMessages = []; + +export function updateUserStatusOnMention(mention, status) { removeStatus(mention); if (status) { - const statusHtml = createUserStatusMessage(status, { showTooltip: true }); - tippyInstances.push(statusHtml._tippy); - mention.appendChild(statusHtml); + const userStatusMessage = new UserStatusMessage(status); + userStatusMessages.push(userStatusMessage); + mention.appendChild(userStatusMessage.html); } } +export function destroyUserStatusOnMentions() { + userStatusMessages.forEach((instance) => { + instance.destroy(); + }); +} + function removeStatus(mention) { mention.querySelector("span.user-status-message")?.remove(); } diff --git a/app/assets/javascripts/discourse/app/lib/user-status-message.js b/app/assets/javascripts/discourse/app/lib/user-status-message.js index 7159b4b2c68..2148c036a39 100644 --- a/app/assets/javascripts/discourse/app/lib/user-status-message.js +++ b/app/assets/javascripts/discourse/app/lib/user-status-message.js @@ -1,62 +1,73 @@ -import createDTooltip from "discourse/lib/d-tooltip"; +import { DTooltip } from "discourse/lib/d-tooltip"; import { emojiUnescape } from "discourse/lib/text"; import { escapeExpression } from "discourse/lib/utilities"; import { until } from "discourse/lib/formatter"; import User from "discourse/models/user"; -function getUntil(endsAt) { - const currentUser = User.current(); +export class UserStatusMessage { + #dTooltip; - const timezone = currentUser - ? currentUser.user_option?.timezone - : moment.tz.guess(); - - return until(endsAt, timezone, currentUser?.locale); -} - -function getEmoji(emojiName) { - const emoji = escapeExpression(`:${emojiName}:`); - return emojiUnescape(emoji, { - skipTitle: true, - }); -} - -function attachTooltip(target, status) { - const content = document.createElement("div"); - content.classList.add("user-status-message-tooltip"); - content.innerHTML = getEmoji(status.emoji); - - const tooltipDescription = document.createElement("span"); - tooltipDescription.classList.add("user-status-tooltip-description"); - tooltipDescription.innerText = status.description; - content.appendChild(tooltipDescription); - - if (status.ends_at) { - const untilElement = document.createElement("div"); - untilElement.classList.add("user-status-tooltip-until"); - untilElement.innerText = getUntil(status.ends_at); - content.appendChild(untilElement); - } - createDTooltip(target, content); -} - -export default function createUserStatusMessage(status, opts) { - const userStatusMessage = document.createElement("span"); - userStatusMessage.classList.add("user-status-message"); - if (opts?.class) { - userStatusMessage.classList.add(opts.class); - } - userStatusMessage.innerHTML = getEmoji(status.emoji); - - if (opts?.showDescription) { - const messageDescription = document.createElement("span"); - messageDescription.classList.add("user-status-message-description"); - messageDescription.innerText = status.description; - userStatusMessage.appendChild(messageDescription); + constructor(status, opts) { + this.html = this.#statusHtml(status, opts); + this.#dTooltip = new DTooltip(this.html, this.#tooltipHtml(status)); } - if (opts?.showTooltip) { - attachTooltip(userStatusMessage, status); + destroy() { + this.#dTooltip.destroy(); + } + + #emojiHtml(emojiName) { + const emoji = escapeExpression(`:${emojiName}:`); + return emojiUnescape(emoji, { + skipTitle: true, + }); + } + + #statusHtml(status, opts) { + const html = document.createElement("span"); + html.classList.add("user-status-message"); + if (opts?.class) { + html.classList.add(opts.class); + } + html.innerHTML = this.#emojiHtml(status.emoji); + + if (opts?.showDescription) { + const description = document.createElement("span"); + description.classList.add("user-status-message-description"); + description.innerText = status.description; + html.appendChild(description); + } + + return html; + } + + #tooltipHtml(status) { + const html = document.createElement("div"); + html.classList.add("user-status-message-tooltip"); + html.innerHTML = this.#emojiHtml(status.emoji); + + const description = document.createElement("span"); + description.classList.add("user-status-tooltip-description"); + description.innerText = status.description; + html.appendChild(description); + + if (status.ends_at) { + const untilElement = document.createElement("div"); + untilElement.classList.add("user-status-tooltip-until"); + untilElement.innerText = this.#until(status.ends_at); + html.appendChild(untilElement); + } + + return html; + } + + #until(endsAt) { + const currentUser = User.current(); + + const timezone = currentUser + ? currentUser.user_option?.timezone + : moment.tz.guess(); + + return until(endsAt, timezone, currentUser?.locale); } - return userStatusMessage; } diff --git a/app/assets/javascripts/discourse/app/lib/user-status-on-autocomplete.js b/app/assets/javascripts/discourse/app/lib/user-status-on-autocomplete.js index 49cdcee1770..b416aa7ab3f 100644 --- a/app/assets/javascripts/discourse/app/lib/user-status-on-autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/user-status-on-autocomplete.js @@ -1,16 +1,16 @@ -import createUserStatusMessage from "discourse/lib/user-status-message"; +import { UserStatusMessage } from "discourse/lib/user-status-message"; -let tippyInstances = []; +let userStatusMessages = []; export function initUserStatusHtml(users) { (users || []).forEach((user, index) => { if (user.status) { user.index = index; - user.statusHtml = createUserStatusMessage(user.status, { - showTooltip: true, + const userStatusMessage = new UserStatusMessage(user.status, { showDescription: true, }); - tippyInstances.push(user.statusHtml._tippy); + user.statusHtml = userStatusMessage.html; + userStatusMessages.push(userStatusMessage); } }); } @@ -28,9 +28,9 @@ export function renderUserStatusHtml(options) { }); } -export function destroyTippyInstances() { - tippyInstances.forEach((instance) => { +export function destroyUserStatuses() { + userStatusMessages.forEach((instance) => { instance.destroy(); }); - tippyInstances = []; + userStatusMessages = []; } diff --git a/app/assets/javascripts/discourse/app/widgets/post-cooked.js b/app/assets/javascripts/discourse/app/widgets/post-cooked.js index 3b2accee8c7..1bf2993d4df 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-cooked.js +++ b/app/assets/javascripts/discourse/app/widgets/post-cooked.js @@ -9,7 +9,10 @@ import { spinnerHTML } from "discourse/helpers/loading-spinner"; import { escape } from "pretty-text/sanitizer"; import domFromString from "discourse-common/lib/dom-from-string"; import getURL from "discourse-common/lib/get-url"; -import { updateUserStatusOnMention } from "discourse/lib/update-user-status-on-mention"; +import { + destroyUserStatusOnMentions, + updateUserStatusOnMention, +} from "discourse/lib/update-user-status-on-mention"; let _beforeAdoptDecorators = []; let _afterAdoptDecorators = []; @@ -36,7 +39,6 @@ function createDetachedElement(nodeName) { export default class PostCooked { originalQuoteContents = null; - tippyInstances = []; constructor(attrs, decoratorHelper, currentUser) { this.attrs = attrs; @@ -77,7 +79,7 @@ export default class PostCooked { destroy() { this._stopTrackingMentionedUsersStatus(); - this._destroyTippyInstances(); + destroyUserStatusOnMentions(); } _decorateAndAdopt(cooked) { @@ -382,14 +384,8 @@ export default class PostCooked { } } - _destroyTippyInstances() { - this.tippyInstances.forEach((instance) => { - instance.destroy(); - }); - } - _rerenderUserStatusOnMentions() { - this._destroyTippyInstances(); + destroyUserStatusOnMentions(); this._post()?.mentioned_users?.forEach((user) => this._rerenderUserStatusOnMention(this.cookedDiv, user) ); @@ -400,7 +396,7 @@ export default class PostCooked { const mentions = postElement.querySelectorAll(`a.mention[href="${href}"]`); mentions.forEach((mention) => { - updateUserStatusOnMention(mention, user.status, this.tippyInstances); + updateUserStatusOnMention(mention, user.status); }); } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js index 0e54b5f6dcf..be2597e6f77 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js @@ -21,7 +21,7 @@ import { Promise } from "rsvp"; import User from "discourse/models/user"; import ChatMessageInteractor from "discourse/plugins/chat/discourse/lib/chat-message-interactor"; import { - destroyTippyInstances, + destroyUserStatuses, initUserStatusHtml, renderUserStatusHtml, } from "discourse/lib/user-status-on-autocomplete"; @@ -415,7 +415,7 @@ export default class ChatComposer extends Component { return obj.username || obj.name; }, dataSource: (term) => { - destroyTippyInstances(); + destroyUserStatuses(); return userSearch({ term, includeGroups: true }).then((result) => { if (result?.users?.length > 0) { const presentUserNames = @@ -439,7 +439,7 @@ export default class ChatComposer extends Component { this.composer.focus(); this.captureMentions(); }, - onClose: destroyTippyInstances, + onClose: destroyUserStatuses, }); } diff --git a/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js b/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js index 9015b80fadf..bf7e925bb86 100644 --- a/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js +++ b/plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js @@ -46,6 +46,7 @@ acceptance("Chat | User status on mentions", function (needs) { cooked: `

Hey @${mentionedUser1.username}

`, mentioned_users: [mentionedUser1], user: actingUser, + created_at: "2020-08-04T15:00:00.000Z", }; const newStatus = { description: "working remotely", @@ -55,7 +56,7 @@ acceptance("Chat | User status on mentions", function (needs) { id: channelId, chatable_id: 1, chatable_type: "Category", - meta: { message_bus_last_ids: {} }, + meta: { message_bus_last_ids: {}, can_delete_self: true }, current_user_membership: { following: true }, chatable: { id: 1 }, }; @@ -78,6 +79,14 @@ acceptance("Chat | User status on mentions", function (needs) { pretender.put(`/chat/1/edit/${messageId}`, () => response({})); pretender.post(`/chat/drafts`, () => response({})); pretender.put(`/chat/api/channels/1/read/1`, () => response({})); + pretender.get(`/chat/api/channels/1/messages`, () => + response({ + messages: [message], + meta: { + can_load_more_future: false, + }, + }) + ); pretender.delete(`/chat/api/channels/1/messages/${messageId}`, () => response({}) ); @@ -85,14 +94,6 @@ acceptance("Chat | User status on mentions", function (needs) { response({}) ); - pretender.get(`/chat/api/channels/1`, () => - response({ - channel, - chat_messages: [message], - meta: { can_delete_self: true }, - }) - ); - pretender.get("/u/search/users", () => response({ users: [mentionedUser2, mentionedUser3],