-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
Resize plot when electron window is resized #25
Comments
This is totally doable. I had it turned on at one point, but decided against making it the default behavior because I wanted to favor the width send height settings in the layout. I could make it an optional feature if you want. I'm wondering what the best api for that would be:
I think I lean towards 2, but then I don't see a way to make that apply to every plot automatically if that's what the user wants. |
I agree, I see a good argument for both options. Would it be possible to have a UI element in the window that switches this? The best option would be if there was just an additional button in the toolbar that is shown already that would switch between original size, and stretched size. But that toolbar is probably "owned" by plotly, right? But something like that might be nice. |
Ultimately we have total control over everything that appears in that window. I think we could probably attach another button to the plotly menubar. If you have any interest in trying this out let me know and I can help you get started. Otherwise I'll try to get around to this when I have a chance (no promises that it will happen quickly). |
I'm completely under water with work for quite a while, so unfortunately I won't be able to contribute to this... No rush, this is a great package already and I really appreciate all the great work you do on it! |
No problem. Thanks for reaching out and please file any other issues you come across! |
I may be interested in helping on this. To keep it simple to start with, could we start having the global that can be changed, and as a second stage we attach the gui element on the bar to it? |
Hi @mirestrepo that would be great! I think that is a good idea for the first pass. If you are able to implement this as a global option and open up a pull request, we can discuss further at that point and figure out the best way to integrate the functionality with the rest of the library. When I first implemented this I just followed this example. To get it working at this stage, I think it will be easiest for you to adapt the If you have other questions on how to do it please let me know. |
@spencerlyon2 I submitted the pull request. I took the liberty to have the default behavior to resize the plot as the electron window is being resized. I can't think why this should not be the case - but perhaps you have strong reasons to think otherwise, so I'm happy to change the code to have the default being a fixed size plot that doesn't resize along with the window. To turn it the resizing off, the user just needs to define in the Main I opted for this as opposed to having a global flag inside PlotlyJS that the user sets/unsets as it seem messy to have that global. But again - the best interface it's probably worth discussing. |
Also out of curiosity, is there a specific reason why you stopped calling html_body inside Base.display rewrote with |
I used the |
Done! |
@spencerlyon2 I can't seem to get this to work for me. I am using PlotlyJs via the Plots.jl api. Is there anything I have to do to make the plots resize correctly. If I understand correctly from the above posts this should be the default behaviour? |
Can you tell me which version of PlotlyJS is installed? |
Running "Pkg.status()" shows 0.3.3 |
Hmm, that is the latest version. It works for me. Can you post a simple example of plotting code that doesn't cause the display to resize? If so I can try to take a look at it. Thanks |
I am using the Juno IDE and run the following: `import Plots #select PlotlyJS backend #plot a simple sine wave |
If I use the PlotlyJS api directly, then it works. i.e. Using the line scatter example from the docs works:
So I assume the issue is in Plots.jl somewhere. |
If it helps, this is called just before displaying the plot: PlotlyJS.relayout!(syncplot, pdict, width = w, height = h) I have no idea how this interacts with the auto-resizing, but if you have a hack inside PlotlyJS it's possible this overrides it. |
Ok I've figured out the problem here... the issue is that Plots.jl always attaches a width and height to the plot's layout even if you don't explicitly set them. When these fields are set on the plot, the size of the plot is fixed from creation time and doesn't respect the resizing of the window. The most obvious way around this is that when autoresize is on, we strip the width and height fields on the layout before sending the plot to plotly.js. This has the downside that the display method will mutate the plot object by adjusting its layout fields. What do people think? Should we wipe these fields from the layout when the user has autoresize mode on? |
As long as it opens with the right size initially, I think it's fine to overwrite it later on a user resize event. |
Yep, that won't be impacted -- the window is opened before the javascript to display the plot is run. The current solution I have in mind would be do to this: pop!(p.layout.fields, :width, nothing)
pop!(p.layout.fields, :height, nothing) just before display. The issue with that is that this ruins the width and height for all interactions with electron -- including saving figures. For example, if you display a plot and call save fig immediately you will get a plot that is the same size as the initial window. If you then maximize the screen and save again you will get a file whose plot is the entire size of your screen. |
Hmmm. That could just as easily be considered a feature, not a bug! :) I say go for it |
Fixed on master |
I'd agree - that is a feature ;) |
Thanks. Tested on Master. Working well for me. |
I'm on Windows. It would be great if the plot size would adjust as I resize the window where it is shown, so that the plot always uses the maximum available window space.
My use case is just the electron output, from the julia command line (i.e. no IJulia or anything like that).
The text was updated successfully, but these errors were encountered: