PERF: Lazily lookup emoji-picker selected-diversity (#16917)

Looking up values from the `emojiStore` calls out to the browser's localStorage API and then decodes a JSON blob. This makes it relatively slow.

Previously we were doing this lookup in the emoji-picker's `init()` function, even if `isActive` was false. If many inactive emoji pickers are rendered simultaneously (e.g. for discourse-chat reactions), this performance hit quickly adds up.

This commit updates the service to notify about changes, and uses a computed property to provide a cached value in the emoji-picker.
This commit is contained in:
David Taylor 2022-05-26 12:37:09 +01:00 committed by GitHub
parent aabbc9e63e
commit b850c12793
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 4 deletions

View File

@ -1,5 +1,8 @@
import { action, computed } from "@ember/object";
import { bind, observes } from "discourse-common/utils/decorators";
import discourseComputed, {
bind,
observes,
} from "discourse-common/utils/decorators";
import {
emojiSearch,
extendedEmojiList,
@ -31,7 +34,6 @@ export default Component.extend({
emojiStore: service("emoji-store"),
tagName: "",
customEmojis: null,
selectedDiversity: null,
recentEmojis: null,
hoveredEmoji: null,
isActive: false,
@ -42,7 +44,6 @@ export default Component.extend({
this._super(...arguments);
this.set("customEmojis", customEmojis());
this.set("selectedDiversity", this.emojiStore.diversity);
if ("IntersectionObserver" in window) {
this._sectionObserver = this._setupSectionObserver();
@ -55,6 +56,13 @@ export default Component.extend({
this.appEvents.on("emoji-picker:close", this, "onClose");
},
// `readOnly` may seem like a better choice here, but the computed property
// provides caching (emojiStore.diversity is a simple getter)
@discourseComputed("emojiStore.diversity")
selectedDiversity(diversity) {
return diversity;
},
// didReceiveAttrs would be a better choice here, but this is sadly causing
// too many unexpected reloads as it's triggered for other reasons than a mutation
// of isActive
@ -186,7 +194,6 @@ export default Component.extend({
onDiversitySelection(index) {
const scale = index + 1;
this.emojiStore.diversity = scale;
this.set("selectedDiversity", scale);
this._applyDiversity(scale);
},

View File

@ -23,6 +23,7 @@ export default Service.extend({
set diversity(value) {
this.store.setObject({ key: EMOJI_SELECTED_DIVERSITY, value: value || 1 });
this.notifyPropertyChange("diversity");
},
get favorites() {