Preliminary commit for vertical timeline ranged events

[enigmartyr]

In a working state, can you see if you have any comments? Things to note so far:

1. You originally had the note, "TODO: Stop Propagation: don't close timeline-card when clicked." I'm not certain what your intention was but the current behavior is that clicking a note leaves any thumbnail and title visible but hides the body, as well as hiding any events encapsulated by a time spanning event.

2. Currently nested spanning events are supported but having a spanning event that begins inside another but ends outside of it is undefined behavior

3. I'm no color theorist and find the green color I randomly plugged to be quite jarring. Additionally, I think nested spanning events would benefit from having differently colored widgets

[enigmartyr]

I should add, the issue surrounding point #2 is purely graphical

[seanlowe]

Looks really good from what I've read so far. A couple thoughts:

point 1: IIRC, I added that line when I was implementing PR 58 from the original plugin. He had that comment in there so I just copied it over. I'm not certain what he truly meant by it. Honestly we can probably just delete that whole comment.

point 2: we could try something like how we determine which side to render the note on, left/right. For the first span, render the oval with some lines slanted one direction, then flip flop them every other time. Granted, this assumes that there won't be more than 1 nested time range within a time range and that could potentially not be true. Thoughts?

point 3: I agree the green is a little jarring, not sure yet on how to make that better but we can tinker around with it.

Could you provide your test events/files so I can recreate your images?

[enigmartyr]

test.zip here are the test notes

[seanlowe]

I pulled down your branch and was looking through it a little deeper. I had this timeline already rendered.

A couple of things I noticed:

• any event which has an end date (regardless of type) will show the time range instead of points
  • type point should definitely not show range.. perhaps that's a check I can add. If type === point, just ignore any end date.
  • type box is the default so I'm not sure if I would want that to ever show the range indicator or not but I'm leaning not
• there's some sorting shenanigans with events that start with a 0 for some reason
  • this isn't something you caused but it is something that is visible simply due to the fact that now the end date is displayed. I don't expect you to fix this. I can push up some code to your branch or I can make a PR into your branch that then will follow into this PR (up to you)
• I would prefer if we can find some way to show map the time to the centerline by percentage somehow.. I'll have to think on that one. In the picture I have an event that runs 550-632 and another that runs 600-700 but both events "end" visually where the 700 ends
  • I say this should be relegated to another issue and tackling it once we get the base functionality implemented, as it doesn't hurt anyone right now. consider this a "nice-to-have"

Here's the notes I used for this picture: ancients.zip

[enigmartyr]

First matter should be fixed, now it only bothers with the duration if the type is explicitly background or range.

As for the second point, I was noncommittal regarding how exactly I compare dates. I didn't have confidence in simply using the > operator with the start and end dates passed in but it seemed to be working. I also tried comparing the outputs of Date.parse() and normalizeDate() but that seemed to be more code than necessary since just checking the raw date seemed to be working.

I'd say try changing the date check in const lengthy and the values passed into [start, end, t].sort() to the output of normalizeDate()

[enigmartyr]

As for the third point, we could perhaps draw a dotted lined from the end date tot he number line that is rendered beneath everything else, or perhaps not even a dotted line since that isn't necessarily the prettiest solution. But in the same way that the notes have an arrow that points at the timeline, the end note can be tied in somehow

[enigmartyr]

I could reinstate the arrow and then instead of drawing a white line segment I can instead draw an L shape that connects to the arrow. I'll see about doing that, it'll probably look good

[enigmartyr]

is this better?

[enigmartyr]

or maybe a combination of this style and the last to differentiate between range and background events? I personally prefer the new style

[enigmartyr]

I think this style has the added benefit that time spanning events that dont contain other events look better

[seanlowe]

This is looking really good! I won't have a ton of time today to look at it until after working hours, but I'll be sure to give a look this evening.

[enigmartyr]

I didn't realize notes with the same start date get combined. knowing that now, I made it so that notes that share a start date but have different end dates can display both end dates. Caveat, there's currently no distinguishing mark on the timeline widget. I think the code merits a refactoring now, when I'm in a better state of mind I'll try cleaning it up

[seanlowe]

I had been considering it for a while but I think this is a perfect opportunity to break the actual timeline functions out into their own files.

I say we take the buildVerticalTimeline and buildHorizontalTimeline and move them into their own individual files. perhaps under src/timelines/vertical.ts and src/timelines/horizontal.ts so that the logic can be a bit more separated and easier to digest / debug.

But hey, you are kicking some serious butt with this PR. I really appreciate how invested you are in this. It's awesome to see!

[enigmartyr]

I'll revisit this in a few day, have a lot of appointments in the meanwhile

[seanlowe]

No worries. I've changed this to point to a feature branch instead of main so that I could merge this PR and then we could both contribute to changes on it moving forward, before we merge it to main. I'll create a new PR to main based off the new branch: vertical-timeline-spans

[seanlowe]

If I understand how GitHub treats contributors, you should be able to push to the vertical-timeline-spans branch. If you can't, please let me know and I'll figure out how to make it so you can. I've opened a new PR (#57) to continue the work you've started here.

Phenomenal job, mate. 👍🏻