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.

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

Upgrade to latest visjs Timeline #50

Closed
daattali opened this issue Mar 2, 2018 · 25 comments
Closed

Upgrade to latest visjs Timeline #50

daattali opened this issue Mar 2, 2018 · 25 comments

Comments

@daattali
Copy link
Owner

daattali commented Mar 2, 2018

Currently timevis uses visjs v4.16.1

It'd be nice to update to the most up to date version because they have new features. But there are some issues (and perhaps more, everything needs to be tested thoroughly to make sure nothing breaks):

@knbknb
Copy link

knbknb commented Jul 23, 2018

I have simply replaced timelines_files/vis-4.16.1/vis.min.js and timelines_files/vis-4.16.1/vis.min.css with those versions from timevis-4.21.zip.

It works for me.

Why did I update? Because v4.21 also has more events defined. I need the new onInitialDrawComplete Event which is not available in v4.16. (I don't know when it was added. I didn't find a mention of this event in the visjs release notes)

Just saying, maybe it helps someone who thinks about updating.

@daattali
Copy link
Owner Author

Yes it does work. However, there are several non-minor regression bugs in the new visjs timeline version, as I outline in the issue description. Until those are resolved, I would prefer to hold off on updating because these issues mean that many people who will upgrade to the new timevis version will have bugs.

visjs is really great but it's a bit of a shame that bugs don't get addressed and that documentation of new features is very lacking (I also ran into that problem of not knowing when new features got added because they aren't documented)

@daattali
Copy link
Owner Author

Merge #70 after upgrading

@strazto
Copy link
Contributor

strazto commented Feb 1, 2019

Are you planning on migrating to the timeline-plus project, when you do upgrade @daattali ?

I experimented with upgrading to the library myself, and found that it was doable but the maintainer's choice of object name (timeline) collided with timevis's timeline.

It's actively maintained, and seems a bit less bloated than the vis.js distro.

I use timevis for work and I find it really really useful, so if you're keen to see some fixes on timeline-plus before you're comfortable upgrading, let me know

@daattali
Copy link
Owner Author

daattali commented Feb 1, 2019

I will upgrade to whichever one, as long as the regression bugs I mentioned are fixed. I opened the same issues on timeline-plus repo, if they are fixed over there then yes I will look into switching. But I will not upgrade until the regression bugs are fixed.

@daattali
Copy link
Owner Author

Update: I again looked into updating to the latest version, but unfortunately it seems like the bugs I reported to visjs Timeline a few years ago are still unresolved and I cannot update until they are fixed.

@daattali daattali changed the title Upgrade to visjs 4.21 Upgrade to latest visjs Timeline Sep 11, 2020
@strazto

This comment has been minimized.

@daattali
Copy link
Owner Author

Interesting. Please do report back with any details (also, what do you mean by "hangs"? Is everything just a bit slower, or does it actually freeze? Or any input, or some specific compelx cases?)

@strazto

This comment has been minimized.

@strazto

This comment has been minimized.

@daattali
Copy link
Owner Author

Have you tested vs the javascript library , using purely just html+JS? That would tell us if its a timevis problem or if it's a problem with visjs which means you can stop investigating and pass it along to them

@strazto
Copy link
Contributor

strazto commented Oct 14, 2020

Oh, sorry to put you through the effort of trying to help me debug (I forgot to reply with my findings) this was a @matthewstrasiotto problem, I forgot to filter one of the inputs to the dataset, a bunch of items in nested groups with no corresponding parents.

The items weren't rendering because they didn't have valid group hierachies, but vis-timeline had to do a LOT of work before finally rendering

@D3SL
Copy link

D3SL commented May 3, 2021

Are these same few issues still holding back updating?

@daattali
Copy link
Owner Author

daattali commented May 3, 2021

Yes. I used to check every month consistently but I don't recall the last time I tried. You (or anyone else) is welcome to try to see if they're fixed :)

@strazto
Copy link
Contributor

strazto commented May 6, 2021

IIRC I fixed a majority of the issues in vis.js . I don't remember what was left. Personally I maintain a fork of timevis that uses the updated library, since I'd dealt with at least the problems i cared about https://github.com/matthewstrasiotto/timevis

@daattali
Copy link
Owner Author

daattali commented May 6, 2021

@matthewstrasiotto you have an open PR https://github.com/daattali/timevis/pull/77/files but as far as I can tell that PR doesn't include updating the version, I'd love a PR if you think the issues can be fixed

@ismirsehregal
Copy link

ismirsehregal commented Sep 2, 2021

Just to add another reason for updating:

With the current CRAN version the initial rendering of a timevis object is very slow if many items / groups are provided.

I tried using a start and end time in the options as suggested here - but it doesn't help.

It seems some relevant updates were included by now.

library(shiny)
library(timevis)

myItems <- seq_len(1500)

data <- data.frame(
  id = myItems,
  start = Sys.time()-myItems,
  content = paste0("Item_", myItems)
)

ui <- fluidPage(
  timevisOutput("myTimevis")
)

server <- function(input, output, session) {
  output$myTimevis <- renderTimevis(
    timevis(
      data,
      options = list(start = data$start[nrow(data)], end = data$start[nrow(data)-10])
    )
  )
}

shinyApp(ui, server)

@daattali
Copy link
Owner Author

daattali commented Sep 2, 2021

There are certainly many good reasons to update! And I would love to. But only once the regression issues are fixed.

@ismirsehregal
Copy link

The above makes your package unuseable regarding my usecase. What a pity - it looked really promising but cosidering the age of those regression issues it seems I'm flogging a dead horse. Will switch to plotly gantt charts then.

@D3SL
Copy link

D3SL commented Sep 2, 2021

The above makes your package unuseable regarding my usecase. What a pity - it looked really promising but cosidering the age of those regression issues it seems I'm flogging a dead horse. Will switch to plotly gantt charts then.

Not just yours, that's a crippling performance issue in general. To be honest I think the "regressions" range from trifling quirks to a minor annoyance at most while refusing to update on the other hand has led to profoundly crippling the R version by missing out on significant feature advancements, as well as severe performance issues like you detail.

But it's Daattali's package and his call in the end.

@daattali
Copy link
Owner Author

daattali commented Sep 9, 2021

I think a happy middleground would be to introduce a new function timevis2() that can use the updated library. I'll try that some time, or anyone else is welcomed to help with a PR. As little code as possible should be affected.

@daattali
Copy link
Owner Author

Took me an entire weekend because of a lot of non backwards compatible changes... but this is done 97fbd7b

timevis() has been upgraded to visjs 7.4.9. Feel free to test and let me know if there are any regression issues

@ismirsehregal
Copy link

Thanks for your work! Highly appreciated! I'll let you know as soon as I've tested the latest version.

@ismirsehregal
Copy link

It seems with te new version, the start and end options I'm setting in my above example are no longer taken into account.

@daattali
Copy link
Owner Author

Did that used to work previously, is it a regression bug? When I try that I see nothing show up - I think it's too many items. Anyway if you think it's a real bug, please open an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants