github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-06-09 17:28:13 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

PERF: Client side triggering multiple requests when opening composer (#21925)

Browse Source 
What is the problem?

When opening the composer, we are seeing multiple requests made to
the `/composer_messages` endpoint. This is due to our use of the
`transitionend` event on the `#reply-control` element. The event is
fired once for each transition event and the `#reply-control` element
has multiple transition events.

What is the solution?

Since are only interested in the `height` transition event, we add a
condition to check that the callback function is only triggered when the
`propertyName` of the `transitionend` event is `height`.

Why is there no tests for this change?

In QUnit, we have `transition: none !important` set in the stylesheet
with no easy way to disable. We'll have to accept the risk of not
writing test for this performance fix.
This commit is contained in:
Alan Guo Xiang Tan
2023-06-05 12:12:38 +09:00
committed by GitHub
parent 7bd826ef11
commit ce2bd96590
2 changed files with 8 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/assets/javascripts/discourse/app/components/composer-body.js
View File

@ -180,8 +180,10 @@ export default Component.extend(KeyEnterEscape, {
}; };
triggerOpen(); triggerOpen();
this.element.addEventListener("transitionend", () => { this.element.addEventListener("transitionend", (event) => {
if (event.propertyName === "height") {
triggerOpen(); triggerOpen();
}
}); });
positioningWorkaround(this.element); positioningWorkaround(this.element);

5
app/assets/javascripts/discourse/tests/acceptance/composer-messages-test.js
View File

@ -16,6 +16,7 @@ import pretender, { response } from "../helpers/create-pretender";
acceptance("Composer - Messages", function (needs) { acceptance("Composer - Messages", function (needs) {
needs.user(); needs.user();
needs.pretender((server, helper) => { needs.pretender((server, helper) => {
server.get("/composer_messages/user_not_seen_in_a_while", () => { server.get("/composer_messages/user_not_seen_in_a_while", () => {
return helper.response({ return helper.response({
@ -139,7 +140,9 @@ acceptance("Composer - Messages - Duplicate links", function (needs) {
await click("button.create"); await click("button.create");
// Work around the lack of CSS transitions in the test env // Work around the lack of CSS transitions in the test env
query("#reply-control").dispatchEvent(new Event("transitionend")); const event = new Event("transitionend");
event.propertyName = "height";
query("#reply-control").dispatchEvent(event);
assert assert
.dom(".composer-popup") .dom(".composer-popup")