From 202ae501e10f017185941d6ff0edd4778d063d1f Mon Sep 17 00:00:00 2001 From: Eugen Ciur <eugen@papermerge.com> Date: Mon, 28 Feb 2022 20:32:14 +0100 Subject: [PATCH] fix UI selection related bugs --- app/components/commander/index.hbs | 7 +- app/components/commander/index.js | 68 ++-------------- app/controllers/authenticated.js | 8 ++ app/modifiers/draggable.js | 30 +++---- app/modifiers/ui_select.js | 123 ++++++++++++++++++++++------- app/utils/array.js | 32 ++++++++ app/utils/index.js | 1 - app/utils/rectangle.js | 12 ++- 8 files changed, 168 insertions(+), 113 deletions(-) create mode 100644 app/utils/array.js diff --git a/app/components/commander/index.hbs b/app/components/commander/index.hbs index 1715140..6bcd4a6 100644 --- a/app/components/commander/index.hbs +++ b/app/components/commander/index.hbs @@ -3,10 +3,13 @@ onDrop=this.onDrop onDragOver=this.onDragOver }} - {{uiSelect}} + {{uiSelect view_mode=this.view_mode enabled_on='grid'}} {{contextMenu}}> - <UiSelect /> + {{#if (is_equal this.view_mode 'grid')}} + <!-- ui select is enabled only in grid mode --> + <UiSelect /> + {{/if}} <ContextMenu @openNewFolderModal={{this.openNewFolderModal}} diff --git a/app/components/commander/index.js b/app/components/commander/index.js index a9a4fb2..924236d 100644 --- a/app/components/commander/index.js +++ b/app/components/commander/index.js @@ -47,22 +47,10 @@ export default class CommanderComponent extends Component { constructor(owner, args) { super(owner, args); - this.websockets.addHandler(this.messageHandler, this); - this.ws_nodes_move.addHandler(this.wsNodesMoveHandler, this); this.preferences.load(); } - wsNodesMoveHandler(message) { - if (message.type === 'nodesmove.tasksucceeded') { - if (this.args.node.id == message.source_parent['id']) { - this._substract_nodes(message.nodes.map(node => node.id)); - } else if (this.args.node.id == message.target_parent['id']) { - this._add_nodes(message.nodes.map(node => node.id)); - } - } - } - messageHandler(message) { let doc; @@ -88,51 +76,6 @@ export default class CommanderComponent extends Component { } // end of switch } - _substract_nodes(node_ids) { - let that = this, doc; - - node_ids.forEach(node_id => { - // maybe document ? - doc = that.store.peekRecord('document', node_id); - if (doc) { - that._substract_node(doc); - } - // maybe folder ? - doc = that.store.peekRecord('folder', node_id); - if (doc) { - that._substract_node(doc); - } - }); - } - - _substract_node(node) { - this.deleted_records.push(node); - this.__deleted_record = node; - this.selected_nodes = []; - } - - _add_nodes(node_ids) { - let that = this, doc; - - node_ids.forEach(node_id => { - // maybe document ? - doc = that.store.peekRecord('document', node_id); - if (doc) { - that._add_node(doc); - } - // maybe folder ? - doc = that.store.peekRecord('folder', node_id); - if (doc) { - that._add_node(doc); - } - }); - } - - _add_node(doc) { - this.new_records.push(doc); - this.__new_record = doc; - } - @action onNodeClicked() { /** @@ -309,14 +252,17 @@ export default class CommanderComponent extends Component { } @action - onDragendSuccess(model) { + onDragendSuccess(model, sel_nodes) { /** - Action invoked when drag operation for a node (folder/document) + Action invoked when drag operation for one or multiple nodes succeeded. It is invoked on the SOURCE panel. - `model` is instance of `model.document` or `model.folder` */ - console.log(`onDragendSuccess on ${this.args.hint}: id=${model.id} type=${model.nodeType}`); + console.log( + `onDragendSuccess on ${this.args.hint}: id=${model.id} type=${model.nodeType}` + ); + console.log(`onDragendSuccess selected_nodes=${sel_nodes}`); + this.selected_nodes = []; // reset currect selected nodes list } get lang() { diff --git a/app/controllers/authenticated.js b/app/controllers/authenticated.js index 7fb32ae..42440ec 100644 --- a/app/controllers/authenticated.js +++ b/app/controllers/authenticated.js @@ -9,17 +9,25 @@ export default class AuthenticatedController extends Controller { // sidebar expanded ? @tracked expanded = true; @service ws_inbox_refresh; + @service ws_nodes_move; @service router; constructor() { super(...arguments); this.ws_inbox_refresh.addHandler(this.wsInboxRefreshHandler, this); + this.ws_nodes_move.addHandler(this.wsNodesMoveHandler, this); } wsInboxRefreshHandler() { this.router.refresh(); } + wsNodesMoveHandler(message) { + if (message.type === 'nodesmove.tasksucceeded') { + this.router.refresh(); + } + } + @action onSidebarToggle() { this.expanded = !this.expanded; diff --git a/app/modifiers/draggable.js b/app/modifiers/draggable.js index 8f94caf..b908729 100644 --- a/app/modifiers/draggable.js +++ b/app/modifiers/draggable.js @@ -1,5 +1,6 @@ import { action } from '@ember/object'; import Modifier from 'ember-modifier'; +import { merge_nodes } from 'papermerge/utils/array'; export default class DraggableModifier extends Modifier { @@ -58,26 +59,19 @@ export default class DraggableModifier extends Modifier { @action onDragStart(event) { - let data, selected_nodes, nodes, source_nodes; + let data, nodes; this.model = this.args.positional[0]; - selected_nodes = this.args.named['selectedNodes']; - nodes = [{ - id: this.model.id - }]; - - if (selected_nodes && selected_nodes.length > 0) { - source_nodes = nodes.concat( - selected_nodes.map(item => { - return {'id': item.get('id')}; - }) - ); - } else { - source_nodes = nodes; - } + this.selected_nodes = this.args.named['selectedNodes']; + + // Merge model from which user started dragging + // with rest of selected nodes (in case there are some) + // into one single list of {id: <node_id>} objects. + // Resulted list won't have any duplicates. + nodes = merge_nodes(this.model.id, this.selected_nodes); data = { - nodes: source_nodes, + nodes: nodes, source_parent: { id: this.model.parent.get('id') } @@ -95,9 +89,9 @@ export default class DraggableModifier extends Modifier { const ondragend_cancel = this.args.named['onDragendCancel']; if (event.dataTransfer.dropEffect === "move") { - ondragend_success(this.model); + ondragend_success(this.model, this.selected_nodes); } else { - ondragend_cancel(this.model); + ondragend_cancel(this.model, this.selected_nodes); } } } diff --git a/app/modifiers/ui_select.js b/app/modifiers/ui_select.js index 8bb30cf..5b70a01 100644 --- a/app/modifiers/ui_select.js +++ b/app/modifiers/ui_select.js @@ -16,7 +16,7 @@ class UISelect { this confusion, DRAG_THRESHOLD is introduced. Any rectangle with height or width < DRAG_THRESHOLD will be discarded. */ - return 5; + return 4; } constructor(parent_selector) { @@ -33,9 +33,6 @@ class UISelect { this.current_y = 0; this.parent = parent_selector; this.select_div = document.getElementById('ui-select'); - this.nodes_arr = Array.from( - document.getElementsByClassName('node') - ); } init(x, y) { @@ -99,11 +96,16 @@ class UISelect { get_nodes_selection(selection_rect) { /** - selection_rect is instance of utils.MgRect + selection_rect is instance of utils.Rectangle **/ - let selected_nodes = [], unselected_nodes = []; + let selected_nodes = [], unselected_nodes = [], nodes_arr; + + nodes_arr = Array.from( + document.getElementsByClassName('node') + ); - this.nodes_arr.forEach(element => { + + nodes_arr.forEach(element => { let _r, rect; _r = element.getBoundingClientRect(); @@ -142,9 +144,13 @@ class UISelect { } deselect_all_nodes() { - let input_el; + let input_el, nodes_arr; + + nodes_arr = Array.from( + document.getElementsByClassName('node') + ); - this.nodes_arr.forEach(element => { + nodes_arr.forEach(element => { input_el = element.querySelector('input'); if (input_el && input_el.checked) { input_el.click(); @@ -152,6 +158,37 @@ class UISelect { }); } + get is_dragging() { + /* + Is this a dragging ? + + Selection can only start OUTSIDE nodes (in that space + between nodes, available grid mode). + If starting point is INSIDE any node, this is dragging operation. + Again: + starting poing INSIDE ANY NODE -> this is dragging + starting point OUTSIDE ALL NODES -> this is selection + */ + + let nodes_arr, that = this, result = false; + + nodes_arr = Array.from( + document.getElementsByClassName('node') + ); + + nodes_arr.forEach(element => { + let _r, rect; + + _r = element.getBoundingClientRect(); + rect = new Rectangle(_r.x, _r.y, _r.width, _r.height); + if (rect.contains_point(that.start_x, that.start_y)) { + result = true; + } + }); + + return result; + } + set width(value) { if (!this.select_div) { return; @@ -191,55 +228,87 @@ class UISelect { export default class UISelectModifier extends Modifier { + /* + * Desktop like select is enabled ONLY when (commander is) in grid mode + i.e. when items to select are showed like a grid. + + When displayed like a grid there is a some space between items which enables + user to create a selection rectangle. On other hand, when in list mode, there is not + much space between items and selection interferes with drag and drop features i.e. + user tries to select an items, but it looks like user tries to drag it. + */ ui_select = undefined; addEventListener() { - this.element.addEventListener('mousedown', this.onMouseDown); - this.element.addEventListener('mouseup', this.onMouseUp); - this.element.addEventListener('mousemove', this.onMouseMove); + if (this.is_enabled) { + this.element.addEventListener('mousedown', this.onMouseDown); + this.element.addEventListener('mouseup', this.onMouseUp); + this.element.addEventListener('mousemove', this.onMouseMove); + } } removeEventListener() { - this.element.removeEventListener('mousedown', this.onMouseDown); - this.element.removeEventListener('mouseup', this.onMouseUp); - this.element.removeEventListener('mousemove', this.onMouseMove); + if (this.is_enabled) { + this.element.removeEventListener('mousedown', this.onMouseDown); + this.element.removeEventListener('mouseup', this.onMouseUp); + this.element.removeEventListener('mousemove', this.onMouseMove); + } } // lifecycle hooks didReceiveArguments() { - this.removeEventListener(); - this.addEventListener(); + if (this.is_enabled) { + this.removeEventListener(); + this.addEventListener(); - this.ui_select = new UISelect(this.element); + this.ui_select = new UISelect(this.element); + } } willDestroy() { - this.removeEventListener(); + if (this.is_enabled) { + this.removeEventListener(); + } } @action onMouseMove(event) { - if (!event.buttons) { - this.hide(); - } else if (this.ui_select) { - this.ui_select.update(event.clientX, event.clientY); + if (this.is_enabled) { + if (!event.buttons) { + this.hide(); + } else if (this.ui_select) { + this.ui_select.update(event.clientX, event.clientY); + } } } @action onMouseUp() { - this.hide(); + if (this.is_enabled) { + this.hide(); + } } @action onMouseDown(event) { - this.ui_select.init(event.clientX, event.clientY); + if (this.is_enabled) { + this.ui_select.init(event.clientX, event.clientY); + } } hide() { - if (this.ui_select) { - this.ui_select.hide(); + if (this.is_enabled) { + if (this.ui_select) { + this.ui_select.hide(); + } } } + + get is_enabled() { + let view_mode = this.args.named['view_mode'], + enabled_on = this.args.named['enabled_on']; + + return view_mode === enabled_on; + } } diff --git a/app/utils/array.js b/app/utils/array.js new file mode 100644 index 0000000..d7f1e17 --- /dev/null +++ b/app/utils/array.js @@ -0,0 +1,32 @@ +function merge_nodes(node_id, nodes) { + /* + Returns a list of {id: <node.id>} objects with no duplicates. + List contains as node_id given as first parameter as well as all nodes + given as second parameter. + */ + let source_nodes; + + if (!nodes) { + return [{id: node_id}]; + } + + if (!nodes.length) { + return [{id: node_id}]; + } + + source_nodes = nodes.map(item => { + return {'id': item.get('id')}; + }); + + // if by concatinating nodes with node_id there + // will be no duplicates: + if (!source_nodes.find(item => item.id == node_id)) { + return [{id: node_id}].concat(source_nodes) + } + + return source_nodes; +} + +export { + merge_nodes +} \ No newline at end of file diff --git a/app/utils/index.js b/app/utils/index.js index e2559ed..81adccf 100644 --- a/app/utils/index.js +++ b/app/utils/index.js @@ -1,4 +1,3 @@ -import ENV from 'papermerge/config/environment'; function group_perms_by_model(permissions) { diff --git a/app/utils/rectangle.js b/app/utils/rectangle.js index 9c60298..5c89745 100644 --- a/app/utils/rectangle.js +++ b/app/utils/rectangle.js @@ -11,11 +11,11 @@ function intersect(rectA, rectB) { rectB_X2 = rectB.p3.x, rectB_Y2 = rectB.p3.y; - if ((rectA_X1 <= rectB_X2) && (rectA_X2 >= rectB_X1) && (rectA_Y1 <= rectB_Y2) && (rectA_Y2 >= rectB_Y1)) { - return true; - } + if ((rectA_X1 <= rectB_X2) && (rectA_X2 >= rectB_X1) && (rectA_Y1 <= rectB_Y2) && (rectA_Y2 >= rectB_Y1)) { + return true; + } - return false; + return false; } @@ -37,6 +37,10 @@ export default class Rectangle { return intersect(rect, this) || intersect(this, rect); } + contains_point(x, y) { + + } + toString() { return `Rectangle(x=${this.x}, y=${this.y}, w=${this.width}, h=${this.height})` } -- GitLab