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

Multiple Issues when running inside JupyterLab #110

Closed
rickmcgeer opened this issue Nov 28, 2022 · 8 comments
Closed

Multiple Issues when running inside JupyterLab #110

rickmcgeer opened this issue Nov 28, 2022 · 8 comments
Assignees

Comments

@rickmcgeer
Copy link
Contributor

  1. Sometimes the Google charts package doesn't load $world.get('dashboard').viewModel.gViz is undefined
  2. When it does load, the charts are properly loaded in the tables but are not drawn
  3. await $world.get('dashboard').viewModel.drawAllCharts() does draw the charts, but they are all at position (0, 0)
  4. When I move the charts by hand (Halo still works) I can see they're all there and everything works fine.
  5. This doesn't happen when I'm looking at the frozen dashboard standalone https://matt.engagelively.com/users/rick/published/studio/index.html. It's only when I instantiate inside JupyterLab
  6. I'm mystified. Assigning to @merryman to see if he has any ideas.
  7. This may have started happening when (at @linusha's suggestion, to fix a different bug) I set renderViaCSS on top bar to false. Will reset to true to check this.
@merryman
Copy link
Collaborator

merryman commented Nov 28, 2022

I think the first point already is an untenable situation. It can't be that we need to essentially busy wait on a global variable to be populated. There needs to be a more canonical way to ensure that google charts is loaded, or some kind of promise we can reliably wait on.

edit: Appears like google killed Support for charts two months ago: google/charts#798. Tbh. I don't know how much more work we should be putting towards google charts anymore.

@rickmcgeer
Copy link
Contributor Author

I'd love to dump Google Charts. FTM, since the project is no longer maintained, the simple solution is for Google to open-source it and let us download the damn thing through NPM, like any other package.
That said, the issue is doing a chart editor. I don't think it's that hard, a month, tops. But I don't want to wait a month to do the new release. So I'd like us to put in what I hope is a couple days' effort to get this into a maintainable, shippable state, do the release, then work damned hard on the chart editor and cut over to Chartjs/Leaflet/Cytoscape.

@rickmcgeer
Copy link
Contributor Author

Incidentally, I set renderViaCSS to true and the problem continues. The next experiment is to try this in a vanilla iframe to see if that is the problem.
My guess is that it is due to some strange effect with the Jupyter proxy...
Note that even when Google charts loads:

  • The Chart Morph position is never updated from (0, 0)
  • The charts don't do an initial draw

So my guess is that there is a timing issue that is triggered by the multiple timeouts, and as a result a ton of async code isn't getting called. I'll put in a test for those next.

@rickmcgeer
Copy link
Contributor Author

OK, I think I have a lead. I put in console.log messages when the morphic properties for a chart are assigned, and when the chart is first drawn. They didn't appear, and a message saying gViz is null DID appear. When I checked gViz in the console, it was there, and then I reloaded the dashboard using $world.get('dashboard').viewModel.loadTestDashboard('drilldown-test') and the messages appeared and the chart drew properly.
So I think the fix is to wait for gViz to load...
Goddamned Google. They're the MS of the 2020s.

@rickmcgeer
Copy link
Contributor Author

And, @merryman, you're right. We have to ditch Google Charts asap. I'm going to devote fulltime to the editor. It's the only way.

@rickmcgeer
Copy link
Contributor Author

I got it fixed, though frankly the fix shouldn't have been necessary. I put in a defensive call in addChart

if (this.gViz) {
    this.drawChart(chartName);
}

which shouldn't be necessary, because loadGoogleChartPackages is in init, it's async and we wait for it, and this happens (and logging confirms it) BEFORE we attempt to draw any charts.

Since we may have not drawn charts because Google Charts wasn't loaded, a callback was added to the charts loader:
(_ => this.drawAllCharts();

@rickmcgeer
Copy link
Contributor Author

I'm leaving this issue open because we need to talk about it. Mostly, this is Google stabbing us in the back, @merryman please look at the code and see if there is anything I have missed.
Here is the init code:

$world.execCommand("open browser", {moduleName: "studio/dashboard.cp.js", packageName: "galyleo-dashboard", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"init","type":"class-instance-method"}]});

And addChart:

$world.execCommand("open browser", {moduleName: "studio/dashboard.cp.js", packageName: "galyleo-dashboard", codeEntity: [{"name":"Dashboard","type":"class-decl"},{"name":"removeChart","type":"class-instance-method"}]});

Everything checked in.

@rickmcgeer
Copy link
Contributor Author

Fixed and closed

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

2 participants