Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When user press ctrl+a combination selection will be correct #75

Closed
wants to merge 6 commits into from
Closed

When user press ctrl+a combination selection will be correct #75

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
800bae2
changed selection feature
YaroslavOvdii Mar 30, 2020
94bf669
made changes as codacy requested
YaroslavOvdii Mar 30, 2020
7c1f031
corrected lint errors
YaroslavOvdii Apr 1, 2020
3ad5ecc
made loader same as in all studio
YaroslavOvdii Apr 2, 2020
35a168d
remade solution for ctrla
YaroslavOvdii Apr 3, 2020
94fe80e
Merge branch 'master' into select-shortcat
YaroslavOvdii Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
321 changes: 100 additions & 221 deletions js/spreadsheet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,248 +67,131 @@ var spreadsheet = function(options) {
}
}

/**
* We user this method to determine where is closest data is lokated from the selected cell
*
* @param {Array} selectedCell - array of the coordinat of the selected cell reacived throw the hot.getSelected() method
*
* @returns {Object} - object of the directions where data is placed occording to the selected cell
*/
function closestData(selectedCell) {
selectedCell = selectedCell[0];
if (!Array.isArray(selectedCell)) {
console.error('We must pass an array of the cell coordinats to the closestData function. First element is cell' +
'row and second element is cell col. In this case script will act as if there was a value in the cell. ' +
'Value that was passed - ',
selectedCell);
return false;
}

var col = selectedCell[1];
var row = selectedCell[0];
var selectedCellData = hot.getDataAtCell(row, col)
// At this block we getting an index of the nearest cells from the selected cell
// If we selected first row it ID is 0 already and if we - 1 from it we will reacive an error in the hot.getDataAtCell() method
var top = row ? row - 1 : row;
var bottom = row + 1;
// Same as in top variable
var left = col ? col - 1 : col;
var right = col + 1;
// At this block we reacive the nearest cells value from the selected cell
var leftValue = hot.getDataAtCell(row, left);
var rightValue = hot.getDataAtCell(row, right);
var topValue = hot.getDataAtCell(top, col);
var bottomValue = hot.getDataAtCell(bottom, col);
var dataAt = {
left: false,
right: false,
top: false,
bottom: false,
all: false,
hasData: false
function getNearestCells(cellPosition) {
var leftShift = cellPosition.y - 1;
var topShift = cellPosition.x - 1;
var left = {
x: cellPosition.x,
y: leftShift < 0 ? cellPosition.y : leftShift
};
var right = {
x: cellPosition.x,
y: cellPosition.y + 1
};
var top = {
x: topShift < 0 ? cellPosition.x : topShift,
y: cellPosition.y
};
var bottom = {
x: cellPosition.x + 1,
y: cellPosition.y
};

// If there is a data in the selected cell we should select data releated to this cell
if (selectedCellData !== null) {
dataAt.hasData = true;
return dataAt;
}

// If no value near the selected cell we should select all table, also fires when we just load data source
// and clicked ctrl+a combination
if (leftValue === null && rightValue === null && topValue === null && bottomValue === null) {
dataAt.all = true;
return dataAt;
}

// Showing where is data position from the selected cell
dataAt.left = leftValue !== null;
dataAt.right = rightValue !== null;
dataAt.top = topValue !== null;
dataAt.bottom = bottomValue !== null;

return dataAt;
return {
left: left,
right: right,
top: top,
bottom: bottom
};
}

/**
* Method to get a coordinats which we need to select
*
* @param {Array} startAt - array of the selected coordinats
* @param {Object} moveTo - object that returned from closestData() function
* The function that surfs through the table data for detecting closest cells with data from the selected cell
*
* @returns {Array} - coordinats that needs to be selected. Example of the returned data: [[startRow, startCol, endRow, endCol]]
* @param {array} tableData - an array of table data received from the hot.getData()
* @param {object} cellPosition - an object of the cell coordinates with x and y keys
* @param {array} processedCells - an array of the cells we have already checked
* @returns {array} returns array of the scaned cells example array[rowIndex][cellIndex] = hasData true/false
*/
function coordinatsToSelect(startAt, moveTo) {
var firstCol, lastCol, firstRow, lastRow, allData;

// Returns array of the data from the table with handsontable API
allData = hot.getData();
startAt = startAt[0];

if (moveTo.left) {
// When data located on the left of the selected cell
lastCol = startAt[1];

// Looking for first col in the array of allData
// When we got a null value in the cell it means that we reached the range borders
for (var i = lastCol - 1; i >= 0; i--) {
if (allData[startAt[0]][i] === null) {
firstCol = i;
break;
}
}

firstCol = firstCol || 0;

// Looking for the first row in the array of allData
// When we got a null value in the cell it means that we reached the range borders
for (var i = startAt[0]; i >= 0; i--) {
if (allData[i][firstCol] === null) {
firstRow = i;
break;
}
}

firstRow = firstRow || 0;

// Looking for the last row in the array of allData
// When we got a null value in the cell it means that we reached the range borders
for (var i = firstRow; i < allData.length; i++) {
if (allData[i][firstCol] === null) {
lastRow = i;
break;
}
}

lastRow = _.max([lastRow - 1, 0]);
} else if (moveTo.right) {
// When data located on the right of the selected cell
firstCol = startAt[1];

for (var i = firstCol + 1; i < allData.length; i++) {
if (allData[startAt[0]][i] === null) {
lastCol = i - 1;
break;
}
}
function getDataCoordinates(tableData, cellPosition, processedCells) {
if (!processedCells) {
processedCells = [];
processedCells[cellPosition.x] = [];
processedCells[cellPosition.x][cellPosition.y] = true;
}

for (var i = startAt[0]; i > 0; i--) {
if (allData[i][lastCol] === null) {
firstRow = i ? i - 1 : i;
}
}
var cellsGroup = getNearestCells(cellPosition);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var cellsGroup = getNearestCells(cellPosition);
var nearestCells = getNearestCells(cellPosition);


firstRow = firstRow || 0;
cellsGroup['currentCell'] = cellPosition;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would there be a currentCell property? I don't see it in the getNearestCells() function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonytlwu
currentCell needed for when our selected cell is without a data on it row or col.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YaroslavOvdii Let's just add it into getNearestCells(). The currentCell property is 100% always added at this point, and it's good for the nearest cells to have a reference of which cell was the starting point.


for (var i = firstRow; i < allData.length; i++) {
if (allData[i][lastCol] === null) {
lastRow = i - 1;
break;
}
}
} else if (moveTo.top) {
// When data located on the top of the selected cell
lastRow = startAt[0];
for (var cell in cellsGroup) {
var hasData = tableData[cellsGroup[cell].x][cellsGroup[cell].y] !== null;
var processedRow = processedCells[cellsGroup[cell].x];

for (var i = lastRow - 1; i > 0; i--) {
if (allData[i][startAt[1]] === null) {
firstRow = i;
break;
}
if (!processedRow) {
processedCells[cellsGroup[cell].x] = [];
}

firstRow = firstRow || 0;
var processedCell = processedCells[cellsGroup[cell].x][cellsGroup[cell].y];

for (var i = startAt[1]; i > 0; i--) {
if (allData[firstRow][i] === null) {
firstCol = i ? i + 1 : i;
break;
}
if (!processedCell) {
processedCells[cellsGroup[cell].x][cellsGroup[cell].y] = false;
}

firstCol = firstCol || 0;

for (var i = firstCol; i < allData.length; i++) {
if (allData[firstRow][i] === null) {
lastCol = i - 1;
break;
}
}
} else if (moveTo.bottom) {
// When data located on the bottom of the selected cell
firstRow = startAt[0];
if (hasData && (!processedRow || !processedCell)) {
processedCells[cellsGroup[cell].x][cellsGroup[cell].y] = true;
getDataCoordinates(tableData, cellsGroup[cell], processedCells);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right in assuming that the process of selecting all cells is a recursive process, as it's indicated in this line?

If so, how efficient is it if we want to select all 100K rows and 100 columns?

Copy link
Contributor Author

@YaroslavOvdii YaroslavOvdii Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonytlwu You were right about large amount of data. This solution couldn't handle it. I've created a new PR with a new solution and tested it on a large data source 60k rows and it works fine.

I'll close this PR because it is not needed anymore.

}
}

for (var i = firstRow + 1; i < allData.length; i++) {
if (allData[i][startAt[1]] === null) {
lastRow = i - 1;
break;
}
}
return processedCells;
}

for (var i = startAt[1]; i > 0; i--) {
if (allData[lastRow][i] === null) {
firstCol = i + 1;
break;
/**
* The method that returns us a range that we should select in the table
*
* @param {array} processedCells array processed by getDataCoordinates function
*/
function getSelectionCoordinates(processedCells) {
var rowIndexes = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plural for index is indices.

Suggested change
var rowIndexes = [];
var rowIndices = [];

var colIndexes = [];

processedCells.forEach(function(row, rowIndex) {
tonytlwu marked this conversation as resolved.
Show resolved Hide resolved
row.forEach(function(cell, colIndex) {
if (cell) {
rowIndexes.push(rowIndex);
colIndexes.push(colIndex);
}
}

firstCol = firstCol || 0;
});
});

for (var i = firstCol; i < allData.length; i++) {
if (allData[lastRow][i] === null) {
lastCol = i - 1;
break;
}
}
} else if (moveTo.hasData) {
// When selected cell has data in it
if (startAt[1] === 0) {
firstCol = 0;
} else {
for (var i = startAt[1]; i > 0; i--) {
if (allData[startAt[0]][i] === null) {
firstCol = i + 1;
break;
}
}
var firstRow = Math.min.apply(null, rowIndexes);
var firstCol = Math.min.apply(null, colIndexes);
var lastRow = Math.max.apply(null, rowIndexes);
var lastCol = Math.max.apply(null, colIndexes);

firstCol = firstCol || 0;
}
return [[firstRow, firstCol, lastRow, lastCol]];
}

for (var i = firstCol; i < allData.length; i++) {
if (allData[startAt[0]][i] === null) {
lastCol = i - 1;
break;
}
}
/**
* The method that decides do we need to select a range or select all data
*
* @param {array} selectionPosition - an array of the selected cell coordinates received from hot.getSelected()
* @returns {array} range of the data that we need to select
*/
function getSelectionRange(selectionPosition) {
var selectedPosition = {
x: selectionPosition[0][0],
y: selectionPosition[0][1]
};
var dataCoordinates = getDataCoordinates(hot.getData(), selectedPosition);
var selectedCells = 0;

if (startAt[0] === 0) {
firstRow = startAt[0];
} else {
for (var i = startAt[0]; i > 0; i--) {
if (allData[i][firstCol] === null) {
firstRow = i + 1;
break;
}
dataCoordinates.forEach(function(row) {
row.forEach(function(cell) {
if (cell) {
selectedCells++;
}
});
});

firstRow = firstRow || 0;
}

for (var i = firstRow; i < allData.length; i++) {
if (allData[i][firstCol] === null) {
lastRow = i - 1;
break;
}
}
} else if (moveTo.all) {
// When selected cell doesn't have a data in it and no data in cells around it
if (selectedCells <= 1) {
return false;
}

return [
[firstRow, firstCol, lastRow, lastCol]
];
var selectionCoords = getSelectionCoordinates(dataCoordinates);

return selectionCoords;
}

/**
Expand Down Expand Up @@ -496,17 +379,13 @@ var spreadsheet = function(options) {

if ((event.ctrlKey || event.metaKey) && event.keyCode === 65 ) {
var selectedCell = hot.getSelected();
var whereToLook = closestData(selectedCell);
var selectedRange = coordinatsToSelect(selectedCell, whereToLook);
var selectedRange = getSelectionRange(selectedCell);

if (!selectedRange) {
return;
}
event.stopImmediatePropagation();

var cols = getColumns().filter(function(column) {
return column;
}).length;

event.stopImmediatePropagation();
hot.deselectCell();
hot.selectCells(selectedRange, false, false);
return false;
Expand Down