PERF: Update widget hooks to avoid global scroll-blocking events (#15116)

For widget event handlers, we register a single listener on the `<body>`, and then notify the relavent widget (if any) when the event fires.

`touchstart` and `touchmove` events are particularly performance sensitive because they block scrolling on mobile. Therefore we want to avoid registering global non-passive listeners for these events.

This commit updates the WidgetTouchStartHook and WidgetDragHook implementations to automatically register listeners on the specific widget DOM elements when required.

This commit removes the last global scroll-blocking event handler from Discourse core. That means that mobile scrolling is now completely decoupled from our JS app. Even if the JS app is completely blocked (e.g. during rendering), scrolling will now continue to work. This should make things feel a lot smoother, especially on lower performance devices.
This commit is contained in:
David Taylor 2021-11-28 10:47:44 +00:00 committed by GitHub
parent bca5c58c90
commit 1b9cf1b1c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 90 additions and 74 deletions

View File

@ -1,4 +1,5 @@
/*eslint no-loop-func:0*/
import { bind } from "discourse-common/utils/decorators";
const CLICK_ATTRIBUTE_NAME = "_discourse_click_widget";
const DOUBLE_CLICK_ATTRIBUTE_NAME = "_discourse_double_click_widget";
@ -7,7 +8,6 @@ const MOUSE_DOWN_OUTSIDE_ATTRIBUTE_NAME =
"_discourse_mouse_down_outside_widget";
const KEY_UP_ATTRIBUTE_NAME = "_discourse_key_up_widget";
const KEY_DOWN_ATTRIBUTE_NAME = "_discourse_key_down_widget";
const DRAG_ATTRIBUTE_NAME = "_discourse_drag_widget";
const INPUT_ATTRIBUTE_NAME = "_discourse_input_widget";
const CHANGE_ATTRIBUTE_NAME = "_discourse_change_widget";
const MOUSE_DOWN_ATTRIBUTE_NAME = "_discourse_mouse_down_widget";
@ -15,15 +15,16 @@ const MOUSE_UP_ATTRIBUTE_NAME = "_discourse_mouse_up_widget";
const MOUSE_MOVE_ATTRIBUTE_NAME = "_discourse_mouse_move_widget";
const MOUSE_OVER_ATTRIBUTE_NAME = "_discourse_mouse_over_widget";
const MOUSE_OUT_ATTRIBUTE_NAME = "_discourse_mouse_out_widget";
const TOUCH_START_ATTRIBUTE_NAME = "_discourse_touch_start_widget";
const TOUCH_END_ATTRIBUTE_NAME = "_discourse_touch_end_widget";
function buildHook(attributeName, setAttr) {
return class {
constructor(widget) {
this.widget = widget;
}
class WidgetBaseHook {
constructor(widget) {
this.widget = widget;
}
}
function buildHook(attributeName, setAttr) {
return class extends WidgetBaseHook {
hook(node) {
if (setAttr) {
node.setAttribute(setAttr, true);
@ -40,6 +41,8 @@ function buildHook(attributeName, setAttr) {
};
}
// For the majority of events, we register a single listener on the `<body>`, and then
// notify the relavent widget (if any) when the event fires (see setupDocumentCallback() below)
export const WidgetClickHook = buildHook(CLICK_ATTRIBUTE_NAME);
export const WidgetDoubleClickHook = buildHook(DOUBLE_CLICK_ATTRIBUTE_NAME);
export const WidgetClickOutsideHook = buildHook(
@ -52,7 +55,6 @@ export const WidgetMouseDownOutsideHook = buildHook(
);
export const WidgetKeyUpHook = buildHook(KEY_UP_ATTRIBUTE_NAME);
export const WidgetKeyDownHook = buildHook(KEY_DOWN_ATTRIBUTE_NAME);
export const WidgetDragHook = buildHook(DRAG_ATTRIBUTE_NAME);
export const WidgetInputHook = buildHook(INPUT_ATTRIBUTE_NAME);
export const WidgetChangeHook = buildHook(CHANGE_ATTRIBUTE_NAME);
export const WidgetMouseUpHook = buildHook(MOUSE_UP_ATTRIBUTE_NAME);
@ -60,9 +62,88 @@ export const WidgetMouseDownHook = buildHook(MOUSE_DOWN_ATTRIBUTE_NAME);
export const WidgetMouseMoveHook = buildHook(MOUSE_MOVE_ATTRIBUTE_NAME);
export const WidgetMouseOverHook = buildHook(MOUSE_OVER_ATTRIBUTE_NAME);
export const WidgetMouseOutHook = buildHook(MOUSE_OUT_ATTRIBUTE_NAME);
export const WidgetTouchStartHook = buildHook(TOUCH_START_ATTRIBUTE_NAME);
export const WidgetTouchEndHook = buildHook(TOUCH_END_ATTRIBUTE_NAME);
// `touchstart` and `touchmove` events are particularly performance sensitive because
// they block scrolling on mobile. Therefore we want to avoid registering global non-passive
// listeners for these events.
// Instead, the WidgetTouchStartHook and WidgetDragHook automatically register listeners on
// the specific widget DOM elements when required.
export class WidgetTouchStartHook extends WidgetBaseHook {
hook(node, propertyName, previousValue) {
if (!previousValue) {
// Adding to DOM
node.addEventListener("touchstart", this.callback, { passive: false });
}
}
unhook(node, propertyName, newValue) {
if (!newValue) {
node.removeEventListener("touchstart", this.callback);
}
}
@bind
callback(e) {
this.widget.touchStart(e);
}
}
let _currentlyDraggingHook;
export class WidgetDragHook extends WidgetBaseHook {
hook(node, propertyName, previousValue) {
if (!previousValue) {
// Adding to DOM
node.addEventListener("touchstart", this.startDrag, { passive: false });
node.addEventListener("mousedown", this.startDrag, { passive: false });
}
}
unhook(node, propertyName, newValue) {
if (!newValue) {
// Removing from DOM
node.removeEventListener("touchstart", this.startDrag);
node.removeEventListener("mousedown", this.startDrag);
}
}
@bind
startDrag(e) {
e.preventDefault();
e.stopPropagation();
_currentlyDraggingHook?.dragEnd();
_currentlyDraggingHook = this;
document.body.classList.add("widget-dragging");
document.addEventListener("touchmove", this.drag, { passive: false });
document.addEventListener("mousemove", this.drag, { passive: false });
document.addEventListener("touchend", this.dragEnd);
document.addEventListener("mouseup", this.dragEnd);
}
@bind
drag(e) {
if (event.type === "mousemove") {
this.widget.drag(e);
} else {
const tt = e.targetTouches[0];
e.preventDefault();
e.stopPropagation();
this.widget.drag(tt);
}
}
@bind
dragEnd(e) {
document.body.classList.remove("widget-dragging");
document.removeEventListener("touchmove", this.drag);
document.removeEventListener("mousemove", this.drag);
document.removeEventListener("touchend", this.dragEnd);
document.removeEventListener("mouseup", this.dragEnd);
this.widget.dragEnd(e);
_currentlyDraggingHook = null;
}
}
function nodeCallback(node, attrName, cb, options = { rerender: true }) {
const { rerender } = options;
const widget = findWidget(node, attrName);
@ -87,39 +168,12 @@ function findWidget(node, attrName) {
}
let _watchingDocument = false;
let _dragging;
const DRAG_NAME = "mousemove.discourse-widget-drag";
function cancelDrag(e) {
$("body").removeClass("widget-dragging");
$(document).off(DRAG_NAME);
// We leave the touchmove event cause touch needs it always bound on iOS
if (_dragging) {
if (_dragging.dragEnd) {
_dragging.dragEnd(e);
}
_dragging = null;
}
}
WidgetClickHook.setupDocumentCallback = function () {
if (_watchingDocument) {
return;
}
let widget;
let onDrag = (dragE) => {
const tt = dragE.targetTouches[0];
if (tt && widget) {
dragE.preventDefault();
dragE.stopPropagation();
widget.drag(tt);
}
};
$(document).on("mouseover.discourse-widget", (e) => {
nodeCallback(e.target, MOUSE_OVER_ATTRIBUTE_NAME, (w) => w.mouseOver(e), {
rerender: false,
@ -132,38 +186,6 @@ WidgetClickHook.setupDocumentCallback = function () {
});
});
document.addEventListener("touchmove", onDrag, {
passive: false,
capture: true,
});
$(document).on(
"mousedown.discource-widget-drag, touchstart.discourse-widget-drag",
(e) => {
cancelDrag(e);
widget = findWidget(e.target, DRAG_ATTRIBUTE_NAME);
if (widget) {
e.preventDefault();
e.stopPropagation();
_dragging = widget;
$("body").addClass("widget-dragging");
$(document).on(DRAG_NAME, (dragE) => {
if (widget) {
widget.drag(dragE);
}
});
}
}
);
$(document).on(
"mouseup.discourse-widget-drag, touchend.discourse-widget-drag",
(e) => {
widget = null;
cancelDrag(e);
}
);
$(document).on("dblclick.discourse-widget", (e) => {
nodeCallback(e.target, DOUBLE_CLICK_ATTRIBUTE_NAME, (w) =>
w.doubleClick(e)
@ -224,12 +246,6 @@ WidgetClickHook.setupDocumentCallback = function () {
});
});
$(document).on("touchstart.discourse-widget", (e) => {
nodeCallback(e.target, TOUCH_START_ATTRIBUTE_NAME, (w) => w.touchStart(e), {
rerender: false,
});
});
$(document).on("touchend.discourse-widget", (e) => {
nodeCallback(e.target, TOUCH_END_ATTRIBUTE_NAME, (w) => w.touchEnd(e), {
rerender: false,