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

FIX: Add a hard coded width bound to canvas rendered as widgets #319

Merged
merged 5 commits into from
Apr 5, 2024

Conversation

MridulS
Copy link
Member

@MridulS MridulS commented Apr 4, 2024

Fixes #169 (a hack-y one).

After some researching around about bouding canvases out of py3js it seems like this is the easiest
way of fixing the overflow problem of the canvas. A better solution would be to expose max_width from
py3js but I guess this is a minimal invasive change.

This does hard code the 80% width value but I guess we can expose that upstream to be user controllable
if the need arises.

@MridulS
Copy link
Member Author

MridulS commented Apr 4, 2024

The failure seems unrelated. I can't restart the runner.

@nvaytet
Copy link
Member

nvaytet commented Apr 4, 2024

The failure seems unrelated. I can't restart the runner.

I now gave you the correct permissions, and restarted the build as well ;-)

Edit: it seems the failure is consistent. I'm wondering if our CI setup is broken for PRs from forks?? I'll investigate.

@MridulS
Copy link
Member Author

MridulS commented Apr 4, 2024

Edit: it seems the failure is consistent. I'm wondering if our CI setup is broken for PRs from forks?? I'll investigate.

Yup, it is indeed. Seems like an issue people run in actions/checkout#551

@nvaytet
Copy link
Member

nvaytet commented Apr 4, 2024

I wonder if this would also remove the need for having e.g. figsize=(550, 400) in the docs, which (I think?) were there to prevent the figure from being too wide so as to not partially/totally hide the toolbar buttons? See for example here.

Edit: it seems to help indeed. I think we can remove most or all of the figsize=(...) for the 3d plots in the docs?

@nvaytet
Copy link
Member

nvaytet commented Apr 4, 2024

I think the same fix could also apply to 2d figures, when using the %matplotlib widget backend, in the (rare) case where you would make a very wide figure. I think it would also hide the toolbar buttons without it? Adding it here should work?

@MridulS
Copy link
Member Author

MridulS commented Apr 4, 2024

@nvaytet updated for 2d canvas and removed the use of figsize in 3d examples.

@MridulS MridulS changed the title FIX: Add a hard coded width bound to the p3js canvas FIX: Add a hard coded width bound to canvas rendered as widgets Apr 4, 2024
Copy link
Member

@nvaytet nvaytet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs #320 for builds to pass

@MridulS MridulS merged commit a8427fa into scipp:main Apr 5, 2024
3 checks passed
@MridulS MridulS deleted the bound_canvas branch April 5, 2024 12:54
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

Successfully merging this pull request may close these issues.

Toolbar disappears when 3D canvas is too big
2 participants