Add a default toolbar for figure#1394
Conversation
martinRenou
left a comment
There was a problem hiding this comment.
Looks great! I only have minor comments
|
|
||
|
|
||
| def show(key=None, display_toolbar=True): | ||
| def show(key=None, display_toolbar=False): |
There was a problem hiding this comment.
Should we really change this? This will break long-time users assumption that it shows a toolbar by default.
There was a problem hiding this comment.
Actually, maybe display_toolbar should be kept True by default and used for setting the figure's show_toolbar property?
There was a problem hiding this comment.
Thinking again about this, this sounds like a good idea by @martinRenou.
There was a problem hiding this comment.
Should we remove the old and ugly toolbar in this PR as well?
There was a problem hiding this comment.
i think we should since user can still add this ugly toolbar independently.
|
Added animation for |
martinRenou
left a comment
There was a problem hiding this comment.
LGTM! Would you mind squashing the commits a bit?
|
Thanks @martinRenou, yes can you squash it please! |
|
Note: I think this is a 0.13 thing since it is a significant change of behavior. |
|
Should we move forward introducing bqscales and removing threejs then? I was also wondering if we should still maintain a 0.12.x branch? |
|
Yes, we should start merging thing and create a 0.12.x branch pointing at the last commit before this PR. |
|
@maartenbreddels just wanted to ping you here. You should be interested in this. |
|
Thanks for the ping. |
References
This patch adds a default toolbar to
Figurewidget, this toolbar contains the same buttons as inToolbarwidget but it is implemented directly inFigureclassUser-facing changes
display_toolbar(defaultTrue) is added to constructor ofFigureto show or hide the toolbar.pyplotfigure. Nowdisplay_toolbarparameter ofplt.showis used to show or hide the default toolbar.