-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Visualise changes over time #175
base: master
Are you sure you want to change the base?
Conversation
Also remove unused imports
Also converts data to array of objects if only a single object
Also fixes default classes for nodes and removed redundant code
src/utils/bods.js
Outdated
|
||
console.log(duplicates); | ||
} else { | ||
// get all statements with statementID values in replacesStatements array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the deal with this? Does this PR add rendering change over time just for BODS 0.4? If so probably best to be explicit about that in this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this PR add rendering change over time just for BODS 0.4?
Yes, the specification states that only 0.4 data renders an interactive slider. I'll update the comment (and remove the console.log
).
Okay, had to hand roll my own BODS 0.3 test data (because examples are so broken): https://gist.github.com/radix0000/5e612d582b41a8aca5bfff4d5e396bf8 So doesn't show change over time with BODS 0.3 (which is fine if that is the intention), but does go into "change-over-time mode" with slider displayed if there is a replacesStatement property present (you just can't slide the slider, and all statements displayed), but latest time it shows conflicts with the fact all statements are shown, which is a bit confusing. So if it is intended that doesn't support change-over-time for 0.1-0.3 then recon that slider should always be hidden regardless of whether there is a replacesStatement property or not. |
@radix0000 Thank you. @kd-ods and I discussed this at length and came to the decision that 0.4 data renders and interactive slider, whereas pre-0.4 data with a change-over-time element renders an inactive slider to indicate that there is change-over-time data, but not render the changes themselves. The slider is hidden only if there is no change-over-time data detected. |
Note, this update introduces a bug with the arrowheads which will be addressed in a future issue (see #174).