From c9d428821442df9003cf9bc5dd5afc4c876c7996 Mon Sep 17 00:00:00 2001 From: Kyle Zhao Date: Mon, 23 Jul 2018 20:25:40 -0400 Subject: [PATCH] UX: auto fix order when reordering categories (#6149) * set correct position number when moving up/down * UX: drop 'fix order' and auto re-order subcategory - auto "fix position" on save - place subcategories after parent category and maintain the relative positions on save --- .../controllers/reorder-categories.js.es6 | 67 ++++++++++++------- .../templates/modal/reorder-categories.hbs | 3 - config/locales/client.en.yml | 2 - .../reorder-categories-test.js.es6 | 50 ++++++++++++++ 4 files changed, 92 insertions(+), 30 deletions(-) create mode 100644 test/javascripts/controllers/reorder-categories-test.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/reorder-categories.js.es6 b/app/assets/javascripts/discourse/controllers/reorder-categories.js.es6 index a5a2bfefc0c..5f181438dcd 100644 --- a/app/assets/javascripts/discourse/controllers/reorder-categories.js.es6 +++ b/app/assets/javascripts/discourse/controllers/reorder-categories.js.es6 @@ -11,7 +11,7 @@ import Ember from "ember"; export default Ember.Controller.extend(ModalFunctionality, Ember.Evented, { @on("init") _fixOrder() { - this.send("fixIndices"); + this.fixIndices(); }, @computed("site.categories") @@ -26,17 +26,6 @@ export default Ember.Controller.extend(ModalFunctionality, Ember.Evented, { "categoriesSorting" ), - showFixIndices: function() { - const cats = this.get("categoriesOrdered"); - const len = cats.get("length"); - for (let i = 0; i < len; i++) { - if (cats.objectAt(i).get("position") !== i) { - return true; - } - } - return false; - }.property("categoriesOrdered.@each.position"), - showApplyAll: function() { let anyChanged = false; this.get("categoriesBuffered").forEach(bc => { @@ -45,21 +34,56 @@ export default Ember.Controller.extend(ModalFunctionality, Ember.Evented, { return anyChanged; }.property("categoriesBuffered.@each.hasBufferedChanges"), - saveDisabled: Ember.computed.or("showApplyAll", "showFixIndices"), + saveDisabled: Ember.computed.alias("showApplyAll"), moveDir(cat, dir) { const cats = this.get("categoriesOrdered"); const curIdx = cats.indexOf(cat); const desiredIdx = curIdx + dir; if (desiredIdx >= 0 && desiredIdx < cats.get("length")) { - const curPos = cat.get("position"); - cat.set("position", curPos + dir); const otherCat = cats.objectAt(desiredIdx); - otherCat.set("position", curPos - dir); + otherCat.set("position", curIdx); + cat.set("position", desiredIdx); this.send("commit"); } }, + /** + 1. Make sure all categories have unique position numbers. + 2. Place sub-categories after their parent categories while maintaining the + same relative order. + + e.g. + parent/c1 parent + parent => parent/c1 + other parent/c2 + parent/c2 other + **/ + fixIndices() { + const categories = this.get("categoriesOrdered"); + const subcategories = {}; + + categories.forEach(category => { + const parentCategoryId = category.get("parent_category_id"); + + if (parentCategoryId) { + subcategories[parentCategoryId] = subcategories[parentCategoryId] || []; + subcategories[parentCategoryId].push(category); + } + }); + + for (let i = 0, position = 0; i < categories.get("length"); ++i) { + const category = categories.objectAt(i); + + if (!category.get("parent_category_id")) { + category.set("position", position++); + (subcategories[category.get("id")] || []).forEach(subcategory => subcategory.set("position", position++)); + } + } + + this.send("commit"); + }, + actions: { moveUp(cat) { this.moveDir(cat, -1); @@ -68,15 +92,6 @@ export default Ember.Controller.extend(ModalFunctionality, Ember.Evented, { this.moveDir(cat, 1); }, - fixIndices() { - const cats = this.get("categoriesOrdered"); - const len = cats.get("length"); - for (let i = 0; i < len; i++) { - cats.objectAt(i).set("position", i); - } - this.send("commit"); - }, - commit() { this.get("categoriesBuffered").forEach(bc => { if (bc.get("hasBufferedChanges")) { @@ -87,6 +102,8 @@ export default Ember.Controller.extend(ModalFunctionality, Ember.Evented, { }, saveOrder() { + this.fixIndices(); + const data = {}; this.get("categoriesBuffered").forEach(cat => { data[cat.get("id")] = cat.get("position"); diff --git a/app/assets/javascripts/discourse/templates/modal/reorder-categories.hbs b/app/assets/javascripts/discourse/templates/modal/reorder-categories.hbs index 25228caf711..acf26834e57 100644 --- a/app/assets/javascripts/discourse/templates/modal/reorder-categories.hbs +++ b/app/assets/javascripts/discourse/templates/modal/reorder-categories.hbs @@ -23,9 +23,6 @@ {{/d-modal-body}}
- {{#if showFixIndices}} - {{d-button action="fixIndices" icon="random" label="categories.reorder.fix_order" title="categories.reorder.fix_order_tooltip"}} - {{/if}} {{#if showApplyAll}} {{d-button action="commit" icon="check" label="categories.reorder.apply_all"}} {{/if}} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ef98e3f26b4..544fb39c7c6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -556,8 +556,6 @@ en: reorder: title: "Reorder Categories" title_long: "Reorganize the category list" - fix_order: "Fix Positions" - fix_order_tooltip: "Not all categories have a unique position number, which may cause unexpected results." save: "Save Order" apply_all: "Apply" position: "Position" diff --git a/test/javascripts/controllers/reorder-categories-test.js.es6 b/test/javascripts/controllers/reorder-categories-test.js.es6 new file mode 100644 index 00000000000..d7a2fe08fe7 --- /dev/null +++ b/test/javascripts/controllers/reorder-categories-test.js.es6 @@ -0,0 +1,50 @@ +import { mapRoutes } from "discourse/mapping-router"; +import createStore from "helpers/create-store"; + +moduleFor("controller:reorder-categories", "controller:reorder-categories", { + beforeEach() { + this.registry.register("router:main", mapRoutes()); + }, + needs: ["controller:modal"] +}); + +QUnit.test("fixIndices set unique position number", function(assert) { + const store = createStore(); + + const categories = []; + for (let i = 0; i < 3; ++i) { + categories.push(store.createRecord("category", { id: i, position: 0 })); + } + + const site = Ember.Object.create({ categories: categories }); + const reorderCategoriesController = this.subject({ site }); + + reorderCategoriesController.fixIndices(); + + reorderCategoriesController.get("categoriesOrdered").forEach((category, index) => { + assert.equal(category.get("position"), index); + }); +}); + +QUnit.test( + "fixIndices places subcategories after their parent categories, while maintaining the relative order", + function(assert) { + const store = createStore(); + + const parent = store.createRecord("category", { id: 1, position: 1, slug: "parent" }); + const child1 = store.createRecord("category", { id: 2, position: 3, slug: "child1", parent_category_id: 1 }); + const child2 = store.createRecord("category", { id: 3, position: 0, slug: "child2", parent_category_id: 1 }); + const other = store.createRecord("category", { id: 4, position: 2, slug: "other" }); + + const categories = [child2, parent, other, child1]; + const expectedOrderSlugs = ["parent", "child2", "child1", "other"]; + + const site = Ember.Object.create({ categories: categories }); + const reorderCategoriesController = this.subject({ site }); + + reorderCategoriesController.fixIndices(); + + assert.deepEqual(reorderCategoriesController.get("categoriesOrdered").mapBy("slug"), expectedOrderSlugs); + } +); +