Skip to content
This repository has been archived by the owner on Oct 30, 2021. It is now read-only.

No bubbles displayed if only start date exist. #92

Merged
merged 13 commits into from
Jan 25, 2017
Merged

No bubbles displayed if only start date exist. #92

merged 13 commits into from
Jan 25, 2017

Conversation

sergibondarenko
Copy link
Contributor

@sergibondarenko sergibondarenko commented Dec 19, 2016

Issue #89
I added the dots by manually manipulating HTML and style. Now it looks exactly like it is in our docs:
screenshot from 2016-12-19 12-16-20

I haven't found a way to do it purely with vis.js. The library doesn't display relations to the X axis when e.type: 'point' is used as an event property. In short words, I got the visualization similar to this http://visjs.org/examples/timeline/items/pointItems.html

Other examples:
http://visjs.org/examples/timeline/items/backgroundAreasWithGroups.html
http://visjs.org/examples/timeline/dataHandling/dataSerialization.html
http://visjs.org/examples/timeline/dataHandling/loadExternalData.html
http://visjs.org/examples/timeline/editing/editingItems.html

Seems like e.type: point is designed to not place relations to the X axis.
I can go and fix it in the library.

@scampi
Copy link
Contributor

scampi commented Dec 20, 2016

@SergeyBondarenko please rebase. This PR has changes about the multivalued PR that shouldn't be here.

@scampi
Copy link
Contributor

scampi commented Dec 20, 2016

@SergeyBondarenko you can propose a PR to the vis lib if you want ;o)

@sergibondarenko
Copy link
Contributor Author

sergibondarenko commented Dec 20, 2016

@scampi Yes, I can, I wrote it above.
Bubbles break your first label emphasize feature. We can draw a border around label box to emphasize.

@scampi
Copy link
Contributor

scampi commented Dec 20, 2016

Bubbles break your first label emphasize feature. We can draw a border around label box to emphasize.

good idea. BTW this was @nreese feature ;o)

@sergibondarenko sergibondarenko changed the title WIP. No bubbles displayed if only start date exist. No bubbles displayed if only start date exist. Dec 28, 2016
@sergibondarenko
Copy link
Contributor Author

sergibondarenko commented Dec 28, 2016

I got "play with CSS" suggestion from one of the vis contributors. almende/vis#2508 (comment)
As I actually did. So maybe it is good to take this way now.

@sergibondarenko sergibondarenko changed the title No bubbles displayed if only start date exist. WIP. No bubbles displayed if only start date exist. Dec 28, 2016
@sergibondarenko
Copy link
Contributor Author

sergibondarenko commented Dec 28, 2016

I've fixed the Emphasize feature. Plus now I use a custom styled div to represent point instead of vis-dot, to be able to change its color.

screenshot from 2016-12-28 12-42-42

@sergibondarenko sergibondarenko changed the title WIP. No bubbles displayed if only start date exist. No bubbles displayed if only start date exist. Dec 28, 2016
@szydan
Copy link
Contributor

szydan commented Jan 5, 2017

rebased on master

style = `border-style: none; background-color: #fff; color: ${groupColor}; border-color: ${groupColor}`;
const divregex = /(<div.*>)(.*)(<\/div>)/g;
const contentDivParts = divregex.exec(content);
const pointDot = '<div style="position:relative;padding:0;border-width:4px;border-style:solid;' +
Copy link
Contributor

Choose a reason for hiding this comment

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

create a class in the less file with this style.

const contentDivParts = divregex.exec(content);
const pointDot = '<div style="position:relative;padding:0;border-width:4px;border-style:solid;' +
'border-radius:4px;float:left;margin-top:6px;margin-right:4px;border-color:' + groupColor + '"></div>';
const labelDiv = '<div style="margin-left: 15px">' + contentDivParts[2] + '</div>';
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

// force vis box look like vis point
style = `border-style: none; background-color: #fff; color: ${groupColor}; border-color: ${groupColor}`;
const divregex = /(<div.*>)(.*)(<\/div>)/g;
const contentDivParts = divregex.exec(content);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of this, you can have an angular template where you would use ng-class to set the appropriate class. Then you would compile that template to get the final HTML. See tooltip of the tilemap vis in kibana for some example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you talking about line 250? But this variable is assigned as property to the e object below to form the visualization items.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, ok, you are talking about stuff below

Copy link
Contributor

@scampi scampi Jan 20, 2017

Choose a reason for hiding this comment

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

yes ;o) I put the comment here because this way you wouldn't have any HTML code in that file and so no need for the regexp

let content = $compile(itemTemplate)($itemscope)[0].innerHTML;
console.log('INNER HTML:');

const content = timelineHelper.createItemTemplate(i, itemDict);
console.log(content);

let style = `background-color: ${groupColor}; color: #fff;`;
Copy link
Contributor Author

@sergibondarenko sergibondarenko Jan 24, 2017

Choose a reason for hiding this comment

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

@szydan Seems I can't deprecate this style variable because I need to change the groupColor for every group.

endFieldValue: endFieldValue
};

const content = timelineHelper.createItemTemplate(i, itemDict);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you using "i" if not please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I do it in the template building method. But I'll look, maybe I can refactor.

Copy link
Contributor Author

@sergibondarenko sergibondarenko Jan 24, 2017

Choose a reason for hiding this comment

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

I got rid of the i variable in the template method.

@@ -1,3 +1,4 @@
/* global angular */
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isn't needed anymore, I'll delete it.

@@ -7,6 +7,16 @@ define(function (require) {
function TimelineHelper() {
}

TimelineHelper.prototype.createItemTemplate = function (itemDict) {
return '<div title="index: ' + itemDict.indexId + ', startField: ' + itemDict.startField +
Copy link
Contributor

Choose a reason for hiding this comment

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

could you try using lodash template ? https://lodash.com/docs/3.10.1#template
maybe it will improve readability

@sergibondarenko
Copy link
Contributor Author

jenkins test it

(!itemDict.endFieldValue || itemDict.startValue === itemDict.endFieldValue
? '<div class="kibi-tl-label-item">' + itemDict.labelValue + '</div>' : itemDict.labelValue) +
(itemDict.useHighlight ? '<p class="tiny-txt">' + itemDict.highlight + '</p>' : '') + '</div>';
let endfield = '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scampi how do you find my readability code refactoring?

@szydan szydan merged commit 9f76a30 into master Jan 25, 2017
@szydan szydan deleted the issue-89 branch January 25, 2017 09:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants