Skip to content

Commit

Permalink
WIP: First changes - checking whether returning null for index beyo…
Browse files Browse the repository at this point in the history
…nd a table boundaries (the `propToCol` method) will work properly #7031
  • Loading branch information
wszymanski committed Jul 9, 2020
1 parent e2eab29 commit 47daf47
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
13 changes: 9 additions & 4 deletions src/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,8 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
changes.splice(i, 1);
} else {
const [row, prop, , newValue] = changes[i];
const col = datamap.propToCol(prop);
const propToColResult = datamap.propToCol(prop);
const col = propToColResult !== null ? propToColResult : prop;
const cellProperties = instance.getCellMeta(row, col);

if (cellProperties.type === 'numeric' && typeof newValue === 'string' && isNumericData(newValue)) {
Expand Down Expand Up @@ -1181,8 +1182,14 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal

if (instance.dataType === 'array' && (!tableMeta.columns || tableMeta.columns.length === 0) &&
tableMeta.allowInsertColumn) {
while (datamap.propToCol(changes[i][1]) > instance.countCols() - 1) {
const prop = changes[i][1];
let propToColResult = datamap.propToCol(changes[i][1]);
let column = propToColResult !== null ? propToColResult : prop;

while (column > instance.countCols() - 1) {
const numberOfCreatedColumns = datamap.createCol(void 0, void 0, source);
propToColResult = datamap.propToCol(changes[i][1]);
column = propToColResult !== null ? propToColResult : prop;

if (numberOfCreatedColumns >= 1) {
metaManager.createColumn(null, numberOfCreatedColumns);
Expand Down Expand Up @@ -1344,8 +1351,6 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
]);
}

console.log(JSON.parse(JSON.stringify(changes)));

if (!changeSource && typeof row === 'object') {
changeSource = column;
}
Expand Down
12 changes: 3 additions & 9 deletions src/dataMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class DataMap {
* Returns property name that corresponds with the given column index.
*
* @param {string|number} column Visual column index or another passed argument.
* @returns {string|number} Column property, physical column index or passed argument.
* @returns {string|number|null} Column property, physical column index or passed argument.
*/
colToProp(column) {
// TODO: Should it work? Please, look at the test:
Expand Down Expand Up @@ -219,7 +219,7 @@ class DataMap {
* Translates property into visual column index.
*
* @param {string|number} prop Column property which may be also a physical column index.
* @returns {string|number} Visual column index or passed argument.
* @returns {string|number|null} Visual column index or passed argument.
*/
propToCol(prop) {
const cachedPhysicalIndex = this.propToColCache.get(prop);
Expand All @@ -229,13 +229,7 @@ class DataMap {
}

// Property may be a physical column index.
const visualColumn = this.instance.toVisualColumn(prop);

if (visualColumn === null) {
return prop;
}

return visualColumn;
return this.instance.toVisualColumn(prop);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/editors/dropdownEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ class DropdownEditor extends AutocompleteEditor {
}

Hooks.getSingleton().add('beforeValidate', function(value, row, col) {
const cellMeta = this.getCellMeta(row, this.propToCol(col));
const propToColResult = this.propToCol(col);
const column = propToColResult !== null ? propToColResult : col;
const cellMeta = this.getCellMeta(row, column);

if (cellMeta.editor === DropdownEditor) {
if (cellMeta.strict === void 0) {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/core/propToCol.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('Core.propToCol', () => {
hot.columnIndexMapper.setIndexesSequence([4, 3, 2, 1, 0]);

expect(propToCol(0)).toBe(4);
expect(propToCol(10)).toBe(10); // I'm not sure if this should return result like that by design.
expect(propToCol(10)).toBe(null);
});

it('should return proper value after calling the function when columns was reorganized (data is array of objects)', () => {
Expand All @@ -51,6 +51,6 @@ describe('Core.propToCol', () => {

expect(propToCol('id')).toBe(3);
expect(propToCol(0)).toBe(3);
expect(propToCol(10)).toBe(10); // I'm not sure if this should return result like that by design.
expect(propToCol(10)).toBe(null);
});
});

0 comments on commit 47daf47

Please sign in to comment.