UX: First pass styling experimental objects typed setting editor (#26194)

Why this change?

This is a first pass at styling the editor for creating/editing/updating
an objects typed theme setting. Only the desktop view is being
considered at the current moment.

The objects typed theme setting is still behind a feature flag at this moment so there is no need for us to get the styling perfect. The purpose of this PR is to get us to a state which we can quickly iterate with a designer on.

app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs

@@ -5,8 +5,10 @@ import { on } from "@ember/modifier";
 import { action } from "@ember/object";
 import { LinkTo } from "@ember/routing";
 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 i18n from "discourse-common/helpers/i18n";
 import { cloneJSON } from "discourse-common/lib/object";
 import I18n from "discourse-i18n";
@@ -77,7 +79,7 @@ export default class SchemaThemeSettingEditor extends Component {
           const subtree = new Tree();
           subtree.propertyName = childObjectsProperty.name;
 
-          data[index][childObjectsProperty.name].forEach(
+          data[index][childObjectsProperty.name]?.forEach(
             (childObj, childIndex) => {
               subtree.nodes.push(
                 new Node({
@@ -224,68 +226,99 @@ export default class SchemaThemeSettingEditor extends Component {
   }
 
   <template>
-    <div class="schema-editor-navigation">
-      {{#if this.backButtonText}}
-        <DButton
-          @action={{this.backButtonClick}}
-          @icon="chevron-left"
-          @translatedLabel={{this.backButtonText}}
-          class="back-button"
-        />
-      {{/if}}
-
-      <ul class="tree">
-        {{#each this.tree.nodes as |node|}}
-          <div class="item-container">
+    <div class="schema-theme-setting-editor">
+      <div class="schema-theme-setting-editor__navigation">
+        <ul class="schema-theme-setting-editor__tree">
+          {{#if this.backButtonText}}
             <li
               role="link"
-              class="parent node{{if node.active ' active'}}"
+              class="schema-theme-setting-editor__tree-node--back-btn"
+              {{on "click" this.backButtonClick}}
+            >
+              <div class="schema-theme-setting-editor__tree-node-text">
+                {{dIcon "chevron-left"}}
+                {{this.backButtonText}}
+              </div>
+            </li>
+          {{/if}}
+
+          {{#each this.tree.nodes as |node|}}
+            <li
+              role="link"
+              class="schema-theme-setting-editor__tree-node --parent
+                {{if node.active ' --active'}}"
               {{on "click" (fn this.onClick node)}}
             >
-              {{node.text}}
+              <div class="schema-theme-setting-editor__tree-node-text">
+                {{node.text}}
+
+                {{#if (gt node.trees.length 0)}}
+                  {{dIcon (if node.active "chevron-down" "chevron-right")}}
+                {{/if}}
+              </div>
             </li>
+
             {{#each node.trees as |nestedTree|}}
-              <ul>
-                {{#each nestedTree.nodes as |childNode|}}
-                  <li
-                    role="link"
-                    class="child node"
-                    {{on
-                      "click"
-                      (fn this.onChildClick childNode nestedTree node)
-                    }}
-                  >{{childNode.text}}</li>
-                {{/each}}
-              </ul>
+              {{#if (gt nestedTree.nodes.length 0)}}
+                <li
+                  class="schema-theme-setting-editor__tree-node --child --heading"
+                  data-test-parent-index={{node.index}}
+                >
+                  <div class="schema-theme-setting-editor__tree-node-text">
+                    {{nestedTree.propertyName}}
+                  </div>
+                </li>
+              {{/if}}
+
+              {{#each nestedTree.nodes as |childNode|}}
+                <li
+                  role="link"
+                  class="schema-theme-setting-editor__tree-node --child"
+                  {{on
+                    "click"
+                    (fn this.onChildClick childNode nestedTree node)
+                  }}
+                  data-test-parent-index={{node.index}}
+                >
+                  <div class="schema-theme-setting-editor__tree-node-text">
+                    {{childNode.text}}
+                    {{dIcon "chevron-right"}}
+                  </div>
+                </li>
+              {{/each}}
             {{/each}}
-          </div>
+          {{/each}}
+        </ul>
+      </div>
+
+      <div class="schema-theme-setting-editor__fields">
+        {{#each this.fields as |field|}}
+          <FieldInput
+            @name={{field.name}}
+            @value={{field.value}}
+            @spec={{field.spec}}
+            @onValueChange={{fn this.inputFieldChanged field}}
+            @description={{field.description}}
+          />
         {{/each}}
-      </ul>
+      </div>
 
-      {{#each this.fields as |field|}}
-        <FieldInput
-          @name={{field.name}}
-          @value={{field.value}}
-          @spec={{field.spec}}
-          @onValueChange={{fn this.inputFieldChanged field}}
-          @description={{field.description}}
+      <div class="schema-theme-setting-editor__footer">
+        <DButton
+          @disabled={{this.saveButtonDisabled}}
+          @action={{this.saveChanges}}
+          @label="save"
+          class="btn-primary"
         />
-      {{/each}}
+
+        <LinkTo
+          @route="adminCustomizeThemes.show"
+          @model={{@themeId}}
+          class="btn-transparent"
+        >
+          {{i18n "cancel"}}
+        </LinkTo>
+      </div>
     </div>
-
-    <DButton
-      @disabled={{this.saveButtonDisabled}}
-      @action={{this.saveChanges}}
-      @label="save"
-      class="btn-primary"
-    />
-
-    <LinkTo
-      @route="adminCustomizeThemes.show"
-      @model={{@themeId}}
-      class="btn-transparent"
-    >
-      {{i18n "cancel"}}
-    </LinkTo>
   </template>
 }

app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs

@@ -1,3 +1,12 @@
+<div class="customize-themes-show-schema__header row">
+  <h2>
+    {{i18n
+      "admin.customize.theme.schema.title"
+      (hash name=@model.setting.setting)
+    }}
+  </h2>
+</div>
+
 <SchemaThemeSetting::Editor
   @themeId={{@model.theme.id}}
   @setting={{@model.setting}}

app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs

@@ -3,7 +3,7 @@ import { module, test } from "qunit";
 import schemaAndData from "discourse/tests/fixtures/theme-setting-schema-data";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
 import pretender, { response } from "discourse/tests/helpers/create-pretender";
-import { queryAll } from "discourse/tests/helpers/qunit-helpers";
+import { query, queryAll } from "discourse/tests/helpers/qunit-helpers";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
 import I18n from "discourse-i18n";
 import AdminSchemaThemeSettingEditor from "admin/components/schema-theme-setting/editor";
@@ -15,23 +15,39 @@ class TreeFromDOM {
   }
 
   refresh() {
-    this.nodes = [...queryAll(".tree .item-container")].map((container) => {
-      const li = container.querySelector(".parent.node");
-      const active = li.classList.contains("active");
-      const children = [...container.querySelectorAll(".node.child")].map(
-        (child) => {
-          return {
-            text: child.textContent.trim(),
-            element: child,
-          };
-        }
+    this.nodes = [
+      ...queryAll(
+        ".schema-theme-setting-editor__tree .schema-theme-setting-editor__tree-node.--parent"
+      ),
+    ].map((container, index) => {
+      const li = container;
+      const active = li.classList.contains("--active");
+
+      const children = [
+        ...queryAll(
+          `.schema-theme-setting-editor__tree-node.--child[data-test-parent-index="${index}"]:not(.--heading)`
+        ),
+      ].map((child) => {
+        return {
+          element: child,
+          textElement: child.querySelector(
+            ".schema-theme-setting-editor__tree-node-text"
+          ),
+        };
+      });
+
+      const childrenHeaderElement = query(
+        `.schema-theme-setting-editor__tree-node.--heading[data-test-parent-index="${index}"]`
       );
 
       return {
-        text: li.textContent.trim(),
         active,
         children,
+        childrenHeaderElement,
         element: li,
+        textElement: li.querySelector(
+          ".schema-theme-setting-editor__tree-node-text"
+        ),
       };
     });
   }
@@ -85,6 +101,8 @@ module(
       const tree = new TreeFromDOM();
 
       assert.true(tree.nodes[0].active);
+      assert.dom(tree.nodes[0].childrenHeaderElement).hasText("children");
+
       assert.strictEqual(
         tree.nodes[0].children.length,
         2,
@@ -127,12 +145,12 @@ module(
       const tree = new TreeFromDOM();
 
       assert.strictEqual(tree.nodes.length, 2);
-      assert.strictEqual(tree.nodes[0].text, "item 1");
+      assert.dom(tree.nodes[0].textElement).hasText("item 1");
       assert.strictEqual(tree.nodes[0].children.length, 2);
-      assert.strictEqual(tree.nodes[0].children[0].text, "child 1-1");
-      assert.strictEqual(tree.nodes[0].children[1].text, "child 1-2");
+      assert.dom(tree.nodes[0].children[0].textElement).hasText("child 1-1");
+      assert.dom(tree.nodes[0].children[1].textElement).hasText("child 1-2");
 
-      assert.strictEqual(tree.nodes[1].text, "item 2");
+      assert.dom(tree.nodes[1].textElement).hasText("item 2");
       assert.strictEqual(tree.nodes[1].children.length, 0);
 
       await click(tree.nodes[1].element);
@@ -140,35 +158,47 @@ module(
       tree.refresh();
 
       assert.strictEqual(tree.nodes.length, 2);
-      assert.strictEqual(tree.nodes[0].text, "item 1");
+      assert.dom(tree.nodes[0].textElement).hasText("item 1");
       assert.false(tree.nodes[0].active);
       assert.strictEqual(tree.nodes[0].children.length, 0);
 
-      assert.strictEqual(tree.nodes[1].text, "item 2");
+      assert.dom(tree.nodes[1].textElement).hasText("item 2");
       assert.true(tree.nodes[1].active);
       assert.strictEqual(tree.nodes[1].children.length, 3);
-      assert.strictEqual(tree.nodes[1].children[0].text, "child 2-1");
-      assert.strictEqual(tree.nodes[1].children[1].text, "child 2-2");
-      assert.strictEqual(tree.nodes[1].children[2].text, "child 2-3");
+      assert.dom(tree.nodes[1].children[0].textElement).hasText("child 2-1");
+      assert.dom(tree.nodes[1].children[1].textElement).hasText("child 2-2");
+      assert.dom(tree.nodes[1].children[2].textElement).hasText("child 2-3");
 
       await click(tree.nodes[1].children[1].element);
 
       tree.refresh();
       assert.strictEqual(tree.nodes.length, 3);
 
-      assert.strictEqual(tree.nodes[0].text, "child 2-1");
+      assert.dom(tree.nodes[0].textElement).hasText("child 2-1");
       assert.false(tree.nodes[0].active);
       assert.strictEqual(tree.nodes[0].children.length, 0);
 
-      assert.strictEqual(tree.nodes[1].text, "child 2-2");
+      assert.dom(tree.nodes[1].textElement).hasText("child 2-2");
       assert.true(tree.nodes[1].active);
       assert.strictEqual(tree.nodes[1].children.length, 4);
-      assert.strictEqual(tree.nodes[1].children[0].text, "grandchild 2-2-1");
-      assert.strictEqual(tree.nodes[1].children[1].text, "grandchild 2-2-2");
-      assert.strictEqual(tree.nodes[1].children[2].text, "grandchild 2-2-3");
-      assert.strictEqual(tree.nodes[1].children[3].text, "grandchild 2-2-4");
 
-      assert.strictEqual(tree.nodes[2].text, "child 2-3");
+      assert
+        .dom(tree.nodes[1].children[0].textElement)
+        .hasText("grandchild 2-2-1");
+
+      assert
+        .dom(tree.nodes[1].children[1].textElement)
+        .hasText("grandchild 2-2-2");
+
+      assert
+        .dom(tree.nodes[1].children[2].textElement)
+        .hasText("grandchild 2-2-3");
+
+      assert
+        .dom(tree.nodes[1].children[3].textElement)
+        .hasText("grandchild 2-2-4");
+
+      assert.dom(tree.nodes[2].textElement).hasText("child 2-3");
       assert.false(tree.nodes[2].active);
       assert.strictEqual(tree.nodes[2].children.length, 0);
 
@@ -178,19 +208,19 @@ module(
 
       assert.strictEqual(tree.nodes.length, 4);
 
-      assert.strictEqual(tree.nodes[0].text, "grandchild 2-2-1");
+      assert.dom(tree.nodes[0].textElement).hasText("grandchild 2-2-1");
       assert.false(tree.nodes[0].active);
       assert.strictEqual(tree.nodes[0].children.length, 0);
 
-      assert.strictEqual(tree.nodes[1].text, "grandchild 2-2-2");
+      assert.dom(tree.nodes[1].textElement).hasText("grandchild 2-2-2");
       assert.true(tree.nodes[1].active);
       assert.strictEqual(tree.nodes[1].children.length, 0);
 
-      assert.strictEqual(tree.nodes[2].text, "grandchild 2-2-3");
+      assert.dom(tree.nodes[2].textElement).hasText("grandchild 2-2-3");
       assert.false(tree.nodes[2].active);
       assert.strictEqual(tree.nodes[2].children.length, 0);
 
-      assert.strictEqual(tree.nodes[3].text, "grandchild 2-2-4");
+      assert.dom(tree.nodes[3].textElement).hasText("grandchild 2-2-4");
       assert.false(tree.nodes[3].active);
       assert.strictEqual(tree.nodes[3].children.length, 0);
     });
@@ -202,32 +232,36 @@ module(
         <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
-      assert.dom(".back-button").doesNotExist();
+      assert
+        .dom(".schema-theme-setting-editor__tree-node--back-btn")
+        .doesNotExist();
 
       const tree = new TreeFromDOM();
       await click(tree.nodes[0].children[0].element);
 
-      assert.dom(".back-button").exists();
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").exists();
       tree.refresh();
-      assert.strictEqual(tree.nodes[0].text, "child 1-1");
+      assert.dom(tree.nodes[0].textElement).hasText("child 1-1");
 
       await click(tree.nodes[0].children[0].element);
 
       tree.refresh();
-      assert.strictEqual(tree.nodes[0].text, "grandchild 1-1-1");
-      assert.dom(".back-button").exists();
+      assert.dom(tree.nodes[0].textElement).hasText("grandchild 1-1-1");
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").exists();
 
-      await click(".back-button");
+      await click(".schema-theme-setting-editor__tree-node--back-btn");
 
       tree.refresh();
-      assert.strictEqual(tree.nodes[0].text, "child 1-1");
-      assert.dom(".back-button").exists();
+      assert.dom(tree.nodes[0].textElement).hasText("child 1-1");
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").exists();
 
-      await click(".back-button");
+      await click(".schema-theme-setting-editor__tree-node--back-btn");
 
       tree.refresh();
-      assert.strictEqual(tree.nodes[0].text, "item 1");
-      assert.dom(".back-button").doesNotExist();
+      assert.dom(tree.nodes[0].textElement).hasText("item 1");
+      assert
+        .dom(".schema-theme-setting-editor__tree-node--back-btn")
+        .doesNotExist();
     });
 
     test("the back button navigates to the index of the active element at the previous level", async function (assert) {
@@ -243,16 +277,16 @@ module(
       tree.refresh();
       await click(tree.nodes[1].children[1].element);
 
-      await click(".back-button");
+      await click(".schema-theme-setting-editor__tree-node--back-btn");
       tree.refresh();
 
       assert.strictEqual(tree.nodes.length, 2);
 
-      assert.strictEqual(tree.nodes[0].text, "item 1");
+      assert.dom(tree.nodes[0].textElement).hasText("item 1");
       assert.false(tree.nodes[0].active);
       assert.strictEqual(tree.nodes[0].children.length, 0);
 
-      assert.strictEqual(tree.nodes[1].text, "item 2");
+      assert.dom(tree.nodes[1].textElement).hasText("item 2");
       assert.true(tree.nodes[1].active);
       assert.strictEqual(tree.nodes[1].children.length, 3);
     });
@@ -270,7 +304,7 @@ module(
       tree.refresh();
       await click(tree.nodes[1].children[1].element);
 
-      assert.dom(".back-button").hasText(
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
         I18n.t("admin.customize.theme.schema.back_button", {
           name: "item 2",
         })
@@ -279,15 +313,15 @@ module(
       tree.refresh();
       await click(tree.nodes[1].children[0].element);
 
-      assert.dom(".back-button").hasText(
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
         I18n.t("admin.customize.theme.schema.back_button", {
           name: "child 2-2",
         })
       );
 
-      await click(".back-button");
+      await click(".schema-theme-setting-editor__tree-node--back-btn");
 
-      assert.dom(".back-button").hasText(
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
         I18n.t("admin.customize.theme.schema.back_button", {
           name: "item 2",
         })
@@ -377,11 +411,12 @@ module(
       assert.dom(inputFields.fields.text.labelElement).hasText("text");
       assert.dom(inputFields.fields.url.labelElement).hasText("url");
       assert.dom(inputFields.fields.icon.labelElement).hasText("icon");
-
       assert.dom(inputFields.fields.text.inputElement).hasValue("About");
+
       assert
         .dom(inputFields.fields.url.inputElement)
         .hasValue("https://example.com/about");
+
       assert.dom(inputFields.fields.icon.inputElement).hasValue("asterisk");
     });
 
@@ -657,16 +692,16 @@ module(
 
       const tree = new TreeFromDOM();
 
-      assert.dom(tree.nodes[0].element).hasText("section 1");
-      assert.dom(tree.nodes[0].children[0].element).hasText("link 1");
-      assert.dom(tree.nodes[0].children[1].element).hasText("link 2");
-      assert.dom(tree.nodes[1].element).hasText("section 2");
+      assert.dom(tree.nodes[0].textElement).hasText("section 1");
+      assert.dom(tree.nodes[0].children[0].textElement).hasText("link 1");
+      assert.dom(tree.nodes[0].children[1].textElement).hasText("link 2");
+      assert.dom(tree.nodes[1].textElement).hasText("section 2");
 
       await click(tree.nodes[1].element);
 
       tree.refresh();
 
-      assert.dom(tree.nodes[1].children[0].element).hasText("link 1");
+      assert.dom(tree.nodes[1].children[0].textElement).hasText("link 1");
     });
 
     test("identifier field instantly updates in the navigation tree when the input field is changed", async function (assert) {
@@ -684,7 +719,9 @@ module(
         "nice section is really nice"
       );
 
-      assert.dom(tree.nodes[0].element).hasText("nice section is really nice");
+      assert
+        .dom(tree.nodes[0].textElement)
+        .hasText("nice section is really nice");
 
       await click(tree.nodes[0].children[0].element);
 
@@ -696,7 +733,9 @@ module(
         "Security instead of Privacy"
       );
 
-      assert.dom(tree.nodes[0].element).hasText("Security instead of Privacy");
+      assert
+        .dom(tree.nodes[0].textElement)
+        .hasText("Security instead of Privacy");
     });
 
     test("edits are remembered when navigating between levels", async function (assert) {
@@ -729,26 +768,26 @@ module(
       tree.refresh();
       inputFields.refresh();
 
-      assert.dom(".back-button").hasText(
+      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
         I18n.t("admin.customize.theme.schema.back_button", {
           name: "cool section is no longer cool",
         })
       );
 
       await fillIn(inputFields.fields.text.inputElement, "Talk to us");
-
-      await click(".back-button");
+      await click(".schema-theme-setting-editor__tree-node--back-btn");
 
       tree.refresh();
       inputFields.refresh();
 
-      assert.dom(tree.nodes[0].element).hasText("changed section name");
+      assert.dom(tree.nodes[0].textElement).hasText("changed section name");
+
       assert
-        .dom(tree.nodes[1].element)
+        .dom(tree.nodes[1].textElement)
         .hasText("cool section is no longer cool");
 
-      assert.dom(tree.nodes[1].children[0].element).hasText("About");
-      assert.dom(tree.nodes[1].children[1].element).hasText("Talk to us");
+      assert.dom(tree.nodes[1].children[0].textElement).hasText("About");
+      assert.dom(tree.nodes[1].children[1].textElement).hasText("Talk to us");
 
       assert
         .dom(inputFields.fields.name.inputElement)

app/assets/stylesheets/common/admin/admin_base.scss

@@ -1068,6 +1068,8 @@ a.inline-editable-field {
 @import "common/admin/admin_intro";
 @import "common/admin/admin_emojis";
 @import "common/admin/mini_profiler";
+@import "common/admin/schema_theme_setting_editor";
+@import "common/admin/customize_themes_show_schema";
 
 // EXPERIMENTAL: Revamped admin styles, probably can be split up later down the line.
 @import "common/admin/admin_revamp";

app/assets/stylesheets/common/admin/customize_themes_show_schema.scss

@@ -0,0 +1,3 @@
+.customize-themes-show-schema__header {
+  margin-bottom: 1em;
+}

app/assets/stylesheets/common/admin/schema_theme_setting_editor.scss

@@ -0,0 +1,76 @@
+.schema-theme-setting-editor {
+  display: grid;
+  grid-template-columns: 0.2fr 0.8fr;
+
+  &__navigation {
+    margin-right: 2em;
+
+    ul {
+      list-style: none;
+    }
+
+    .schema-theme-setting-editor__tree {
+      margin: 0;
+
+      .schema-theme-setting-editor__tree-node--back-btn {
+        cursor: pointer;
+        width: 100%;
+        text-align: left;
+
+        .schema-theme-setting-editor__tree-node-text {
+          .d-icon {
+            margin-left: 0;
+            margin-right: 0.5em;
+          }
+        }
+      }
+
+      .schema-theme-setting-editor__tree-node-text {
+        padding: 0.5em;
+        color: var(--primary);
+        display: flex;
+        flex-direction: row;
+        align-items: center;
+
+        .d-icon {
+          margin-left: auto;
+          font-size: var(--font-down-3);
+          color: var(--primary-500);
+        }
+      }
+
+      .schema-theme-setting-editor__tree-node {
+        cursor: pointer;
+
+        &.--active {
+          > .schema-theme-setting-editor__tree-node-text {
+            background-color: var(--tertiary);
+            color: var(--secondary);
+
+            .d-icon {
+              color: var(--secondary);
+            }
+          }
+        }
+
+        &.--child {
+          margin-left: 0.5em;
+          border-left: 1.5px solid var(--primary-200);
+        }
+
+        &.--heading {
+          cursor: default;
+          margin-top: 0.5em;
+          font-weight: bold;
+        }
+      }
+    }
+  }
+
+  &__fields {
+  }
+
+  &__footer {
+    margin-top: 0.5em;
+  }
+}

config/locales/client.en.yml

@@ -5643,6 +5643,7 @@ en:
           inactive_filter: "Inactive"
           updates_available_filter: "Updates Available"
           schema:
+            title: "Edit %{name} setting"
             back_button: "Back to %{name}"
         colors:
           select_base:

spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb

@@ -17,7 +17,11 @@ module PageObjects
       end
 
       def click_link(name)
-        find(".schema-editor-navigation .node", text: name).click
+        find(
+          ".schema-theme-setting-editor__navigation .schema-theme-setting-editor__tree-node",
+          text: name,
+        ).click
+
         self
       end