DEV: Migrate reorder-categories to the new modal API (#24209)

app/assets/javascripts/discourse/app/components/discovery/navigation.js

@@ -1,8 +1,8 @@
 import Component from "@glimmer/component";
 import { action } from "@ember/object";
 import { inject as service } from "@ember/service";
+import ReorderCategories from "discourse/components/modal/reorder-categories";
 import { calculateFilterMode } from "discourse/lib/filter-mode";
-import showModal from "discourse/lib/show-modal";
 import { TRACKED_QUERY_PARAM_VALUE } from "discourse/lib/topic-list-tracked-filter";
 import DiscourseURL from "discourse/lib/url";
 import Category from "discourse/models/category";
@@ -56,6 +56,6 @@ export default class DiscoveryNavigation extends Component {
 
   @action
   reorderCategories() {
-    showModal("reorder-categories");
+    this.modal.show(ReorderCategories);
   }
 }

app/assets/javascripts/discourse/app/components/modal/reorder-categories.hbs

@@ -0,0 +1,53 @@
+<DModal
+  @title={{i18n "categories.reorder.title"}}
+  @closeModal={{@closeModal}}
+  class="reorder-categories"
+>
+  <:body>
+    <table>
+      <thead>
+        <th>{{i18n "categories.category"}}</th>
+        <th>{{i18n "categories.reorder.position"}}</th>
+      </thead>
+      <tbody>
+        {{#each this.categoriesOrdered as |category|}}
+          <tr data-category-id={{category.id}}>
+            <td>
+              <div class={{concat "reorder-categories-depth-" category.depth}}>
+                {{category-badge category allowUncategorized="true"}}
+              </div>
+            </td>
+
+            <td>
+              <div class="reorder-categories-actions">
+                <NumberField
+                  @number={{readonly category.position}}
+                  @change={{fn this.change category}}
+                  @min="0"
+                />
+                <DButton
+                  @action={{fn this.move category -1}}
+                  @icon="arrow-up"
+                  class="btn-default no-text"
+                />
+                <DButton
+                  @action={{fn this.move category 1}}
+                  @icon="arrow-down"
+                  class="btn-default no-text"
+                />
+              </div>
+            </td>
+          </tr>
+        {{/each}}
+      </tbody>
+    </table>
+  </:body>
+
+  <:footer>
+    <DButton
+      @action={{this.save}}
+      @label="categories.reorder.save"
+      class="btn-primary"
+    />
+  </:footer>
+</DModal>

app/assets/javascripts/discourse/app/components/modal/reorder-categories.js

@@ -1,24 +1,22 @@
-import Controller from "@ember/controller";
+import Component from "@ember/component";
+import { action } from "@ember/object";
 import { sort } from "@ember/object/computed";
-import Evented from "@ember/object/evented";
-import BufferedProxy from "ember-buffered-proxy/proxy";
+import { next } from "@ember/runloop";
+import { inject as service } from "@ember/service";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
-import ModalFunctionality from "discourse/mixins/modal-functionality";
-import discourseComputed, { on } from "discourse-common/utils/decorators";
 
-export default Controller.extend(ModalFunctionality, Evented, {
+export default class ReorderCategories extends Component {
+  @service site;
+
+  categoriesSorting = ["position"];
+
+  @sort("site.categories", "categoriesSorting") categoriesOrdered;
+
   init() {
-    this._super(...arguments);
-    this.categoriesSorting = ["position"];
-  },
-
-  @discourseComputed("site.categories.[]")
-  categoriesBuffered(categories) {
-    return (categories || []).map((c) => BufferedProxy.create({ content: c }));
-  },
-
-  categoriesOrdered: sort("categoriesBuffered", "categoriesSorting"),
+    super.init(...arguments);
+    next(() => this.reorder());
+  }
 
   /**
    * 1. Make sure all categories have unique position numbers.
@@ -31,46 +29,39 @@ export default Controller.extend(ModalFunctionality, Evented, {
    *      parent          =>    parent/c2
    *      other                 parent/c2/c1
    *      parent/c2             other
-   *
    **/
-  @on("init")
   reorder() {
-    const reorderChildren = (categoryId, depth, index) => {
-      this.categoriesOrdered.forEach((category) => {
-        if (
-          (categoryId === null && !category.get("parent_category_id")) ||
-          category.get("parent_category_id") === categoryId
-        ) {
-          category.setProperties({ depth, position: index++ });
-          index = reorderChildren(category.get("id"), depth + 1, index);
-        }
-      });
+    this.reorderChildren(null, 0, 0);
+  }
 
-      return index;
-    };
-
-    reorderChildren(null, 0, 0);
-
-    this.categoriesBuffered.forEach((bc) => {
-      if (bc.get("hasBufferedChanges")) {
-        bc.applyBufferedChanges();
+  reorderChildren(categoryId, depth, index) {
+    for (const category of this.categoriesOrdered) {
+      if (
+        (categoryId === null && !category.get("parent_category_id")) ||
+        category.get("parent_category_id") === categoryId
+      ) {
+        category.setProperties({ depth, position: index++ });
+        index = this.reorderChildren(category.get("id"), depth + 1, index);
       }
-    });
+    }
 
-    this.notifyPropertyChange("categoriesBuffered");
-  },
+    return index;
+  }
 
   countDescendants(category) {
-    return category.get("subcategories")
-      ? category
-          .get("subcategories")
-          .reduce(
-            (count, subcategory) => count + this.countDescendants(subcategory),
-            category.get("subcategories").length
-          )
-      : 0;
-  },
+    if (!category.get("subcategories")) {
+      return 0;
+    }
 
+    return category
+      .get("subcategories")
+      .reduce(
+        (count, subcategory) => count + this.countDescendants(subcategory),
+        category.get("subcategories").length
+      );
+  }
+
+  @action
   move(category, direction) {
     let targetPosition = category.get("position") + direction;
 
@@ -114,7 +105,7 @@ export default Controller.extend(ModalFunctionality, Evented, {
     }
 
     // Update other categories between current and target position
-    this.categoriesOrdered.map((c) => {
+    for (const c of this.categoriesOrdered) {
       if (direction < 0) {
         // Moving up (position gets smaller)
         if (
@@ -134,47 +125,42 @@ export default Controller.extend(ModalFunctionality, Evented, {
           c.set("position", newPosition);
         }
       }
-    });
+    }
 
     // Update this category's position to target position
     category.set("position", targetPosition);
 
     this.reorder();
-  },
+  }
 
-  actions: {
-    change(category, event) {
-      let newPosition = parseFloat(event.target.value);
-      newPosition =
-        newPosition < category.get("position")
-          ? Math.ceil(newPosition)
-          : Math.floor(newPosition);
-      const direction = newPosition - category.get("position");
-      this.move(category, direction);
-    },
+  @action
+  change(category, event) {
+    let newPosition = parseFloat(event.target.value);
+    newPosition =
+      newPosition < category.get("position")
+        ? Math.ceil(newPosition)
+        : Math.floor(newPosition);
+    const direction = newPosition - category.get("position");
+    this.move(category, direction);
+  }
 
-    moveUp(category) {
-      this.move(category, -1);
-    },
+  @action
+  async save() {
+    this.reorder();
 
-    moveDown(category) {
-      this.move(category, 1);
-    },
+    const data = {};
+    for (const category of this.site.categories) {
+      data[category.get("id")] = category.get("position");
+    }
 
-    save() {
-      this.reorder();
-
-      const data = {};
-      this.categoriesBuffered.forEach((cat) => {
-        data[cat.get("id")] = cat.get("position");
-      });
-
-      ajax("/categories/reorder", {
+    try {
+      await ajax("/categories/reorder", {
         type: "POST",
         data: { mapping: JSON.stringify(data) },
-      })
-        .then(() => window.location.reload())
-        .catch(popupAjaxError);
-    },
-  },
-});
+      });
+      window.location.reload();
+    } catch (e) {
+      popupAjaxError(e);
+    }
+  }
+}

app/assets/javascripts/discourse/app/routes/discovery-categories.js

@@ -10,6 +10,7 @@ import DiscourseRoute from "discourse/routes/discourse";
 import I18n from "discourse-i18n";
 
 export default class DiscoveryCategoriesRoute extends DiscourseRoute {
+  @service modal;
   @service router;
   @service session;
 

app/assets/javascripts/discourse/app/services/modal.js

@@ -11,20 +11,6 @@ import deprecated, {
 } from "discourse-common/lib/deprecated";
 import I18n from "discourse-i18n";
 
-// Known legacy modals in core. Silence deprecation warnings for these so the messages
-// don't cause unnecessary noise.
-const KNOWN_LEGACY_MODALS = [
-  "avatar-selector",
-  "change-owner",
-  "change-post-notice",
-  "create-invite-bulk",
-  "create-invite",
-  "grant-badge",
-  "group-default-notifications",
-  "reject-reason-reviewable",
-  "reorder-categories",
-];
-
 const LEGACY_OPTS = new Set([
   "admin",
   "templateName",
@@ -139,17 +125,15 @@ export default class ModalServiceWithLegacySupport extends ModalService {
 
     this.close({ initiatedBy: CLOSE_INITIATED_BY_MODAL_SHOW });
 
-    if (!KNOWN_LEGACY_MODALS.includes(modal)) {
-      deprecated(
-        `Defining modals using a controller is deprecated. Use the component-based API instead. (modal: ${modal})`,
-        {
-          id: "discourse.modal-controllers",
-          since: "3.1",
-          dropFrom: "3.2",
-          url: "https://meta.discourse.org/t/268057",
-        }
-      );
-    }
+    deprecated(
+      `Defining modals using a controller is deprecated. Use the component-based API instead. (modal: ${modal})`,
+      {
+        id: "discourse.modal-controllers",
+        since: "3.1",
+        dropFrom: "3.2",
+        url: "https://meta.discourse.org/t/268057",
+      }
+    );
 
     const name = modal;
     const container = getOwner(this);

app/assets/javascripts/discourse/app/templates/modal/reorder-categories.hbs

@@ -1,50 +0,0 @@
-<DModalBody
-  @class="reorder-categories full-height-modal"
-  @title="categories.reorder.title"
->
-  <div id="rc-scroll-anchor"></div>
-  <table>
-    <thead>
-      <th class="th-cat">{{i18n "categories.category"}}</th>
-      <th class="th-pos">{{i18n "categories.reorder.position"}}</th>
-    </thead>
-    <tbody>
-      {{#each this.categoriesOrdered as |cat|}}
-        <tr data-category-id={{cat.id}}>
-          <td>
-            <div class={{concat "reorder-categories-depth-" cat.depth}}>
-              {{category-badge cat allowUncategorized="true"}}
-            </div>
-          </td>
-
-          <td>
-            <NumberField
-              @number={{readonly cat.position}}
-              @change={{action "change" cat}}
-              @min="0"
-            />
-            <DButton
-              @action={{fn (action "moveUp") cat}}
-              @icon="arrow-up"
-              class="btn-default no-text"
-            />
-            <DButton
-              @action={{fn (action "moveDown") cat}}
-              @icon="arrow-down"
-              class="btn-default no-text"
-            />
-          </td>
-        </tr>
-      {{/each}}
-    </tbody>
-  </table>
-  <div id="rc-scroll-bottom"></div>
-</DModalBody>
-
-<div class="modal-footer">
-  <DButton
-    @action={{action "save"}}
-    @label="categories.reorder.save"
-    class="btn-primary"
-  />
-</div>

app/assets/javascripts/discourse/tests/unit/components/reorder-categories-test.js

@@ -2,11 +2,13 @@ import { getOwner } from "@ember/application";
 import { setupTest } from "ember-qunit";
 import { module, test } from "qunit";
 
-module("Unit | Controller | reorder-categories", function (hooks) {
+module("Unit | Component | reorder-categories", function (hooks) {
   setupTest(hooks);
 
   test("reorder set unique position number", function (assert) {
-    const controller = getOwner(this).lookup("controller:reorder-categories");
+    const component = this.owner
+      .factoryFor("component:modal/reorder-categories")
+      .create();
     const store = getOwner(this).lookup("service:store");
 
     const site = getOwner(this).lookup("service:site");
@@ -16,15 +18,17 @@ module("Unit | Controller | reorder-categories", function (hooks) {
       store.createRecord("category", { id: 3, position: 0 }),
     ]);
 
-    controller.reorder();
+    component.reorder();
 
-    controller.categoriesOrdered.forEach((category, index) => {
+    component.categoriesOrdered.forEach((category, index) => {
       assert.strictEqual(category.get("position"), index);
     });
   });
 
   test("reorder places subcategories after their parent categories, while maintaining the relative order", function (assert) {
-    const controller = getOwner(this).lookup("controller:reorder-categories");
+    const component = this.owner
+      .factoryFor("component:modal/reorder-categories")
+      .create();
     const store = getOwner(this).lookup("service:store");
 
     const parent = store.createRecord("category", {
@@ -54,16 +58,18 @@ module("Unit | Controller | reorder-categories", function (hooks) {
     const site = getOwner(this).lookup("service:site");
     site.set("categories", [child2, parent, other, child1]);
 
-    controller.reorder();
+    component.reorder();
 
     assert.deepEqual(
-      controller.categoriesOrdered.mapBy("slug"),
+      component.categoriesOrdered.mapBy("slug"),
       expectedOrderSlugs
     );
   });
 
   test("changing the position number of a category should place it at given position", function (assert) {
-    const controller = getOwner(this).lookup("controller:reorder-categories");
+    const component = this.owner
+      .factoryFor("component:modal/reorder-categories")
+      .create();
     const store = getOwner(this).lookup("service:store");
 
     const elem1 = store.createRecord("category", {
@@ -88,9 +94,9 @@ module("Unit | Controller | reorder-categories", function (hooks) {
     site.set("categories", [elem1, elem2, elem3]);
 
     // Move category 'foo' from position 0 to position 2
-    controller.send("change", elem1, { target: { value: "2" } });
+    component.change(elem1, { target: { value: "2" } });
 
-    assert.deepEqual(controller.categoriesOrdered.mapBy("slug"), [
+    assert.deepEqual(component.categoriesOrdered.mapBy("slug"), [
       "bar",
       "test",
       "foo",
@@ -98,7 +104,9 @@ module("Unit | Controller | reorder-categories", function (hooks) {
   });
 
   test("changing the position number of a category should place it at given position and respect children", function (assert) {
-    const controller = getOwner(this).lookup("controller:reorder-categories");
+    const component = this.owner
+      .factoryFor("component:modal/reorder-categories")
+      .create();
     const store = getOwner(this).lookup("service:store");
 
     const elem1 = store.createRecord("category", {
@@ -129,9 +137,9 @@ module("Unit | Controller | reorder-categories", function (hooks) {
     const site = getOwner(this).lookup("service:site");
     site.set("categories", [elem1, child1, elem2, elem3]);
 
-    controller.send("change", elem1, { target: { value: 3 } });
+    component.change(elem1, { target: { value: 3 } });
 
-    assert.deepEqual(controller.categoriesOrdered.mapBy("slug"), [
+    assert.deepEqual(component.categoriesOrdered.mapBy("slug"), [
       "bar",
       "test",
       "foo",
@@ -140,7 +148,9 @@ module("Unit | Controller | reorder-categories", function (hooks) {
   });
 
   test("changing the position through click on arrow of a category should place it at given position and respect children", function (assert) {
-    const controller = getOwner(this).lookup("controller:reorder-categories");
+    const component = this.owner
+      .factoryFor("component:modal/reorder-categories")
+      .create();
     const store = getOwner(this).lookup("service:store");
 
     const child2 = store.createRecord("category", {
@@ -180,11 +190,11 @@ module("Unit | Controller | reorder-categories", function (hooks) {
     const site = getOwner(this).lookup("service:site");
     site.set("categories", [elem1, child1, child2, elem2, elem3]);
 
-    controller.reorder();
+    component.reorder();
 
-    controller.send("moveDown", elem1);
+    component.move(elem1, 1);
 
-    assert.deepEqual(controller.categoriesOrdered.mapBy("slug"), [
+    assert.deepEqual(component.categoriesOrdered.mapBy("slug"), [
       "bar",
       "foo",
       "foo-child",

app/assets/stylesheets/common/base/_index.scss

@@ -3,7 +3,7 @@
 @import "activation";
 @import "alert";
 @import "bbcode";
-@import "cat_reorder";
+@import "reorder-categories";
 @import "category-list";
 @import "code_highlighting";
 @import "colorpicker";

app/assets/stylesheets/common/base/reorder-categories.scss

@@ -5,16 +5,15 @@
     }
   }
   input[type="text"] {
+    margin: 0;
     max-width: 2.5em;
     padding: 0.35em;
     text-align: center;
+
     @include breakpoint(mobile-extra-large) {
       width: 2em;
     }
   }
-  #rc-scroll-anchor {
-    padding: 0;
-  }
   table {
     padding-bottom: 150px;
     margin: 0 0.667em;
@@ -34,6 +33,11 @@
   }
 }
 
+.reorder-categories-actions {
+  display: flex;
+  gap: 0.5rem;
+}
+
 .reorder-categories-depth-1 {
   margin-left: 20px;
 }