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
Figure now resizes to fit the window #14
Conversation
examples/chart/app.py
Outdated
f = Figure(figsize=(5, 4)) | ||
ax = f.add_subplot(1, 1, 1) | ||
dpi = 100 # as of writing, 100 is also the default DPI for matplotlib.figure.Figure | ||
f = Figure(figsize=(self.chart.layout.content_width / dpi, self.chart.layout.content_height / dpi), dpi=dpi) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not particularly a fan of how this accesses something that doesn't really seem to be documented (or at least somewhere I thought to look), but it's how it's done in the Canvas example for Toga: https://github.com/beeware/toga/blob/e1650326e77d09a27f7e916072bb3379b334977f/examples/canvas/canvas/app.py#L282
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - it's not formally documented, but I'd interpret the layout
attribute as fair game for read access.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay in reviewing this. It looks great!
I've added to your code to take these improvements a step further. Old code should continue to work as it did before; but I've modified the API (and the example) to provide what I hope is a more natural API that is focussed on what the user is actually trying to do: draw a chart.
The major benefit:
- The chart automatically resizes when the canvas resizes. This seems like a natural and obvious behaviour to me, so I can't see any reason it shouldn't be the default.
- The end user doesn't need to interrogate the layout width/height attributes - they get a pre-built Figure that is the right size.
- The end user doesn't need to import any of matplotlib for simple cases; they just manipulate a Figure that they've been given.
If the user doesn't want the resize behavior, they can define their own "no-op" resize, and fall back to manually drawing as before.
on_resize
to init so it can be used by the userI was going to add something to make it deal with resizing the figure on it's own, but I didn't see a clean way to do it. So if you see one I can add it.
Let me know if you want anything changed.