UX: adjust objects editor styles for full page layout (#26265)

2024-03-20 15:55:52 -04:00 · 2024-03-20 15:55:52 -04:00 · 59217b8a18
parent 3417d15730
commit 59217b8a18
7 changed files with 178 additions and 131 deletions
--- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs
+++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs
@ -3,13 +3,11 @@ import { cached, tracked } from "@glimmer/tracking";
 import { fn } from "@ember/helper";
 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";
 import FieldInput from "./field";
@ -284,17 +282,18 @@ export default class SchemaThemeSettingEditor extends Component {
  }

  <template>
+    {{! template-lint-disable no-nested-interactive }}
    <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="schema-theme-setting-editor__tree-node--back-btn"
+              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"}}
+                {{dIcon "arrow-left"}}
                {{this.backButtonText}}
              </div>
            </li>
@ -308,59 +307,59 @@ export default class SchemaThemeSettingEditor extends Component {
              {{on "click" (fn this.onClick node)}}
            >
              <div class="schema-theme-setting-editor__tree-node-text">
-                {{node.text}}
-
-                {{#if (gt node.trees.length 0)}}
+                <span>{{node.text}}</span>
+                {{#if node.parentTree.propertyName}}
+                  {{dIcon "chevron-right"}}
+                {{else}}
                  {{dIcon (if node.active "chevron-down" "chevron-right")}}
                {{/if}}
              </div>
-            </li>

-            {{#each node.trees as |nestedTree|}}
-              {{#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 node.trees as |nestedTree|}}
+                <ul>
+                  {{#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">
+                        <span>{{childNode.text}}</span>
+                        {{dIcon "chevron-right"}}
+                      </div>
+                    </li>
+                  {{/each}}
+                  <li
+                    class="schema-theme-setting-editor__tree-node --child --add-button"
+                  >
+                    <DButton
+                      @action={{fn this.addItem nestedTree}}
+                      @translatedLabel={{nestedTree.schema.name}}
+                      @icon="plus"
+                      class="btn-transparent schema-theme-setting-editor__tree-add-button --child"
+                      data-test-parent-index={{node.index}}
+                    />
+                  </li>
+                </ul>
              {{/each}}
-
-              <DButton
-                @action={{fn this.addItem nestedTree}}
-                @translatedLabel={{nestedTree.schema.name}}
-                @icon="plus"
-                class="schema-theme-setting-editor__tree-add-button --child"
-                data-test-parent-index={{node.index}}
-              />
-            {{/each}}
+            </li>
          {{/each}}
+
+          <li
+            class="schema-theme-setting-editor__tree-node --parent --add-button"
+          >
+            <DButton
+              @action={{fn this.addItem this.tree}}
+              @translatedLabel={{this.tree.schema.name}}
+              @icon="plus"
+              class="btn-transparent schema-theme-setting-editor__tree-add-button --root"
+            />
+          </li>
        </ul>
-        <DButton
-          @action={{fn this.addItem this.tree}}
-          @translatedLabel={{this.tree.schema.name}}
-          @icon="plus"
-          class="schema-theme-setting-editor__tree-add-button --root"
-        />
      </div>

      <div class="schema-theme-setting-editor__fields">
@ -389,14 +388,6 @@ export default class SchemaThemeSettingEditor extends Component {
          @label="save"
          class="btn-primary"
        />
-
-        <LinkTo
-          @route="adminCustomizeThemes.show"
-          @model={{@themeId}}
-          class="btn-transparent"
-        >
-          {{i18n "cancel"}}
-        </LinkTo>
      </div>
    </div>
  </template>
--- a/app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs
+++ b/app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs
@ -1,4 +1,11 @@
 <div class="customize-themes-show-schema__header row">
+  <LinkTo
+    @route="adminCustomizeThemes.show"
+    @model={{@model.theme.id}}
+    class="btn-transparent customize-themes-show-schema__back"
+  >
+    {{d-icon "arrow-left"}}{{@model.theme.name}}
+  </LinkTo>
  <h2>
    {{i18n
      "admin.customize.theme.schema.title"
--- a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs
+++ b/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 { query, queryAll } from "discourse/tests/helpers/qunit-helpers";
+import { 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";
@ -36,9 +36,6 @@ class TreeFromDOM {
        };
      });

-      const childrenHeaderElement = query(
-        `.schema-theme-setting-editor__tree-node.--heading[data-test-parent-index="${index}"]`
-      );
      const addButtons = [
        ...queryAll(
          `.schema-theme-setting-editor__tree-add-button.--child[data-test-parent-index="${index}"]`
@ -48,7 +45,6 @@ class TreeFromDOM {
      return {
        active,
        children,
-        childrenHeaderElement,
        addButtons,
        element: li,
        textElement: li.querySelector(
@ -96,7 +92,7 @@ module(

      const tree = new TreeFromDOM();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);
      assert.true(tree.nodes[0].active, "the first node is active");
      assert.false(tree.nodes[1].active, "other nodes are not active");
    });
@ -111,7 +107,6 @@ 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,
@ -154,7 +149,7 @@ module(

      const tree = new TreeFromDOM();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);
      assert.dom(tree.nodes[0].textElement).hasText("item 1");
      assert.strictEqual(tree.nodes[0].children.length, 2);
      assert.dom(tree.nodes[0].children[0].textElement).hasText("child 1-1");
@ -167,7 +162,7 @@ module(

      tree.refresh();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);
      assert.dom(tree.nodes[0].textElement).hasText("item 1");
      assert.false(tree.nodes[0].active);
      assert.strictEqual(tree.nodes[0].children.length, 0);
@ -182,7 +177,7 @@ module(
      await click(tree.nodes[1].children[1].element);

      tree.refresh();
-      assert.strictEqual(tree.nodes.length, 3);
+      assert.strictEqual(tree.nodes.length, 4);

      assert.dom(tree.nodes[0].textElement).hasText("child 2-1");
      assert.false(tree.nodes[0].active);
@ -216,7 +211,7 @@ module(

      tree.refresh();

-      assert.strictEqual(tree.nodes.length, 4);
+      assert.strictEqual(tree.nodes.length, 5);

      assert.dom(tree.nodes[0].textElement).hasText("grandchild 2-2-1");
      assert.false(tree.nodes[0].active);
@ -242,14 +237,12 @@ module(
        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
      </template>);

-      assert
-        .dom(".schema-theme-setting-editor__tree-node--back-btn")
-        .doesNotExist();
+      assert.dom(".--back-btn").doesNotExist();

      const tree = new TreeFromDOM();
      await click(tree.nodes[0].children[0].element);

-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").exists();
+      assert.dom(".--back-btn").exists();
      tree.refresh();
      assert.dom(tree.nodes[0].textElement).hasText("child 1-1");

@ -257,21 +250,19 @@ module(

      tree.refresh();
      assert.dom(tree.nodes[0].textElement).hasText("grandchild 1-1-1");
-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").exists();
+      assert.dom(".--back-btn").exists();

-      await click(".schema-theme-setting-editor__tree-node--back-btn");
+      await click(".--back-btn");

      tree.refresh();
      assert.dom(tree.nodes[0].textElement).hasText("child 1-1");
-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").exists();
+      assert.dom(".--back-btn").exists();

-      await click(".schema-theme-setting-editor__tree-node--back-btn");
+      await click(".--back-btn");

      tree.refresh();
      assert.dom(tree.nodes[0].textElement).hasText("item 1");
-      assert
-        .dom(".schema-theme-setting-editor__tree-node--back-btn")
-        .doesNotExist();
+      assert.dom(".--back-btn").doesNotExist();
    });

    test("the back button navigates to the index of the active element at the previous level", async function (assert) {
@ -287,10 +278,10 @@ module(
      tree.refresh();
      await click(tree.nodes[1].children[1].element);

-      await click(".schema-theme-setting-editor__tree-node--back-btn");
+      await click(".--back-btn");
      tree.refresh();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);

      assert.dom(tree.nodes[0].textElement).hasText("item 1");
      assert.false(tree.nodes[0].active);
@ -314,7 +305,7 @@ module(
      tree.refresh();
      await click(tree.nodes[1].children[1].element);

-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
+      assert.dom(".--back-btn").hasText(
        I18n.t("admin.customize.theme.schema.back_button", {
          name: "item 2",
        })
@ -323,15 +314,15 @@ module(
      tree.refresh();
      await click(tree.nodes[1].children[0].element);

-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
+      assert.dom(".--back-btn").hasText(
        I18n.t("admin.customize.theme.schema.back_button", {
          name: "child 2-2",
        })
      );

-      await click(".schema-theme-setting-editor__tree-node--back-btn");
+      await click(".--back-btn");

-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
+      assert.dom(".--back-btn").hasText(
        I18n.t("admin.customize.theme.schema.back_button", {
          name: "item 2",
        })
@ -421,13 +412,13 @@ 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.text.inputElement).hasValue("Contact");

      assert
        .dom(inputFields.fields.url.inputElement)
-        .hasValue("https://example.com/about");
+        .hasValue("https://example.com/contact");

-      assert.dom(inputFields.fields.icon.inputElement).hasValue("asterisk");
+      assert.dom(inputFields.fields.icon.inputElement).hasValue("phone");
    });

    test("input fields of type integer", async function (assert) {
@ -778,14 +769,14 @@ module(
      tree.refresh();
      inputFields.refresh();

-      assert.dom(".schema-theme-setting-editor__tree-node--back-btn").hasText(
+      assert.dom(".--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(".schema-theme-setting-editor__tree-node--back-btn");
+      await click(".--back-btn");

      tree.refresh();
      inputFields.refresh();
@ -822,13 +813,13 @@ module(

      const tree = new TreeFromDOM();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);

      await click(TOP_LEVEL_ADD_BTN);
      await click(TOP_LEVEL_ADD_BTN);
      tree.refresh();

-      assert.strictEqual(tree.nodes.length, 4);
+      assert.strictEqual(tree.nodes.length, 5);
      assert.dom(tree.nodes[2].textElement).hasText("level1 3");
      assert.dom(tree.nodes[3].textElement).hasText("level1 4");
    });
@ -869,13 +860,13 @@ module(
      tree.refresh();

      assert.dom(TOP_LEVEL_ADD_BTN).hasText("level2");
-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);

      await click(TOP_LEVEL_ADD_BTN);
      await click(TOP_LEVEL_ADD_BTN);

      tree.refresh();
-      assert.strictEqual(tree.nodes.length, 4);
+      assert.strictEqual(tree.nodes.length, 5);
      assert.dom(tree.nodes[2].textElement).hasText("level2 3");
      assert.dom(tree.nodes[3].textElement).hasText("level2 4");
    });
@ -916,7 +907,7 @@ module(
      const tree = new TreeFromDOM();
      const inputFields = new InputFieldsFromDOM();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);
      assert.dom(tree.nodes[0].textElement).hasText("item 1");
      assert.dom(tree.nodes[1].textElement).hasText("item 2");
      assert.dom(inputFields.fields.name.inputElement).hasValue("item 1");
@ -926,7 +917,7 @@ module(
      tree.refresh();
      inputFields.refresh();

-      assert.strictEqual(tree.nodes.length, 1);
+      assert.strictEqual(tree.nodes.length, 2);
      assert.dom(tree.nodes[0].textElement).hasText("item 2");
      assert.dom(inputFields.fields.name.inputElement).hasValue("item 2");

@ -935,7 +926,7 @@ module(
      tree.refresh();
      inputFields.refresh();

-      assert.strictEqual(tree.nodes.length, 0);
+      assert.strictEqual(tree.nodes.length, 1);
      assert.strictEqual(inputFields.count, 0);
      assert.dom(REMOVE_ITEM_BTN).doesNotExist();
      assert.dom(TOP_LEVEL_ADD_BTN).hasText("level1");
@ -955,19 +946,19 @@ module(

      const inputFields = new InputFieldsFromDOM();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);
      assert.dom(tree.nodes[0].textElement).hasText("child 1-1");
      assert.dom(tree.nodes[1].textElement).hasText("child 1-2");
-      assert.dom(inputFields.fields.name.inputElement).hasValue("child 1-2");
+      assert.dom(inputFields.fields.name.inputElement).hasValue("child 1-1");

      await click(REMOVE_ITEM_BTN);

      tree.refresh();
      inputFields.refresh();

-      assert.strictEqual(tree.nodes.length, 1);
-      assert.dom(tree.nodes[0].textElement).hasText("child 1-1");
-      assert.dom(inputFields.fields.name.inputElement).hasValue("child 1-1");
+      assert.strictEqual(tree.nodes.length, 2);
+      assert.dom(tree.nodes[0].textElement).hasText("child 1-2");
+      assert.dom(inputFields.fields.name.inputElement).hasValue("child 1-2");

      // removing the last object navigates back to the previous level
      await click(REMOVE_ITEM_BTN);
@ -975,15 +966,13 @@ module(
      tree.refresh();
      inputFields.refresh();

-      assert.strictEqual(tree.nodes.length, 2);
+      assert.strictEqual(tree.nodes.length, 3);
      assert.strictEqual(tree.nodes[0].children.length, 0);

      assert.dom(tree.nodes[0].textElement).hasText("item 1");
      assert.dom(tree.nodes[1].textElement).hasText("item 2");
      assert.dom(inputFields.fields.name.inputElement).hasValue("item 1");
-      assert
-        .dom(".schema-theme-setting-editor__tree-node--back-btn")
-        .doesNotExist();
+      assert.dom(".--back-btn").doesNotExist();
    });
  }
 );
--- a/app/assets/stylesheets/common/admin/customize_themes_show_schema.scss
+++ b/app/assets/stylesheets/common/admin/customize_themes_show_schema.scss
@ -1,3 +1,16 @@
-.customize-themes-show-schema__header {
-  margin-bottom: 1em;
+.customize-themes-show-schema {
+  &__header {
+    margin-bottom: 1em;
+  }
+
+  &__back {
+    display: flex;
+    align-items: center;
+    gap: 0.25em;
+    color: var(--tertiary);
+    .d-icon {
+      font-size: var(--font-down-1);
+      color: currentColor;
+    }
+  }
 }
--- a/app/assets/stylesheets/common/admin/schema_field.scss
+++ b/app/assets/stylesheets/common/admin/schema_field.scss
@ -4,6 +4,7 @@
  .schema-field__input {
    input {
      margin-bottom: 0;
+      width: 100%;
    }

    margin-bottom: 0.3em;
--- a/app/assets/stylesheets/common/admin/schema_theme_setting_editor.scss
+++ b/app/assets/stylesheets/common/admin/schema_theme_setting_editor.scss
@ -1,9 +1,12 @@
 .schema-theme-setting-editor {
+  --schema-space: 0.5em;
  display: grid;
-  grid-template-columns: 0.2fr 0.8fr;
+  grid-template-columns: minmax(10em, 0.3fr) 1fr;
+  gap: 2em 4vw;

  &__navigation {
-    margin-right: 2em;
+    overflow: hidden;
+    border: 1px solid var(--primary-low);

    ul {
      list-style: none;
@ -11,27 +14,47 @@

    .schema-theme-setting-editor__tree {
      margin: 0;
+      ul {
+        padding: 0;
+        margin: var(--schema-space) var(--schema-space) 1em 0;
+        li:hover {
+          background: var(--primary-very-low);
+        }
+        &:last-child {
+          margin-bottom: calc(var(--schema-space) * 2);
+        }
+      }

-      .schema-theme-setting-editor__tree-node--back-btn {
+      .schema-theme-setting-editor__tree-node.--back-btn {
        cursor: pointer;
        width: 100%;
-        text-align: left;
+        border-bottom: 1px solid var(--primary-low);
+        color: var(--primary-700);
+        &:hover {
+          color: var(--primary-800);
+          background: var(--primary-very-low);
+        }

        .schema-theme-setting-editor__tree-node-text {
+          color: currentColor;
          .d-icon {
+            color: currentColor;
            margin-left: 0;
-            margin-right: 0.5em;
+            margin-right: var(--schema-space);
          }
        }
      }

      .schema-theme-setting-editor__tree-node-text {
-        padding: 0.5em;
+        padding: var(--schema-space);
        color: var(--primary);
        display: flex;
-        flex-direction: row;
        align-items: center;

+        span {
+          @include ellipsis;
+        }
+
        .d-icon {
          margin-left: auto;
          font-size: var(--font-down-3);
@ -53,30 +76,53 @@
          }
        }

-        &.--child {
-          margin-left: 0.5em;
-          border-left: 1.5px solid var(--primary-200);
+        &.--parent {
+          border: 2px solid transparent;
+
+          &:not(:has(ul)) {
+            &:hover {
+              background: var(--primary-very-low);
+            }
+          }
+
+          &.--active {
+            border: 2px solid var(--tertiary);
+          }
+
+          &.--add-button {
+            border-top: 1px solid var(--primary-low);
+          }
        }

-        &.--heading {
-          cursor: default;
-          margin-top: 0.5em;
-          font-weight: bold;
+        &.--child {
+          margin-left: var(--schema-space);
+          border-left: 1px solid var(--primary-200);
        }
      }
+    }

-      .schema-theme-setting-editor__tree-add-button {
-        &.--child {
-          margin-left: 0.5em;
+    .schema-theme-setting-editor__tree-add-button {
+      color: var(--tertiary);
+      width: 100%;
+      line-height: 1.4; // match li height
+      justify-content: start;
+
+      .d-icon {
+        color: currentColor;
+        font-size: var(--font-down-1);
+      }
+
+      &:hover {
+        background: var(--primary-very-low);
+        color: var(--tertiary-hover);
+        .d-icon {
+          color: currentColor;
        }
      }
    }
  }

  &__fields {
-  }
-
-  &__footer {
-    margin-top: 0.5em;
+    margin-top: -0.15em; // visual alignment with top of nav
  }
 }
--- a/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb
+++ b/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb
@ -18,7 +18,7 @@ module PageObjects

      def click_link(name)
        find(
-          ".schema-theme-setting-editor__navigation .schema-theme-setting-editor__tree-node",
+          ".schema-theme-setting-editor__navigation .schema-theme-setting-editor__tree-node.--child",
          text: name,
        ).click