From 31d37e7e75569d1c5c33ef97085539ea1b4afa0a Mon Sep 17 00:00:00 2001 From: Thijs Wenker Date: Thu, 15 Nov 2018 02:38:39 +0100 Subject: [PATCH 1/4] Fix entity list double click rename / highlight issue --- scripts/system/html/js/entityList.js | 32 +++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/scripts/system/html/js/entityList.js b/scripts/system/html/js/entityList.js index 39673b8f16..ac2a4132ae 100644 --- a/scripts/system/html/js/entityList.js +++ b/scripts/system/html/js/entityList.js @@ -153,6 +153,9 @@ const ICON_FOR_TYPE = { Text: "l", }; +const DOUBLE_CLICK_TIMEOUT = 300; // ms +const RENAME_COOLDOWN = 400; // ms + // List of all entities let entities = []; // List of all entities, indexed by Entity ID @@ -181,6 +184,9 @@ let currentResizeEl = null; let startResizeEvent = null; let resizeColumnIndex = 0; let startThClick = null; +let renameTimeout = null; +let renameLastBlur = null; +let renameLastEntityID = null; let elEntityTable, elEntityTableHeader, @@ -394,6 +400,7 @@ function loaded() { entityListContextMenu = new EntityListContextMenu(); function startRenamingEntity(entityID) { + renameLastEntityID = entityID; let entity = entitiesByID[entityID]; if (!entity || entity.locked || !entity.elRow) { return; @@ -423,6 +430,8 @@ function loaded() { })); entity.name = value; elCell.innerText = value; + + renameLastBlur = Date.now(); }; elCell.innerHTML = ""; @@ -478,6 +487,13 @@ function loaded() { entityListContextMenu.open(clickEvent, entityID, enabledContextMenuItems); } + let clearRenameTimeout = () => { + if (renameTimeout !== null) { + window.clearTimeout(renameTimeout); + renameTimeout = null; + } + }; + function onRowClicked(clickEvent) { let entityID = this.dataset.entityID; let selection = [entityID]; @@ -516,7 +532,15 @@ function loaded() { } else if (!clickEvent.ctrlKey && !clickEvent.shiftKey && selectedEntities.length === 1) { // if reselecting the same entity then start renaming it if (selectedEntities[0] === entityID) { - startRenamingEntity(entityID); + if (renameLastBlur && renameLastEntityID === entityID && (Date.now() - renameLastBlur) < RENAME_COOLDOWN) { + + return; + } + clearRenameTimeout(); + renameTimeout = window.setTimeout(() => { + renameTimeout = null; + startRenamingEntity(entityID); + }, DOUBLE_CLICK_TIMEOUT); } } @@ -530,6 +554,8 @@ function loaded() { } function onRowDoubleClicked() { + clearRenameTimeout(); + let selection = [this.dataset.entityID]; updateSelectedEntities(selection, false); @@ -1100,12 +1126,12 @@ function loaded() { startResizeEvent = ev; } } - } + }; document.onmouseup = function(ev) { startResizeEvent = null; ev.stopPropagation(); - } + }; function setSpaceMode(spaceMode) { if (spaceMode === "local") { From e89346c146386a17cc08143fcd113a569152690d Mon Sep 17 00:00:00 2001 From: Thijs Wenker Date: Fri, 16 Nov 2018 20:02:16 +0100 Subject: [PATCH 2/4] making rename functionality list refresh proof --- scripts/system/html/js/entityList.js | 45 +++++++++++++++++++++++--- scripts/system/html/js/listView.js | 8 +++-- scripts/system/libraries/entityList.js | 2 +- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/scripts/system/html/js/entityList.js b/scripts/system/html/js/entityList.js index ac2a4132ae..328c8c76e1 100644 --- a/scripts/system/html/js/entityList.js +++ b/scripts/system/html/js/entityList.js @@ -187,6 +187,7 @@ let startThClick = null; let renameTimeout = null; let renameLastBlur = null; let renameLastEntityID = null; +let isRenameFieldIsBeingMoved = false; let elEntityTable, elEntityTableHeader, @@ -210,7 +211,8 @@ let elEntityTable, elNoEntitiesMessage, elColumnsMultiselectBox, elColumnsOptions, - elToggleSpaceMode; + elToggleSpaceMode, + elRenameInput; const ENABLE_PROFILING = false; let profileIndent = ''; @@ -395,7 +397,7 @@ function loaded() { elEntityTableHeaderRow = document.querySelectorAll("#entity-table thead th"); entityList = new ListView(elEntityTableBody, elEntityTableScroll, elEntityTableHeaderRow, - createRow, updateRow, clearRow, WINDOW_NONVARIABLE_HEIGHT); + createRow, updateRow, clearRow, beforeUpdate, afterUpdate, WINDOW_NONVARIABLE_HEIGHT); entityListContextMenu = new EntityListContextMenu(); @@ -407,7 +409,7 @@ function loaded() { } let elCell = entity.elRow.childNodes[getColumnIndex("name")]; - let elRenameInput = document.createElement("input"); + elRenameInput = document.createElement("input"); elRenameInput.setAttribute('class', 'rename-entity'); elRenameInput.value = entity.name; let ignoreClicks = function(event) { @@ -422,6 +424,9 @@ function loaded() { }; elRenameInput.onblur = function(event) { + if (isRenameFieldIsBeingMoved) { + return; + } let value = elRenameInput.value; EventBridge.emitWebEvent(JSON.stringify({ type: 'rename', @@ -429,9 +434,10 @@ function loaded() { name: value })); entity.name = value; - elCell.innerText = value; + elRenameInput.parentElement.innerText = value; renameLastBlur = Date.now(); + elRenameInput = null; }; elCell.innerHTML = ""; @@ -440,6 +446,32 @@ function loaded() { elRenameInput.select(); } + function beforeUpdate() { + // move the rename input to the body + if (elRenameInput) { + isRenameFieldIsBeingMoved = true; + document.body.appendChild(elRenameInput); + // keep the focus + elRenameInput.select(); + } + } + + function afterUpdate() { + if (!elRenameInput || !isRenameFieldIsBeingMoved) { + return; + } + let entity = entitiesByID[renameLastEntityID]; + if (!entity || entity.locked || !entity.elRow) { + return; + } + let elCell = entity.elRow.childNodes[getColumnIndex("name")]; + elCell.innerHTML = ""; + elCell.appendChild(elRenameInput); + // keep the focus + elRenameInput.select(); + isRenameFieldIsBeingMoved = false; + } + entityListContextMenu.setOnSelectedCallback(function(optionName, selectedEntityID) { switch (optionName) { case "Cut": @@ -464,6 +496,11 @@ function loaded() { }); function onRowContextMenu(clickEvent) { + if (elRenameInput) { + // disallow the context menu from popping up while renaming + return; + } + let entityID = this.dataset.entityID; if (!selectedEntities.includes(entityID)) { diff --git a/scripts/system/html/js/listView.js b/scripts/system/html/js/listView.js index f775a4cb24..4336698688 100644 --- a/scripts/system/html/js/listView.js +++ b/scripts/system/html/js/listView.js @@ -14,7 +14,7 @@ debugPrint = function (message) { }; function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunction, - updateRowFunction, clearRowFunction, WINDOW_NONVARIABLE_HEIGHT) { + updateRowFunction, clearRowFunction, beforeRefreshFunction, afterRefreshFunction, WINDOW_NONVARIABLE_HEIGHT) { this.elTableBody = elTableBody; this.elTableScroll = elTableScroll; this.elTableHeaderRow = elTableHeaderRow; @@ -25,6 +25,8 @@ function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunctio this.createRowFunction = createRowFunction; this.updateRowFunction = updateRowFunction; this.clearRowFunction = clearRowFunction; + this.beforeRefreshFunction = beforeRefreshFunction; + this.afterRefreshFunction = afterRefreshFunction; // the list of row elements created in the table up to max viewable height plus SCROLL_ROWS rows for scrolling buffer this.elRows = []; @@ -173,6 +175,7 @@ ListView.prototype = { }, refresh: function() { + this.beforeRefreshFunction(); // block refreshing before rows are initialized let numRows = this.getNumRows(); if (numRows === 0) { @@ -211,6 +214,7 @@ ListView.prototype = { this.lastRowShiftScrollTop = 0; } } + this.afterRefreshFunction(); }, refreshBuffers: function() { @@ -230,7 +234,7 @@ ListView.prototype = { refreshRowOffset: function() { // make sure the row offset isn't causing visible rows to pass the end of the item list and is clamped to 0 - var numRows = this.getNumRows(); + let numRows = this.getNumRows(); if (this.rowOffset + numRows > this.itemData.length) { this.rowOffset = this.itemData.length - numRows; } diff --git a/scripts/system/libraries/entityList.js b/scripts/system/libraries/entityList.js index 8942c84f33..eeb16fd60d 100644 --- a/scripts/system/libraries/entityList.js +++ b/scripts/system/libraries/entityList.js @@ -17,7 +17,7 @@ var profileIndent = ''; const PROFILE_NOOP = function(_name, fn, args) { fn.apply(this, args); }; -PROFILE = !PROFILING_ENABLED ? PROFILE_NOOP : function(name, fn, args) { +const PROFILE = !PROFILING_ENABLED ? PROFILE_NOOP : function(name, fn, args) { console.log("PROFILE-Script " + profileIndent + "(" + name + ") Begin"); var previousIndent = profileIndent; profileIndent += ' '; From 7b3d2bed15a61caf4a2fef24612e6ca32ba01efb Mon Sep 17 00:00:00 2001 From: Thijs Wenker Date: Fri, 16 Nov 2018 20:49:25 +0100 Subject: [PATCH 3/4] CR fixes --- scripts/system/html/js/entityList.js | 16 ++++++++-------- scripts/system/html/js/listView.js | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/scripts/system/html/js/entityList.js b/scripts/system/html/js/entityList.js index 328c8c76e1..fe657cd8b5 100644 --- a/scripts/system/html/js/entityList.js +++ b/scripts/system/html/js/entityList.js @@ -187,7 +187,7 @@ let startThClick = null; let renameTimeout = null; let renameLastBlur = null; let renameLastEntityID = null; -let isRenameFieldIsBeingMoved = false; +let isRenameFieldBeingMoved = false; let elEntityTable, elEntityTableHeader, @@ -397,7 +397,7 @@ function loaded() { elEntityTableHeaderRow = document.querySelectorAll("#entity-table thead th"); entityList = new ListView(elEntityTableBody, elEntityTableScroll, elEntityTableHeaderRow, - createRow, updateRow, clearRow, beforeUpdate, afterUpdate, WINDOW_NONVARIABLE_HEIGHT); + createRow, updateRow, clearRow, preRefresh, postRefresh, WINDOW_NONVARIABLE_HEIGHT); entityListContextMenu = new EntityListContextMenu(); @@ -424,7 +424,7 @@ function loaded() { }; elRenameInput.onblur = function(event) { - if (isRenameFieldIsBeingMoved) { + if (isRenameFieldBeingMoved) { return; } let value = elRenameInput.value; @@ -446,18 +446,18 @@ function loaded() { elRenameInput.select(); } - function beforeUpdate() { + function preRefresh() { // move the rename input to the body if (elRenameInput) { - isRenameFieldIsBeingMoved = true; + isRenameFieldBeingMoved = true; document.body.appendChild(elRenameInput); // keep the focus elRenameInput.select(); } } - function afterUpdate() { - if (!elRenameInput || !isRenameFieldIsBeingMoved) { + function postRefresh() { + if (!elRenameInput || !isRenameFieldBeingMoved) { return; } let entity = entitiesByID[renameLastEntityID]; @@ -469,7 +469,7 @@ function loaded() { elCell.appendChild(elRenameInput); // keep the focus elRenameInput.select(); - isRenameFieldIsBeingMoved = false; + isRenameFieldBeingMoved = false; } entityListContextMenu.setOnSelectedCallback(function(optionName, selectedEntityID) { diff --git a/scripts/system/html/js/listView.js b/scripts/system/html/js/listView.js index 4336698688..482576e2fe 100644 --- a/scripts/system/html/js/listView.js +++ b/scripts/system/html/js/listView.js @@ -14,7 +14,7 @@ debugPrint = function (message) { }; function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunction, - updateRowFunction, clearRowFunction, beforeRefreshFunction, afterRefreshFunction, WINDOW_NONVARIABLE_HEIGHT) { + updateRowFunction, clearRowFunction, preRefreshFunction, postRefreshFunction, WINDOW_NONVARIABLE_HEIGHT) { this.elTableBody = elTableBody; this.elTableScroll = elTableScroll; this.elTableHeaderRow = elTableHeaderRow; @@ -25,8 +25,8 @@ function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunctio this.createRowFunction = createRowFunction; this.updateRowFunction = updateRowFunction; this.clearRowFunction = clearRowFunction; - this.beforeRefreshFunction = beforeRefreshFunction; - this.afterRefreshFunction = afterRefreshFunction; + this.preRefreshFunction = preRefreshFunction; + this.postRefreshFunction = postRefreshFunction; // the list of row elements created in the table up to max viewable height plus SCROLL_ROWS rows for scrolling buffer this.elRows = []; @@ -175,7 +175,7 @@ ListView.prototype = { }, refresh: function() { - this.beforeRefreshFunction(); + this.preRefreshFunction(); // block refreshing before rows are initialized let numRows = this.getNumRows(); if (numRows === 0) { @@ -214,7 +214,7 @@ ListView.prototype = { this.lastRowShiftScrollTop = 0; } } - this.afterRefreshFunction(); + this.postRefreshFunction(); }, refreshBuffers: function() { From ba790ae47013470e9bce78a87e07c060e1a1c5b8 Mon Sep 17 00:00:00 2001 From: Thijs Wenker Date: Sat, 17 Nov 2018 01:20:36 +0100 Subject: [PATCH 4/4] fix behavior after merge. --- scripts/system/html/js/entityList.js | 6 +++--- scripts/system/html/js/listView.js | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/system/html/js/entityList.js b/scripts/system/html/js/entityList.js index fe657cd8b5..84ad59df36 100644 --- a/scripts/system/html/js/entityList.js +++ b/scripts/system/html/js/entityList.js @@ -396,8 +396,8 @@ function loaded() { elEntityTableHeaderRow = document.querySelectorAll("#entity-table thead th"); - entityList = new ListView(elEntityTableBody, elEntityTableScroll, elEntityTableHeaderRow, - createRow, updateRow, clearRow, preRefresh, postRefresh, WINDOW_NONVARIABLE_HEIGHT); + entityList = new ListView(elEntityTableBody, elEntityTableScroll, elEntityTableHeaderRow, createRow, updateRow, + clearRow, preRefresh, postRefresh, preRefresh, WINDOW_NONVARIABLE_HEIGHT); entityListContextMenu = new EntityListContextMenu(); @@ -448,7 +448,7 @@ function loaded() { function preRefresh() { // move the rename input to the body - if (elRenameInput) { + if (!isRenameFieldBeingMoved && elRenameInput) { isRenameFieldBeingMoved = true; document.body.appendChild(elRenameInput); // keep the focus diff --git a/scripts/system/html/js/listView.js b/scripts/system/html/js/listView.js index 482576e2fe..49a91388a5 100644 --- a/scripts/system/html/js/listView.js +++ b/scripts/system/html/js/listView.js @@ -13,8 +13,8 @@ debugPrint = function (message) { console.log(message); }; -function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunction, - updateRowFunction, clearRowFunction, preRefreshFunction, postRefreshFunction, WINDOW_NONVARIABLE_HEIGHT) { +function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunction, updateRowFunction, clearRowFunction, + preRefreshFunction, postRefreshFunction, preResizeFunction, WINDOW_NONVARIABLE_HEIGHT) { this.elTableBody = elTableBody; this.elTableScroll = elTableScroll; this.elTableHeaderRow = elTableHeaderRow; @@ -27,6 +27,7 @@ function ListView(elTableBody, elTableScroll, elTableHeaderRow, createRowFunctio this.clearRowFunction = clearRowFunction; this.preRefreshFunction = preRefreshFunction; this.postRefreshFunction = postRefreshFunction; + this.preResizeFunction = preResizeFunction; // the list of row elements created in the table up to max viewable height plus SCROLL_ROWS rows for scrolling buffer this.elRows = []; @@ -248,7 +249,7 @@ ListView.prototype = { debugPrint("ListView.resize - no valid table body or table scroll element"); return; } - + this.preResizeFunction(); let prevScrollTop = this.elTableScroll.scrollTop; // take up available window space