PERF: Client side triggering multiple requests when opening composer (#21823)
What is the problem? There are two problems being fixed here: 1. 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. 2. System tests have animations disabled so the `transitionend` event does not fire at all. What is the solution? Instead of relying on the `transitionend` event, we can instead just observer the `composerState` property of the `ComposerBody` component and trigger the `composer:opened` appEvent with a delay that is similar to the transition duration used for the `ComposerBody` component.
This commit is contained in:
parent
7610553c82
commit
e1ba4c6b73
|
@ -170,20 +170,7 @@ export default Component.extend(KeyEnterEscape, {
|
||||||
|
|
||||||
didInsertElement() {
|
didInsertElement() {
|
||||||
this._super(...arguments);
|
this._super(...arguments);
|
||||||
|
|
||||||
this.setupComposerResizeEvents();
|
this.setupComposerResizeEvents();
|
||||||
|
|
||||||
const triggerOpen = () => {
|
|
||||||
if (this.get("composer.composeState") === Composer.OPEN) {
|
|
||||||
this.appEvents.trigger("composer:opened");
|
|
||||||
}
|
|
||||||
};
|
|
||||||
triggerOpen();
|
|
||||||
|
|
||||||
this.element.addEventListener("transitionend", () => {
|
|
||||||
triggerOpen();
|
|
||||||
});
|
|
||||||
|
|
||||||
positioningWorkaround(this.element);
|
positioningWorkaround(this.element);
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
|
@ -5,6 +5,7 @@ import discourseComputed, {
|
||||||
observes,
|
observes,
|
||||||
on,
|
on,
|
||||||
} from "discourse-common/utils/decorators";
|
} from "discourse-common/utils/decorators";
|
||||||
|
import discourseLater from "discourse-common/lib/later";
|
||||||
import {
|
import {
|
||||||
emailValid,
|
emailValid,
|
||||||
escapeExpression,
|
escapeExpression,
|
||||||
|
@ -225,6 +226,12 @@ const Composer = RestModel.extend({
|
||||||
if (this.composeState === OPEN) {
|
if (this.composeState === OPEN) {
|
||||||
this.set("composerOpened", oldOpen || new Date());
|
this.set("composerOpened", oldOpen || new Date());
|
||||||
elem.classList.add("composer-open");
|
elem.classList.add("composer-open");
|
||||||
|
|
||||||
|
// If the duration changes here, it must also be changed in `stylesheets/common/base/compose.scss` which is where
|
||||||
|
// the transition duration is defined. A delay is added here as we only want to fire the event after the transition ends.
|
||||||
|
discourseLater(() => {
|
||||||
|
this.appEvents.trigger("composer:opened");
|
||||||
|
}, 250);
|
||||||
} else {
|
} else {
|
||||||
if (oldOpen) {
|
if (oldOpen) {
|
||||||
const oldTotal = this.composerTotalOpened || 0;
|
const oldTotal = this.composerTotalOpened || 0;
|
||||||
|
|
|
@ -138,9 +138,6 @@ acceptance("Composer - Messages - Duplicate links", function (needs) {
|
||||||
await visit("/t/internationalization-localization/280");
|
await visit("/t/internationalization-localization/280");
|
||||||
await click("button.create");
|
await click("button.create");
|
||||||
|
|
||||||
// Work around the lack of CSS transitions in the test env
|
|
||||||
query("#reply-control").dispatchEvent(new Event("transitionend"));
|
|
||||||
|
|
||||||
assert
|
assert
|
||||||
.dom(".composer-popup")
|
.dom(".composer-popup")
|
||||||
.doesNotExist("composer warning is not shown by default");
|
.doesNotExist("composer warning is not shown by default");
|
||||||
|
|
|
@ -23,8 +23,11 @@ html.composer-open {
|
||||||
min-width: 0;
|
min-width: 0;
|
||||||
}
|
}
|
||||||
z-index: z("composer", "content");
|
z-index: z("composer", "content");
|
||||||
|
|
||||||
|
// If the transition duration is changed here, it must also be changed in the `composeStateChanged` function of models/composer.js
|
||||||
transition: height 250ms ease, background 250ms ease, transform 250ms ease,
|
transition: height 250ms ease, background 250ms ease, transform 250ms ease,
|
||||||
max-width 250ms ease, padding-bottom 250ms ease;
|
max-width 250ms ease, padding-bottom 250ms ease;
|
||||||
|
|
||||||
background-color: var(--secondary);
|
background-color: var(--secondary);
|
||||||
box-shadow: shadow("composer");
|
box-shadow: shadow("composer");
|
||||||
|
|
||||||
|
|
|
@ -12,10 +12,6 @@ describe "Composer don't feed the trolls popup", type: :system, js: true do
|
||||||
before { sign_in user }
|
before { sign_in user }
|
||||||
|
|
||||||
it "shows a popup when about to reply to a troll" do
|
it "shows a popup when about to reply to a troll" do
|
||||||
skip(
|
|
||||||
"TGX: This does not work when Capybara.disable_animation is set to true. We're in the midst of fixing this.",
|
|
||||||
)
|
|
||||||
|
|
||||||
SiteSetting.educate_until_posts = 0
|
SiteSetting.educate_until_posts = 0
|
||||||
|
|
||||||
topic_page.visit_topic(topic)
|
topic_page.visit_topic(topic)
|
||||||
|
|
Loading…
Reference in New Issue