Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

UX: Improvements for reordering categories (#13013) 

* UX: Improvements to reorder categories UX

Before, moving a category from, for example, position 25 to position 0 would result in switching the positions of the two categories at those positions.

Category A at position 0 would move to position 25, and Category B at position 25 would move to position 0.

Instead of switching positions, the reorder categories function should retain the order of categories except for the one being moved.

So, Category B at position 25 would still move to position 0, but Category A is merely bumped down to position 1.

This improves the UX because if a user *really* wants to switch the two categories, it results in one extra step. However in the other (what I think is normal) case, it saves the 24 other switches the user has to make to get Category A back to position 1 (you can imagine the user having to click the up arrow button repeatedly to return Category A to the top of the page). Now, imagine trying to do this with a site with 100s of categories. Yikes!

The UX improvement described above is what this commit accomplishes by redesigning the `move()` method of the reorder-categories controller. It adds some overhead to adjust the positions of all categories in between the origin and target positions, but in testing this is not noticible to the user. It's better for the computer to do extra work than the user.

* UX: Allow decimal input in reorder-categories for more precise positioning.

A common UX pattern when reordering a list of items is to allow a user to specify a target position as a decimal between two valid integer positions. The user is indicating they want the target list item to move in between the list items at the positions on either side of the target position.

For example, say there are three categories Category A at position 0, Category B at position 1, and Category C at position 3.

To move Category C in between Categories A and B, a user can now simply update Category C's position to 0.5.
This commit is contained in:
Grayden 2021-06-09 06:01:06 -04:00 committed by GitHub
parent 023ff9a282
commit 7ba35e0d71
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 141 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

121
app/assets/javascripts/discourse/app/controllers/reorder-categories.js
View File

@ -60,56 +60,97 @@ export default Controller.extend(ModalFunctionality, Evented, {
this.notifyPropertyChange("categoriesBuffered");
},
countDescendants(category) {
return category.get("subcategories")
? category
.get("subcategories")
.reduce(
(count, subcategory) => count + this.countDescendants(subcategory),
category.get("subcategories").length
)
: 0;
},
move(category, direction) {
let otherCategory;
let targetPosition = category.get("position") + direction;
if (direction === -1) {
// First category above current one
const categoriesOrderedDesc = this.categoriesOrdered.reverse();
otherCategory = categoriesOrderedDesc.find(
(c) =>
category.get("parent_category_id") === c.get("parent_category_id") &&
c.get("position") < category.get("position")
);
} else if (direction === 1) {
// First category under current one
otherCategory = this.categoriesOrdered.find(
(c) =>
category.get("parent_category_id") === c.get("parent_category_id") &&
c.get("position") > category.get("position")
);
} else {
// Find category occupying target position
otherCategory = this.categoriesOrdered.find(
(c) => c.get("position") === category.get("position") + direction
);
}
if (otherCategory) {
// Try to swap positions of the two categories
if (category.get("id") !== otherCategory.get("id")) {
const currentPosition = category.get("position");
category.set("position", otherCategory.get("position"));
otherCategory.set("position", currentPosition);
// Adjust target position for sub-categories
if (direction > 0) {
// Moving down (position gets larger)
if (category.get("isParent")) {
// This category has subcategories, adjust targetPosition to account for them
let offset = this.countDescendants(category);
if (direction <= offset) {
// Only apply offset if target position is occupied by a subcategory
// Seems weird but fixes a UX quirk
targetPosition += offset;
}
}
} else {
// Moving up (position gets smaller)
const otherCategory = this.categoriesOrdered.find(
(c) =>
// find category currently at targetPosition
c.get("position") === targetPosition
);
if (otherCategory && otherCategory.get("ancestors")) {
// Target category is a subcategory, adjust targetPosition to account for ancestors
const highestAncestor = otherCategory
.get("ancestors")
.reduce((current, min) =>
current.get("position") < min.get("position") ? current : min
);
targetPosition = highestAncestor.get("position");
}
} else if (direction < 0) {
category.set("position", -1);
} else if (direction > 0) {
category.set("position", this.categoriesOrdered.length);
}
// Adjust target position for range bounds
if (targetPosition >= this.categoriesOrdered.length) {
// Set to max
targetPosition = this.categoriesOrdered.length - 1;
} else if (targetPosition < 0) {
// Set to min
targetPosition = 0;
}
// Update other categories between current and target position
this.categoriesOrdered.map((c) => {
if (direction < 0) {
// Moving up (position gets smaller)
if (
c.get("position") < category.get("position") &&
c.get("position") >= targetPosition
) {
const newPosition = c.get("position") + 1;
c.set("position", newPosition);
}
} else {
// Moving down (position gets larger)
if (
c.get("position") > category.get("position") &&
c.get("position") <= targetPosition
) {
const newPosition = c.get("position") - 1;
c.set("position", newPosition);
}
}
});
// Update this category's position to target position
category.set("position", targetPosition);
this.reorder();
},
actions: {
change(category, event) {
let newPosition = parseInt(event.target.value, 10);
newPosition = Math.min(
Math.max(newPosition, 0),
this.categoriesOrdered.length - 1
);
this.move(category, newPosition - category.get("position"));
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) {

128
app/assets/javascripts/discourse/tests/unit/controllers/reorder-categories-test.js
View File

@ -11,17 +11,14 @@ discourseModule("Unit | Controller | reorder-categories", function () {
categories.push(store.createRecord("category", { id: i, position: 0 }));
}
const reorderCategoriesController = this.getController(
"reorder-categories",
{ site: { categories } }
);
reorderCategoriesController.reorder();
const controller = this.getController("reorder-categories", {
site: { categories },
});
controller.reorder();
reorderCategoriesController
.get("categoriesOrdered")
.forEach((category, index) => {
assert.equal(category.get("position"), index);
});
controller.get("categoriesOrdered").forEach((category, index) => {
assert.equal(category.get("position"), index);
});
});
test("reorder places subcategories after their parent categories, while maintaining the relative order", function (assert) {
@ -51,14 +48,13 @@ discourseModule("Unit | Controller | reorder-categories", function () {
});
const expectedOrderSlugs = ["parent", "child2", "child1", "other"];
const reorderCategoriesController = this.getController(
"reorder-categories",
{ site: { categories: [child2, parent, other, child1] } }
);
reorderCategoriesController.reorder();
const controller = this.getController("reorder-categories", {
site: { categories: [child2, parent, other, child1] },
});
controller.reorder();
assert.deepEqual(
reorderCategoriesController.get("categoriesOrdered").mapBy("slug"),
controller.get("categoriesOrdered").mapBy("slug"),
expectedOrderSlugs
);
});
@ -84,21 +80,18 @@ discourseModule("Unit | Controller | reorder-categories", function () {
slug: "test",
});
const reorderCategoriesController = this.getController(
"reorder-categories",
{ site: { categories: [elem1, elem2, elem3] } }
);
const controller = this.getController("reorder-categories", {
site: { categories: [elem1, elem2, elem3] },
});
reorderCategoriesController.actions.change.call(
reorderCategoriesController,
elem1,
{ target: { value: "2" } }
);
// Move category 'foo' from position 0 to position 2
controller.send("change", elem1, { target: { value: "2" } });
assert.deepEqual(
reorderCategoriesController.get("categoriesOrdered").mapBy("slug"),
["test", "bar", "foo"]
);
assert.deepEqual(controller.get("categoriesOrdered").mapBy("slug"), [
"bar",
"test",
"foo",
]);
});
test("changing the position number of a category should place it at given position and respect children", function (assert) {
@ -129,72 +122,71 @@ discourseModule("Unit | Controller | reorder-categories", function () {
slug: "test",
});
const reorderCategoriesController = this.getController(
"reorder-categories",
{ site: { categories: [elem1, child1, elem2, elem3] } }
);
const controller = this.getController("reorder-categories", {
site: { categories: [elem1, child1, elem2, elem3] },
});
reorderCategoriesController.actions.change.call(
reorderCategoriesController,
elem1,
{ target: { value: 3 } }
);
controller.send("change", elem1, { target: { value: 3 } });
assert.deepEqual(
reorderCategoriesController.get("categoriesOrdered").mapBy("slug"),
["test", "bar", "foo", "foochild"]
);
assert.deepEqual(controller.get("categoriesOrdered").mapBy("slug"), [
"bar",
"test",
"foo",
"foochild",
]);
});
test("changing the position through click on arrow of a category should place it at given position and respect children", function (assert) {
const store = createStore();
const elem1 = store.createRecord("category", {
id: 1,
position: 0,
slug: "foo",
const child2 = store.createRecord("category", {
id: 105,
position: 2,
slug: "foochildchild",
parent_category_id: 104,
});
const child1 = store.createRecord("category", {
id: 4,
id: 104,
position: 1,
slug: "foochild",
parent_category_id: 1,
parent_category_id: 101,
subcategories: [child2],
});
const child2 = store.createRecord("category", {
id: 5,
position: 2,
slug: "foochildchild",
parent_category_id: 4,
const elem1 = store.createRecord("category", {
id: 101,
position: 0,
slug: "foo",
subcategories: [child1],
});
const elem2 = store.createRecord("category", {
id: 2,
id: 102,
position: 3,
slug: "bar",
});
const elem3 = store.createRecord("category", {
id: 3,
id: 103,
position: 4,
slug: "test",
});
const reorderCategoriesController = this.getController(
"reorder-categories",
{ site: { categories: [elem1, child1, child2, elem2, elem3] } }
);
reorderCategoriesController.reorder();
const controller = this.getController("reorder-categories", {
site: { categories: [elem1, child1, child2, elem2, elem3] },
});
reorderCategoriesController.actions.moveDown.call(
reorderCategoriesController,
elem1
);
controller.reorder();
assert.deepEqual(
reorderCategoriesController.get("categoriesOrdered").mapBy("slug"),
["bar", "foo", "foochild", "foochildchild", "test"]
);
controller.send("moveDown", elem1);
assert.deepEqual(controller.get("categoriesOrdered").mapBy("slug"), [
"bar",
"foo",
"foochild",
"foochildchild",
"test",
]);
});
});