search

queryverse / ElectronDisplay.jl

An Electron.jl based figure and table display.
Other
85 stars 17 forks source link

New menu #70

Open TonyLianLong opened 4 years ago

TonyLianLong commented 4 years ago

This fixes #69.

davidanthoff commented 4 years ago

Cool! I'm testing on different platforms.

On Windows, I get:

image

Everything else seems to work :) I'll test on Linux and Mac next.

davidanthoff commented 4 years ago

Here are my Mac comments:

davidanthoff commented 4 years ago

And Linux:

davidanthoff commented 4 years ago

Oh, and for some reason tests fail? Probably also needs some update to fix that?

TonyLianLong commented 4 years ago

Windows:

  • [x] I think we don't need the whole top-level Window menu, can you remove that?

Removed

  • [x] Can you remove the dev tool window item? I don't think we should expose that to normal users.

Removed

  • [ ] I wonder whether we could enable/disable the zoom menu items depending on whether the active plot is one where these commands work? If that works we could also remove the text (For Static Images) because that would be kind of obvious in that case, right?

It's possible, but it's hard and it's likely the code will be messy because this is dynamic in plotgallery.

TonyLianLong commented 4 years ago

Mac:

  • [x] There is a menu Window -> Window, that should probably be removed?

Yes.

  • [ ] Could we rename the main menu (the one in bold) from Julia to ElectronDisplay?

It depends on Electron.jl which generates the application.

  • [ ] The Edit menu has a submenu speech? Does that do anything?

That's Mac's. Most if not all windows have them. I can't remove them in the code.

  • [ ] There is also a "Start dictation" and emoji menu, do these do anything?

Same.

Linux menu has been dealt while removing the Window menu.

TonyLianLong commented 4 years ago

Problem with changing the name in menu for mac: https://discuss.atom.io/t/change-app-name-in-menu-bar/39839

TonyLianLong commented 4 years ago

I have looked into test.jl. It does not work because the stringmime in vega.jl returns Base64, which is unwanted. I tried replacing stringmime with repr, but it does not work either. I'm not familiar with this part. Can you look into it? @davidanthoff

TonyLianLong commented 4 years ago

Should I quote the base64 image with quotes in line 329 and 338 of ElectronDisplay.jl? I mean change them to displayplot(d, "image", "'" + imgdata + "'").

davidanthoff commented 4 years ago

@TonyLianLong I added some methods in a commit in this branch that I believe should fix the base64 problem, i.e. with those methods I think stringmime should now just return text, if I understand this stuff properly. Can you give it a try?

TonyLianLong commented 4 years ago

Now these tests run successfully on my computer. Although not all plots are able to load on the window, there is no error on the Julia side (this could be an issue with fetch API on the JavaScript side, which we are currently not fixing). Let's see what will happen on CI.

TonyLianLong commented 4 years ago

@davidanthoff The tests are successful, but some are cancelled after being successful. Does that mean I still have some code in the test files need to fix, or it is caused by Github Actions?