cytoscape / cytoscape-explore

Network visualization webapp.
MIT License
12 stars 4 forks source link

import-wizard - new UI #91

Closed chrtannus closed 2 years ago

chrtannus commented 2 years ago

General information

Associated issues: #78

Checklist

Author:

Reviewers:

Notes

maxkfranz commented 2 years ago

I'm ticking the testing box, since the CX conversion tests should cover import (esp. once we use CX for Cy3 importing as well).

maxkfranz commented 2 years ago

Overall, looks great. Just have to sort out some issues.

For the tooltips, it's probably best to avoid using innerHTML. innerHTML isn't used much anymore for several reasons.

Take a look at how we use Cytoscape Tippy tooltips in Biofactoid: https://github.com/PathwayCommons/factoid/blob/unstable/src/client/components/editor/cy/tippy.js

They're working well there. Basically, the approach is:

const div = document.createElement('div');

const component = <SomeComponent>Some stuff in here</SomeComponent>;

ReactDom.render( component, div );

If that doesn't help, then I suspect that MUI is incompatible with outside components. That's the downside of using a UI framework like that. One workaround may be to create a setup with the Tippy that is like an entirely new app from the perspective of MUI. I suspect that it expects the grid to be owned by a root-level like the network editor is:

<ThemeProvider theme={theme}>
  <CssBaseline />
  <div className="network-editor">
    <Header controller={controller} />
    <div className="network-editor-content">
      <div className="cy">
        <div id="cy" />
        <NetworkBackground controller={controller} />
      </div>
      <ToolPanel controller={controller} />
    </div>
  </div>
</ThemeProvider>

The tippies should be appended to the document.body: https://atomiks.github.io/tippyjs/v6/all-props/#appendto

There seems to be an issue if the MUI dialog is scrollable:

Screen Shot 2021-10-27 at 12 05 27 PM

maxkfranz commented 2 years ago

@chrtannus, do you want to merge in your PR before #97? If there are any merge conflicts between the PRs, I can sort them out.

The package lock file conflict can be resolved by deleting the file and then running npm i. That may result in slightly newer patch versions on some dependencies, but that's generally fine.

chrtannus commented 2 years ago

@maxkfranz I merged develop into my branch first and sorted out the conflicts. Also fixed a couple of other issues and refactored it a little bit. I think it's good to merge now.

maxkfranz commented 2 years ago

Great! Let's merge it in