pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[MERGE][IMP] web: improve properties selection options ordering

Browse Source 
This merge commits contains 2 improvements aiming to simplify the ordering of
selection options for property fields:

- Correctly insert the new element at the right position when pressing enter
- Allow to drag & drop property values

See underlying commits for details.

Task-3338157

closes odoo/odoo#123216

Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
This commit is contained in:
Odoo's Mergebot
2023-07-31 19:02:30 +02:00
committed by GitHub
parent 19f06f9c86 dab3771377
commit f97e704fb4
4 changed files with 166 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/views/fields/properties/property_definition_selection.js
+100 -9
View File
@@ -4,6 +4,7 @@ import { useService } from "@web/core/utils/hooks";
import { uuid } from "../../utils";
import { Component, useState, useRef, useEffect } from "@odoo/owl";
import { useSortable } from "@web/core/utils/sortable";
export class PropertyDefinitionSelection extends Component {
setup() {
@@ -12,20 +13,38 @@ export class PropertyDefinitionSelection extends Component {
// when we create a new option, it's added in the state
// when we have finished to edit it (blur / enter) we propagate
// the new value in the props
this.state = useState({ newOption: null });
this.state = useState({
newOption: null,
});
this.propertyDefinitionSelectionRef = useRef("propertyDefinitionSelection");
this.addButtonRef = useRef("addButton");
useEffect(() => {
// automatically give the focus to the new option if it is empty
if (!this.state.newOption) {
return;
}
const inputs = this.propertyDefinitionSelectionRef.el.querySelectorAll(
".o_field_property_selection_option input"
);
if (inputs && inputs.length && !inputs[inputs.length - 1].value) {
inputs[inputs.length - 1].focus();
if (inputs && inputs.length && !inputs[this.state.newOption.index].value) {
inputs[this.state.newOption.index].focus();
}
});
useSortable({
enable: () => this.props.canChangeDefinition && !this.props.readonly,
ref: this.propertyDefinitionSelectionRef,
handle: ".o_field_property_selection_drag",
elements: ".o_field_property_selection_option",
cursor: "grabbing",
onDrop: async ({ element, previous }) => {
const movedOption = element.getAttribute("option-name");
const destinationOption = previous && previous.getAttribute("option-name");
await this.onOptionMoveTo(movedOption, destinationOption);
},
});
}
/* --------------------------------------------------------
@@ -51,7 +70,11 @@ export class PropertyDefinitionSelection extends Component {
*/
get optionsVisible() {
const options = this.options || [];
return this.state.newOption ? [...options, this.state.newOption] : options;
const newOption = this.state.newOption;
if (newOption) {
options.splice(newOption.index, 0, [newOption.name, ""]);
}
return options;
}
/* --------------------------------------------------------
@@ -61,8 +84,11 @@ export class PropertyDefinitionSelection extends Component {
/**
* Add a new empty selection option.
*/
onOptionCreate() {
this.state.newOption = [uuid(), ""];
onOptionCreate(index) {
this.state.newOption = {
index: index,
name: uuid(),
};
}
/**
@@ -93,7 +119,7 @@ export class PropertyDefinitionSelection extends Component {
const nonEmptyOptions = options.filter((option) => option[1] && option[1].length);
this.props.onOptionsChange(nonEmptyOptions);
if (this.state.newOption && this.state.newOption[1] && this.state.newOption[1].length) {
if (this.state.newOption) {
// the new option has been propagated in the props
this.state.newOption = null;
}
@@ -103,6 +129,10 @@ export class PropertyDefinitionSelection extends Component {
* Loose focus on an option, should cancel the newly
* created option if we didn't write on it.
*
* The attribute `_ignoreBlur` can be set if we don't want to remove
* the option if it's empty (and it will re-gain the focus at the
* next `useEffect` call).
*
* @param {event} event
* @param {integer} optionIndex
*/
@@ -110,6 +140,9 @@ export class PropertyDefinitionSelection extends Component {
if (event.target.value && event.target.value.length) {
// losing the focus on an non-empty option should have no effect
return;
} else if (this._ignoreBlur) {
this._ignoreBlur = false;
return;
}
if (event.relatedTarget === this.addButtonRef.el) {
@@ -117,7 +150,7 @@ export class PropertyDefinitionSelection extends Component {
// if the value is empty, just ignore and cancel the event
event.stopPropagation();
event.preventDefault();
} else if (optionIndex >= this.options.length) {
} else if (optionIndex === this.state.newOption?.index) {
// we remove the focus from the new empty option, remove it
this.state.newOption = null;
}
@@ -144,7 +177,7 @@ export class PropertyDefinitionSelection extends Component {
}
this.onOptionChange(event, optionIndex);
this.onOptionCreate();
this.onOptionCreate(optionIndex + 1);
} else if (["ArrowUp", "ArrowDown"].includes(event.key)) {
event.stopPropagation();
event.preventDefault();
@@ -186,6 +219,64 @@ export class PropertyDefinitionSelection extends Component {
options.splice(optionIndex, 1);
this.props.onOptionsChange(options);
}
/**
* Move an option after an other one.
*
* @param {string} from, the option to move
* @param {string} to, the target option
* (null if we move the option at the first index)
*/
onOptionMoveTo(movedOption, destinationOption) {
this._ignoreBlur = true;
let options = this.optionsVisible;
// if destinationOption is null, destinationOptionIndex will be -1 which is intended
let destinationOptionIndex = options.findIndex((option) => option[0] == destinationOption);
const movedOptionIndex = options.findIndex((option) => option[0] == movedOption);
if (destinationOptionIndex < movedOptionIndex) {
// the first splice operation won't change the index (and we except it to decrease it)
// for example if we have [A, B, C], and we move C such that it becomes [A, C, B]
// destinationOption is A and the destination index is 0, but we need the index to be 1
// (if the destination is after the moved option, the first splice will fix it for us)
destinationOptionIndex++;
}
const activeEl = document.activeElement;
if (
activeEl &&
this.propertyDefinitionSelectionRef.el.contains(activeEl) &&
activeEl.tagName === "INPUT"
) {
const optionName = activeEl
.closest(".o_field_property_selection_option")
.getAttribute("option-name");
const editedOptionIndex = options.findIndex((option) => option[0] === optionName);
// we might be editing the value and drag and drop something else just after
options[editedOptionIndex][1] = activeEl.value;
}
options.splice(destinationOptionIndex, 0, options.splice(movedOptionIndex, 1)[0]);
if (this.state.newOption) {
const newOptionIndex = options.findIndex(
(option) => option[0] === this.state.newOption.name
);
if (!options[newOptionIndex][1]?.length) {
// if there's an empty option, fix it's index in the state
// and do not propagate it in the props
this.state.newOption = {
...this.state.newOption,
index: newOptionIndex,
};
options = options.filter((option) => option[0] !== this.state.newOption.name);
} else {
this.state.newOption = null;
}
}
this.props.onOptionsChange(options);
}
}
PropertyDefinitionSelection.template = "web.PropertyDefinitionSelection";
addons/web/static/src/views/fields/properties/property_definition_selection.scss
+8
View File
@@ -3,3 +3,11 @@
color: $o-main-favorite-color;
}
}
.o_field_property_selection_drag {
@include o-grab-cursor;
color: #adb5bd;
&:hover {
color: $o-component-active-color;
}
}
addons/web/static/src/views/fields/properties/property_definition_selection.xml
+5 -2
View File
@@ -6,7 +6,10 @@
<div class="w-100">
<div
t-foreach="optionsVisible" t-as="option" t-key="option[0]"
class="o_field_property_selection_option d-flex flex-row align-items-center justify-content-between mb-2 rounded">
class="o_field_property_selection_option d-flex flex-row align-items-center justify-content-between mb-2 rounded"
t-att-option-name="option[0]">
<i t-if="!props.readonly and props.canChangeDefinition"
class="o_field_property_selection_drag oi oi-draggable ui-sortable-handle me-2 text-center"/>
<span t-if="props.readonly" t-out="option[1]"/>
<input
t-else=""
@@ -35,7 +38,7 @@
<button
t-if="!props.readonly"
class="btn btn-link btn-sm"
t-on-click="onOptionCreate"
t-on-click="() => this.onOptionCreate(this.options.length)"
t-ref="addButton">
<i class="fa fa-plus"/>
Add a Value
addons/web/static/tests/views/fields/properties_field_tests.js
+53 -9
View File
@@ -4,6 +4,7 @@ import {
click,
clickDiscard,
clickSave,
dragAndDrop,
editInput,
editSelect,
getFixture,
@@ -486,9 +487,16 @@ QUnit.module("Fields", (hooks) => {
const type = popover.querySelector(".o_field_property_definition_type input");
assert.strictEqual(type.value, "Selection");
const getOptions = () => {
return popover.querySelectorAll(".o_field_property_selection_option");
};
const getOptionsValues = () => {
return [...getOptions()].map((option) => option.querySelector("input").value);
};
// Create a new selection option
await click(target, ".o_field_property_selection .fa-plus");
let options = popover.querySelectorAll(".o_field_property_selection_option");
let options = getOptions();
assert.strictEqual(options.length, 4, "Should have added the new option");
assert.strictEqual(
document.activeElement,
@@ -511,7 +519,7 @@ QUnit.module("Fields", (hooks) => {
);
await nextTick();
options = popover.querySelectorAll(".o_field_property_selection_option");
options = getOptions();
assert.strictEqual(options.length, 5, "Should have added the new option on Enter");
assert.strictEqual(
document.activeElement,
@@ -530,12 +538,12 @@ QUnit.module("Fields", (hooks) => {
await nextTick();
options = getOptions();
assert.strictEqual(
document.activeElement,
options[3].querySelector("input"),
"Should focus the previous option"
);
options = popover.querySelectorAll(".o_field_property_selection_option");
assert.strictEqual(
options.length,
4,
@@ -554,19 +562,55 @@ QUnit.module("Fields", (hooks) => {
await nextTick();
assert.strictEqual(document.activeElement, options[2].querySelector("input"));
options = popover.querySelectorAll(".o_field_property_selection_option");
assert.strictEqual(options.length, 4, "Should not remove any options");
assert.strictEqual(getOptions().length, 4, "Should not remove any options");
// Remove the second option
await click(target, ".o_field_property_selection_option:nth-child(2) .fa-trash-o");
options = popover.querySelectorAll(".o_field_property_selection_option");
assert.strictEqual(options.length, 3, "Should have removed the second option");
const optionValues = [...options].map((option) => option.querySelector("input").value);
assert.deepEqual(
optionValues,
getOptionsValues(),
["A", "C", "New option"],
"Should have removed the second option"
);
// focus should be in option C
assert.strictEqual(document.activeElement, getOptions()[1].querySelector("input"));
// test that pressing 'Enter' inserts a new option after the one currently focused (and not last).
await triggerEvent(
target,
".o_field_property_selection_option:nth-child(2) input",
"keydown",
{ key: "Enter" }
);
await editInput(
target,
".o_field_property_selection_option:nth-child(3) input",
"New option 2"
);
assert.deepEqual(
getOptionsValues(),
["A", "C", "New option 2", "New option"],
"Should have added a new option at the correct spot"
);
await nextTick();
const getOptionDraggableElement = (index) => {
return target.querySelector(
`.o_field_property_selection_option:nth-child(${index + 1})` +
" .o_field_property_selection_drag"
);
};
await dragAndDrop(getOptionDraggableElement(0), getOptionDraggableElement(2));
assert.deepEqual(getOptionsValues(), ["C", "New option 2", "A", "New option"]);
await dragAndDrop(getOptionDraggableElement(3), getOptionDraggableElement(0));
assert.deepEqual(getOptionsValues(), ["New option", "C", "New option 2", "A"]);
// create an empty option and move it
await click(target, ".o_field_property_selection > div > .btn-link");
assert.deepEqual(getOptionsValues(), ["New option", "C", "New option 2", "A", ""]);
await dragAndDrop(getOptionDraggableElement(4), getOptionDraggableElement(1));
assert.deepEqual(getOptionsValues(), ["New option", "", "C", "New option 2", "A"]);
});
/**