Skip to content

Commit

Permalink
fix(type-coercion): use Data Pipes (#259)
Browse files Browse the repository at this point in the history 
* fix(type-coercion): use Data Pipes

Switch from the deprecated type property of Data Set to Data Pipes. This
solves some or maybe all issues from #254.

PS: There is a bug in Vis Data that prevents this from working see
visjs/vis-data/pull/78.

* docs(type-coercion): add more documentation

* fix(type-coersion): prevent potential NPE
  • Loading branch information
@Thomaash @yotamberk
Thomaash authored and yotamberk committed Jan 25, 2020
1 parent dad6123 commit 109c121
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 63 deletions.
17 changes: 8 additions & 9 deletions lib/timeline/Graph2d.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import moment from '../module/moment';
import util from '../util';
import util, { typeCoerceDataSet } from '../util';
import { DataSet } from 'vis-data';
import { DataView } from 'vis-data';
import Range from './Range';
Expand Down Expand Up @@ -193,21 +193,20 @@ Graph2d.prototype.setItems = function(items) {
newDataSet = null;
}
else if (items instanceof DataSet || items instanceof DataView) {
newDataSet = items;
newDataSet = typeCoerceDataSet(items);
}
else {
// turn an array into a dataset
newDataSet = new DataSet(items, {
type: {
start: 'Date',
end: 'Date'
}
});
newDataSet = typeCoerceDataSet(new DataSet(items));
}

// set items
if (this.itemsData) {
// stop maintaining a coerced version of the old data set
this.itemsData.dispose()
}
this.itemsData = newDataSet;
this.linegraph && this.linegraph.setItems(newDataSet);
this.linegraph && this.linegraph.setItems(newDataSet != null ? newDataSet.rawDS : null);

if (initialLoad) {
if (this.options.start != undefined || this.options.end != undefined) {
Expand Down
38 changes: 15 additions & 23 deletions lib/timeline/Timeline.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import moment from '../module/moment';
import util from '../util';
import util, { typeCoerceDataSet } from '../util';
import { DataSet } from 'vis-data';
import { DataView } from 'vis-data';
import Range from './Range';
Expand Down Expand Up @@ -289,9 +289,9 @@ export default class Timeline extends Core {
const itemsData = this.itemsData;
if (itemsData) {
const selection = this.getSelection();
this.setItems(null); // remove all
this.setItems(itemsData); // add all
this.setSelection(selection); // restore selection
this.setItems(null); // remove all
this.setItems(itemsData.rawDS); // add all
this.setSelection(selection); // restore selection
}
}
}
Expand All @@ -310,21 +310,20 @@ export default class Timeline extends Core {
newDataSet = null;
}
else if (items instanceof DataSet || items instanceof DataView) {
newDataSet = items;
newDataSet = typeCoerceDataSet(items);
}
else {
// turn an array into a dataset
newDataSet = new DataSet(items, {
type: {
start: 'Date',
end: 'Date'
}
});
newDataSet = typeCoerceDataSet(new DataSet(items));
}

// set items
if (this.itemsData) {
// stop maintaining a coerced version of the old data set
this.itemsData.dispose();
}
this.itemsData = newDataSet;
this.itemSet && this.itemSet.setItems(newDataSet);
this.itemSet && this.itemSet.setItems(newDataSet != null ? newDataSet.rawDS : null);
}

/**
Expand Down Expand Up @@ -420,12 +419,7 @@ export default class Timeline extends Core {
const ids = Array.isArray(id) ? id : [id];

// get the specified item(s)
const itemsData = this.itemsData.getDataSet().get(ids, {
type: {
start: 'Date',
end: 'Date'
}
});
const itemsData = this.itemsData.get(ids);

// calculate minimum start and maximum end of specified items
let start = null;
Expand Down Expand Up @@ -534,8 +528,7 @@ export default class Timeline extends Core {
const animation = (options && options.animation !== undefined) ? options.animation : true;
let range;

const dataset = this.itemsData && this.itemsData.getDataSet();
if (dataset.length === 1 && dataset.get()[0].end === undefined) {
if (this.itemsData.length === 1 && this.itemsData.get()[0].end === undefined) {
// a single item -> don't fit, just show a range around the item from -4 to +3 days
range = this.getDataRange();
this.moveTo(range.min.valueOf(), {animation}, callback);
Expand Down Expand Up @@ -646,9 +639,8 @@ export default class Timeline extends Core {
let min = null;
let max = null;

const dataset = this.itemsData && this.itemsData.getDataSet();
if (dataset) {
dataset.forEach(item => {
if (this.itemsData) {
this.itemsData.forEach(item => {
const start = util.convert(item.start, 'Date').valueOf();
const end = util.convert(item.end != undefined ? item.end : item.start, 'Date').valueOf();
if (min === null || start < min) {
Expand Down
44 changes: 20 additions & 24 deletions lib/timeline/component/ItemSet.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Hammer from '../../module/hammer';
import util from '../../util';
import util, { typeCoerceDataSet } from '../../util';
import { DataSet } from 'vis-data';
import { DataView } from 'vis-data';
import TimeStep from '../TimeStep';
Expand Down Expand Up @@ -130,12 +130,7 @@ class ItemSet extends Component {
this.options = util.extend({}, this.defaultOptions);
this.options.rtl = options.rtl;
this.options.onTimeout = options.onTimeout;

// options for getting items from the DataSet with the correct type
this.itemOptions = {
type: {start: 'Date', end: 'Date'}
};


this.conversion = {
toScreen: body.util.toScreen,
toTime: body.util.toTime
Expand Down Expand Up @@ -963,7 +958,7 @@ class ItemSet extends Component {
this.itemsData = null;
}
else if (items instanceof DataSet || items instanceof DataView) {
this.itemsData = items;
this.itemsData = typeCoerceDataSet(items);
}
else {
throw new TypeError('Data must be an instance of DataSet or DataView');
Expand All @@ -975,6 +970,9 @@ class ItemSet extends Component {
oldItemsData.off(event, callback);
});

// stop maintaining a coerced version of the old data set
oldItemsData.dispose()

// remove all drawn items
ids = oldItemsData.getIds();
this._onRemove(ids);
Expand Down Expand Up @@ -1003,7 +1001,7 @@ class ItemSet extends Component {
* @returns {vis.DataSet | null}
*/
getItems() {
return this.itemsData;
return this.itemsData != null ? this.itemsData.rawDS : null;
}

/**
Expand Down Expand Up @@ -1097,15 +1095,14 @@ class ItemSet extends Component {
*/
removeItem(id) {
const item = this.itemsData.get(id);
const dataset = this.itemsData.getDataSet();

if (item) {
// confirm deletion
this.options.onRemove(item, item => {
if (item) {
// remove by id here, it is possible that an item has no id defined
// itself, so better not delete by the item itself
dataset.remove(id);
this.itemsData.remove(id);
}
});
}
Expand Down Expand Up @@ -1146,7 +1143,7 @@ class ItemSet extends Component {
const me = this;

ids.forEach(id => {
const itemData = me.itemsData.get(id, me.itemOptions);
const itemData = me.itemsData.get(id);
let item = me.items[id];
const type = itemData ? me._getType(itemData) : null;

Expand Down Expand Up @@ -1610,7 +1607,7 @@ class ItemSet extends Component {
};

const id = util.randomUUID();
itemData[this.itemsData._idProp] = id;
itemData[this.itemsData.idProp] = id;

const group = this.groupFromTarget(event);
if (group) {
Expand Down Expand Up @@ -1823,20 +1820,19 @@ class ItemSet extends Component {
event.stopPropagation();

const me = this;
const dataset = this.itemsData.getDataSet();
const itemProps = this.touchParams.itemProps;
this.touchParams.itemProps = null;

itemProps.forEach(props => {
const id = props.item.id;
const exists = me.itemsData.get(id, me.itemOptions) != null;
const exists = me.itemsData.get(id) != null;

if (!exists) {
// add a new item
me.options.onAdd(props.item.data, itemData => {
me._removeItem(props.item); // remove temporary item
if (itemData) {
me.itemsData.getDataSet().add(itemData);
me.itemsData.add(itemData);
}

// force re-stacking of all items next redraw
Expand All @@ -1849,8 +1845,8 @@ class ItemSet extends Component {
me.options.onMove(itemData, itemData => {
if (itemData) {
// apply changes
itemData[dataset._idProp] = id; // ensure the item contains its id (can be undefined)
dataset.update(itemData);
itemData[this.itemsData.idProp] = id; // ensure the item contains its id (can be undefined)
this.itemsData.update(itemData);
}
else {
// restore original values
Expand Down Expand Up @@ -2304,7 +2300,7 @@ class ItemSet extends Component {
const itemData = me.itemsData.get(item.id); // get a clone of the data from the dataset
this.options.onUpdate(itemData, itemData => {
if (itemData) {
me.itemsData.getDataSet().update(itemData);
me.itemsData.update(itemData);
}
});
}
Expand Down Expand Up @@ -2348,7 +2344,7 @@ class ItemSet extends Component {
newItemData.content = newItemData.content ? newItemData.content : 'new item';
newItemData.start = newItemData.start ? newItemData.start : (snap ? snap(start, scale, step) : start);
newItemData.type = newItemData.type || 'box';
newItemData[this.itemsData._idProp] = newItemData.id || util.randomUUID();
newItemData[this.itemsData.idProp] = newItemData.id || util.randomUUID();

if (newItemData.type == 'range' && !newItemData.end) {
end = this.body.util.toTime(x + this.props.width / 5);
Expand All @@ -2359,7 +2355,7 @@ class ItemSet extends Component {
start: snap ? snap(start, scale, step) : start,
content: 'new item'
};
newItemData[this.itemsData._idProp] = util.randomUUID();
newItemData[this.itemsData.idProp] = util.randomUUID();

// when default type is a range, add a default end date to the new item
if (this.options.type === 'range') {
Expand All @@ -2377,7 +2373,7 @@ class ItemSet extends Component {
newItemData = this._cloneItemData(newItemData); // convert start and end to the correct type
this.options.onAdd(newItemData, item => {
if (item) {
me.itemsData.getDataSet().add(item);
me.itemsData.add(item);
if (event.type == 'drop') {
me.setSelection([item.id]);
}
Expand Down Expand Up @@ -2421,7 +2417,7 @@ class ItemSet extends Component {
if (!this.options.multiselectPerGroup || lastSelectedGroup == undefined || lastSelectedGroup == itemGroup) {
selection.push(item.id);
}
const range = ItemSet._getItemRange(this.itemsData.get(selection, this.itemOptions));
const range = ItemSet._getItemRange(this.itemsData.get(selection));

if (!this.options.multiselectPerGroup || lastSelectedGroup == itemGroup) {
// select all items within the selection range
Expand Down Expand Up @@ -2609,7 +2605,7 @@ class ItemSet extends Component {

if (!type) {
// convert start and end date to the type (Date, Moment, ...) configured in the DataSet
type = this.itemsData.getDataSet()._options.type;
type = this.itemsData.type;
}

if (clone.start != undefined) {
Expand Down
9 changes: 6 additions & 3 deletions lib/timeline/component/LineGraph.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import util from '../../util';
import util, { typeCoerceDataSet } from '../../util';
import * as DOMutil from '../../DOMutil';
import { DataSet } from 'vis-data';
import { DataView } from 'vis-data';
Expand Down Expand Up @@ -254,7 +254,7 @@ LineGraph.prototype.setItems = function (items) {
this.itemsData = null;
}
else if (items instanceof DataSet || items instanceof DataView) {
this.itemsData = items;
this.itemsData = typeCoerceDataSet(items);
}
else {
throw new TypeError('Data must be an instance of DataSet or DataView');
Expand All @@ -266,6 +266,9 @@ LineGraph.prototype.setItems = function (items) {
oldItemsData.off(event, callback);
});

// stop maintaining a coerced version of the old data set
oldItemsData.dispose()

// remove all drawn items
ids = oldItemsData.getIds();
this._onRemove(ids);
Expand Down Expand Up @@ -433,7 +436,7 @@ LineGraph.prototype._updateAllGroupData = function (ids, groupIds) {
if (this.itemsData != null) {
var groupsContent = {};
var items = this.itemsData.get();
var fieldId = this.itemsData._idProp;
var fieldId = this.itemsData.idProp;
var idMap = {};
if (ids){
ids.map(function (id) {
Expand Down
Loading

0 comments on commit 109c121

Please sign in to comment.