search

QuantStack / jupyterlab-drawio

A standalone embedding of the FOSS drawio / mxgraph package into jupyterlab
BSD 3-Clause "New" or "Revised" License
599 stars 75 forks source link

Refactor and improvements #80

Closed hbcarlos closed 3 years ago

hbcarlos commented 3 years ago

Moves the menu bar and status bar buttons to JupterLab.

drawio

hbcarlos commented 3 years ago

There is an issue with menus not showing up because commands are added when the document it's opened.

menu

hbcarlos commented 3 years ago

We need to add shortcuts to the command, not in the label.

Screenshot from 2021-03-15 17-12-27

jtpio commented 3 years ago

Thanks!

A quick question after looking at the screencast. Should the menu entries be separate? (Diagram Arrange, Diagram Extras) Or grouped under a single Diagram entry? (with submenus)

jtpio commented 3 years ago

Were all the files under src/mxgraph/dotnet, src/mxgraph/docs and similar coming from the source mxGraph repo?

If so we can indeed remove them :+1:

hbcarlos commented 3 years ago

Were all the files under src/mxgraph/dotnet, src/mxgraph/docs and similar coming from the source mxGraph repo? If so we can indeed remove them +1

I think so. I'll remove it too.

hbcarlos commented 3 years ago

A quick question after looking at the screencast. Should the menu entries be separate? (Diagram Arrange, Diagram Extras) Or grouped under a single Diagram entry? (with submenus)

Good question indeed. Would look much nicer if grouping all under Diagram but will have 3 levels of submenus. Let me do a screencast to see how it looks like.

hbcarlos commented 3 years ago

Thanks!

A quick question after looking at the screencast. Should the menu entries be separate? (Diagram Arrange, Diagram Extras) Or grouped under a single Diagram entry? (with submenus)

Yep, looks nicer.

menu-diagram

jtpio commented 3 years ago

Looks nice, thanks for sharing the screencast :+1:

wolfv commented 3 years ago

wow, this looks really properly integrated now! :tada:

hbcarlos commented 3 years ago

Thanks Jeremy!

jtpio commented 3 years ago

Just tested this PR locally and it looks great!

The Binder for this branch:

Binder

seems to be failing at the pip check step:

https://github.com/QuantStack/jupyterlab-drawio/blob/4a7753d0d70ef4170106843b3aae97163e5d0c2b/binder/postBuild#L30

image

jtpio commented 3 years ago

Another thing that could be addressed separately is theming support for the toolbar icons, for example with the dark theme:

image

Maybe that will mean recreating a set of icons and use a LabIcon to add the items to the toolbar.

hbcarlos commented 3 years ago

Thanks Jeremy. Can you open an issue to address this later?

jtpio commented 3 years ago

Sounds good :+1:

jtpio commented 3 years ago

Let's merge this and see if we can iterate in follow-up PRs before cutting a new release.