FIX: hide tooltips when scrolling on mobile (#23098)

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.
This commit is contained in:
Andrei Prigorshnev 2023-08-23 15:39:58 +04:00 committed by GitHub
parent fef0225a22
commit 9c0e50e1cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 168 additions and 109 deletions

View File

@ -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,
});
}

View File

@ -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();

View File

@ -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();
}
}

View File

@ -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();
}

View File

@ -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;
}

View File

@ -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 = [];
}

View File

@ -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);
});
}

View File

@ -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,
});
}

View File

@ -46,6 +46,7 @@ acceptance("Chat | User status on mentions", function (needs) {
cooked: `<p>Hey <a class="mention" href="/u/${mentionedUser1.username}">@${mentionedUser1.username}</a></p>`,
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],