From 70f7d369fb6f9f5ae05e8b3c2731b9b387622de5 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 28 Mar 2024 21:13:02 +0800 Subject: [PATCH] DEV: Rewrite `SchemaThemeSetting::Editor` to avoid rerendering problems (#26416) Why this change? Prior to this change, the `SchemaThemeSetting::Editor#tree` was creating a new `Tree` instance which holds instances of `Node`. Both classes consisted of tracked properties. The problem with this approach is that when any tracked properties is updated, Ember will revaluate `SchemaThemeSetting::Editor#tree` and because that method always return a new instance of `Tree`, it causes the whole navigation tree to rerender just because on tracked property changed. This rerendering of the whole navigation tree every time made it hard to implement simple features like hiding a section in 9baa820d53757aec36e4cb86efab8d0bb3f264c0. Instead of being able to just declare a tracked property to hide/show a section, we end up with a more complicated solution. This commit rewrites `SchemaThemeSetting::Editor` to depend on Ember components to form the tree structure instead. As needed, each component in the tree structure can declare its own tracked property as necessary. --- .../schema-theme-setting/editor.gjs | 569 ++++++------------ .../editor/child-tree-node.gjs | 16 + .../editor/child-tree.gjs | 63 ++ .../schema-theme-setting/editor/tree-node.gjs | 90 +++ .../schema-theme-setting/editor/tree.gjs | 47 ++ 5 files changed, 406 insertions(+), 379 deletions(-) create mode 100644 app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/child-tree-node.gjs create mode 100644 app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/child-tree.gjs create mode 100644 app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree-node.gjs create mode 100644 app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree.gjs diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs index 2d34e124a13..c0a9ec9dfec 100644 --- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs @@ -1,160 +1,161 @@ import Component from "@glimmer/component"; -import { cached, tracked } from "@glimmer/tracking"; +import { tracked } from "@glimmer/tracking"; import { fn } from "@ember/helper"; -import { on } from "@ember/modifier"; import { action } from "@ember/object"; import { service } from "@ember/service"; import { gt } from "truth-helpers"; import DButton from "discourse/components/d-button"; import { popupAjaxError } from "discourse/lib/ajax-error"; -import dIcon from "discourse-common/helpers/d-icon"; import { cloneJSON } from "discourse-common/lib/object"; import I18n from "discourse-i18n"; -import FieldInput from "./field"; +import Tree from "admin/components/schema-theme-setting/editor/tree"; +import FieldInput from "admin/components/schema-theme-setting/field"; -class Node { - @tracked text; - object; - schema; - index; - active = false; - parentTree; - trees = []; - - constructor({ text, index, object, schema, parentTree }) { - this.text = text; - this.index = index; - this.object = object; - this.schema = schema; - this.parentTree = parentTree; - } -} - -class Tree { - @tracked nodes = []; - data = []; - propertyName; - schema; -} - -export default class SchemaThemeSettingEditor extends Component { +export default class SchemaThemeSettingNewEditor extends Component { @service router; + + @tracked history = []; @tracked activeIndex = 0; - @tracked backButtonText; + @tracked activeDataPaths = []; + @tracked activeSchemaPaths = []; @tracked saveButtonDisabled = false; - @tracked visibilityStates = []; + inputFieldObserver = new Map(); data = cloneJSON(this.args.setting.value); - history = []; schema = this.args.setting.objects_schema; - @cached - get tree() { - let schema = this.schema; - let data = this.data; - let tree = new Tree(); - tree.data = data; - tree.schema = schema; - - for (const point of this.history) { - data = data[point.parentNode.index][point.propertyName]; - schema = schema.properties[point.propertyName].schema; - - tree.propertyName = point.propertyName; - tree.schema = point.node.schema; - tree.data = data; - } - - data.forEach((object, index) => { - const node = new Node({ - index, - schema, - object, - text: - object[schema.identifier] || - this.defaultSchemaIdentifier(schema.name, index), - parentTree: tree, - }); - - if (index === this.activeIndex) { - node.active = true; - - const childObjectsProperties = this.findChildObjectsProperties( - schema.properties - ); - - for (const childObjectsProperty of childObjectsProperties) { - const subtree = new Tree(); - subtree.propertyName = childObjectsProperty.name; - subtree.schema = childObjectsProperty.schema; - subtree.data = data[index][childObjectsProperty.name] ||= []; - - data[index][childObjectsProperty.name]?.forEach( - (childObj, childIndex) => { - subtree.nodes.push( - new Node({ - text: - childObj[childObjectsProperty.schema.identifier] || - `${childObjectsProperty.schema.name} ${childIndex + 1}`, - index: childIndex, - object: childObj, - schema: childObjectsProperty.schema, - parentTree: subtree, - }) - ); - } - ); - - node.trees.push(subtree); - } - } - - tree.nodes.push(node); + @action + onChildClick(index, propertyName, parentNodeIndex) { + this.history.pushObject({ + dataPaths: [...this.activeDataPaths], + schemaPaths: [...this.activeSchemaPaths], + index: this.activeIndex, }); - return tree; + this.activeIndex = index; + this.activeDataPaths.pushObjects([parentNodeIndex, propertyName]); + this.activeSchemaPaths.pushObject(propertyName); + this.inputFieldObserver.clear(); } - @cached - get activeNode() { - return this.tree.nodes.find((node, index) => { - return index === this.activeIndex; + @action + updateIndex(index) { + this.activeIndex = index; + } + + generateSchemaTitle(object, schema, index) { + return object[schema.identifier] || `${schema.name} ${index + 1}`; + } + + get backButtonText() { + if (this.history.length === 0) { + return; + } + + const lastHistory = this.history[this.history.length - 1]; + + return I18n.t("admin.customize.theme.schema.back_button", { + name: this.generateSchemaTitle( + this.#resolveDataFromPaths(lastHistory.dataPaths)[lastHistory.index], + this.#resolveSchemaFromPaths(lastHistory.schemaPaths), + lastHistory.index + ), }); } + get activeData() { + return this.#resolveDataFromPaths(this.activeDataPaths); + } + + #resolveDataFromPaths(paths) { + if (paths.length === 0) { + return this.data; + } + + let data = this.data; + + paths.forEach((path) => { + data = data[path]; + }); + + return data; + } + + get activeSchema() { + return this.#resolveSchemaFromPaths(this.activeSchemaPaths); + } + + #resolveSchemaFromPaths(paths) { + if (paths.length === 0) { + return this.schema; + } + + let schema = this.schema; + + paths.forEach((path) => { + schema = schema.properties[path].schema; + }); + + return schema; + } + + @action + registerInputFieldObserver(index, callback) { + this.inputFieldObserver[index] = callback; + } + + @action + unregisterInputFieldObserver(index) { + delete this.inputFieldObserver[index]; + } + + descriptions(fieldName, key) { + // The `property_descriptions` metadata is an object with keys in the following format as an example: + // + // { + // some_property.description: , + // some_property.label: , + // some_objects_property.some_other_property.description: , + // some_objects_property.some_other_property.label: , + // } + const descriptions = this.args.setting.metadata?.property_descriptions; + + if (!descriptions) { + return; + } + + if (this.activeSchemaPaths.length > 0) { + key = `${this.activeSchemaPaths.join(".")}.${fieldName}.${key}`; + } else { + key = `${fieldName}.${key}`; + } + + return descriptions[key]; + } + + fieldLabel(fieldName) { + return this.descriptions(fieldName, "label") || fieldName; + } + + fieldDescription(fieldName) { + return this.descriptions(fieldName, "description"); + } + get fields() { - const node = this.activeNode; const list = []; - if (!node) { - return list; - } + if (this.activeData.length !== 0) { + for (const [name, spec] of Object.entries(this.activeSchema.properties)) { + if (spec.type === "objects") { + continue; + } - for (const [name, spec] of Object.entries(node.schema.properties)) { - if (spec.type === "objects") { - continue; - } - - list.push({ - name, - spec, - value: node.object[name], - description: this.fieldDescription(name), - label: this.fieldLabel(name), - }); - } - - return list; - } - - findChildObjectsProperties(properties) { - const list = []; - - for (const [name, spec] of Object.entries(properties)) { - if (spec.type === "objects") { list.push({ name, - schema: spec.schema, + spec, + value: this.activeData[this.activeIndex][name], + description: this.fieldDescription(name), + label: this.fieldLabel(name), }); } } @@ -162,6 +163,52 @@ export default class SchemaThemeSettingEditor extends Component { return list; } + @action + clickBack() { + const { + dataPaths: lastDataPaths, + schemaPaths: lastSchemaPaths, + index: lastIndex, + } = this.history.popObject(); + + this.activeDataPaths = lastDataPaths; + this.activeSchemaPaths = lastSchemaPaths; + this.activeIndex = lastIndex; + this.inputFieldObserver.clear(); + } + + @action + addChildItem(propertyName, parentNodeIndex) { + this.activeData[parentNodeIndex][propertyName].pushObject({}); + } + + @action + addItem() { + this.activeData.pushObject({}); + } + + @action + removeItem() { + this.activeData.removeAt(this.activeIndex); + + if (this.activeData.length > 0) { + this.activeIndex = Math.max(this.activeIndex - 1, 0); + } else if (this.history.length > 0) { + this.clickBack(); + } else { + this.activeIndex = 0; + } + } + + @action + inputFieldChanged(field, newVal) { + this.activeData[this.activeIndex][field.name] = newVal; + + if (field.name === this.activeSchema.identifier) { + this.inputFieldObserver[this.activeIndex](); + } + } + @action saveChanges() { this.saveButtonDisabled = true; @@ -180,260 +227,23 @@ export default class SchemaThemeSettingEditor extends Component { .finally(() => (this.saveButtonDisabled = false)); } - @action - onClick(node) { - this.activeIndex = node.index; - } - - @action - onChildClick(node, tree, parentNode) { - this.history.push({ - propertyName: tree.propertyName, - parentNode, - node, - }); - - this.backButtonText = I18n.t("admin.customize.theme.schema.back_button", { - name: parentNode.text, - }); - - this.activeIndex = node.index; - } - - @action - backButtonClick() { - const historyPoint = this.history.pop(); - this.activeIndex = historyPoint.parentNode.index; - - if (this.history.length > 0) { - this.backButtonText = I18n.t("admin.customize.theme.schema.back_button", { - name: this.history[this.history.length - 1].parentNode.text, - }); - } else { - this.backButtonText = null; - } - } - - @action - inputFieldChanged(field, newVal) { - if (field.name === this.activeNode.schema.identifier) { - this.activeNode.text = newVal; - } - - this.activeNode.object[field.name] = newVal; - } - - @action - addItem(tree) { - const schema = tree.schema; - const node = this.createNodeFromSchema(schema, tree); - tree.data.push(node.object); - tree.nodes = [...tree.nodes, node]; - } - - @action - removeItem() { - const data = this.activeNode.parentTree.data; - data.splice(this.activeIndex, 1); - this.tree.nodes = this.tree.nodes.filter((n, i) => i !== this.activeIndex); - - if (data.length > 0) { - this.activeIndex = Math.max(this.activeIndex - 1, 0); - } else if (this.history.length > 0) { - this.backButtonClick(); - } - } - - @action - toggleListVisibility(listIdentifier) { - if (this.visibilityStates.includes(listIdentifier)) { - this.visibilityStates = this.visibilityStates.filter( - (id) => id !== listIdentifier - ); - } else { - this.visibilityStates = [...this.visibilityStates, listIdentifier]; - } - } - - get isListVisible() { - return (listIdentifier) => { - return this.visibilityStates.includes(listIdentifier); - }; - } - - descriptions(fieldName, key) { - // The `property_descriptions` metadata is an object with keys in the following format as an example: - // - // { - // some_property.description: , - // some_property.label: , - // some_objects_property.some_other_property.description: , - // some_objects_property.some_other_property.label: , - // } - const descriptions = this.args.setting.metadata?.property_descriptions; - - if (!descriptions) { - return; - } - - if (this.activeNode.parentTree.propertyName) { - key = `${this.activeNode.parentTree.propertyName}.${fieldName}.${key}`; - } else { - key = `${fieldName}.${key}`; - } - - return descriptions[key]; - } - - fieldLabel(fieldName) { - return this.descriptions(fieldName, "label") || fieldName; - } - - fieldDescription(fieldName) { - return this.descriptions(fieldName, "description"); - } - - defaultSchemaIdentifier(schemaName, index) { - return `${schemaName} ${index + 1}`; - } - - createNodeFromSchema(schema, tree) { - const object = {}; - const index = tree.nodes.length; - const defaultName = this.defaultSchemaIdentifier(schema.name, index); - - if (schema.identifier) { - object[schema.identifier] = defaultName; - } - - for (const [name, spec] of Object.entries(schema.properties)) { - if (spec.type === "objects") { - object[name] = []; - } - } - - return new Node({ - schema, - object, - index, - text: defaultName, - parentTree: tree, - }); - } - - uniqueNodeId(nestedTreePropertyName, nodeIndex) { - return `${nestedTreePropertyName}-${nodeIndex}`; - } - diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/child-tree.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/child-tree.gjs new file mode 100644 index 00000000000..1e3064f732e --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/child-tree.gjs @@ -0,0 +1,63 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { fn } from "@ember/helper"; +import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import DButton from "discourse/components/d-button"; +import dIcon from "discourse-common/helpers/d-icon"; +import ChildTreeNode from "admin/components/schema-theme-setting/editor/child-tree-node"; + +export default class SchemaThemeSettingNewEditorChildTree extends Component { + @tracked expanded = true; + + @action + toggleVisibility() { + this.expanded = !this.expanded; + } + + @action + onChildClick(index) { + return this.args.onChildClick( + index, + this.args.name, + this.args.parentNodeIndex, + this.args.parentNodeText + ); + } + + +} diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree-node.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree-node.gjs new file mode 100644 index 00000000000..fef04453482 --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree-node.gjs @@ -0,0 +1,90 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { fn, get } from "@ember/helper"; +import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import didInsert from "@ember/render-modifiers/modifiers/did-insert"; +import { gt } from "truth-helpers"; +import dIcon from "discourse-common/helpers/d-icon"; +import ChildTree from "admin/components/schema-theme-setting/editor/child-tree"; + +export default class SchemaThemeSettingNewEditorTreeNode extends Component { + @tracked text; + + childObjectsProperties = this.findChildObjectsProperties( + this.args.schema.properties + ); + + constructor() { + super(...arguments); + this.#setText(); + } + + @action + registerInputFieldObserver() { + this.args.registerInputFieldObserver( + this.args.index, + this.#setText.bind(this) + ); + } + + #setText() { + this.text = this.args.generateSchemaTitle( + this.args.object, + this.args.schema, + this.args.index + ); + } + + findChildObjectsProperties(properties) { + const list = []; + + for (const [name, spec] of Object.entries(properties)) { + if (spec.type === "objects") { + this.args.object[name] ||= []; + + list.push({ + name, + schema: spec.schema, + }); + } + } + + return list; + } + + +} diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree.gjs new file mode 100644 index 00000000000..e2ee9f26116 --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor/tree.gjs @@ -0,0 +1,47 @@ +import { fn } from "@ember/helper"; +import { on } from "@ember/modifier"; +import { eq } from "truth-helpers"; +import DButton from "discourse/components/d-button"; +import dIcon from "discourse-common/helpers/d-icon"; +import TreeNode from "admin/components/schema-theme-setting/editor/tree-node"; + +