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

fix(type-coercion): use Data Pipes #259

Merged
merged 4 commits into from
Jan 25, 2020
Merged

fix(type-coercion): use Data Pipes #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95793b7
fix(type-coercion): use Data Pipes
Thomaash Jan 19, 2020
22c676f
docs(type-coercion): add more documentation
Thomaash Jan 21, 2020
1089835
fix(type-coersion): prevent potential NPE
Thomaash Jan 22, 2020
2146475
Merge branch 'master' into issues/254
Thomaash Jan 25, 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
17 changes: 8 additions & 9 deletions lib/timeline/Graph2d.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

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

Why was me.itemOptions removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I found no place where it would be altered and it just set the same type coercion as was used everywhere anyway.

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
Open in desktop
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