Skip to content

Updated WxAgg NavigationToolbar2 breaks custom toolbars #18212

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.

Sign up for GitHub

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

Jump to bottom
Closed
jbhopkins opened this issue Aug 10, 2020 · 3 comments · Fixed by #18219
Closed

Updated WxAgg NavigationToolbar2 breaks custom toolbars #18212

jbhopkins opened this issue Aug 10, 2020 · 3 comments · Fixed by #18219
Labels
GUI: wx
Milestone
v3.3.1

Comments

@jbhopkins
Copy link
Contributor

Bug report

Bug summary

In matplotlib 3.3, the WxAgg backend NavigationToolbar2 has added a readout of the cursor position on the far right of the toolbar. This breaks the layout of custom toolbars where users have added their own buttons or other widgets. Because the cursor position is added with a stretchspacer, any widget added after it as also added to the far right of the toolbar. In addition (possibly due to adding the custom widgets after the toolbar is initially realized?), the cursor readout can appear under the custom toolbar buttons.

Here's a custom toolbar from a program I maintain using matplotlib 3.3 and 3.2.2.

With 3.2.2:
custom_toolbar_mpl3p2p2

With 3.3:
custom_toolbar_mpl3p3

Code for reproduction

import wx
import matplotlib as mpl
from matplotlib.backends.backend_wxagg import NavigationToolbar2WxAgg
from matplotlib.backends.backend_wxagg import FigureCanvasWxAgg

class CustomToolbar(NavigationToolbar2WxAgg):
    def __init__(self, canvas):

        NavigationToolbar2WxAgg.__init__(self, canvas)

        move_bitmap = wx.Bitmap('./images/move.png')
        self.AddCheckTool(wx.ID_ANY, '', move_bitmap, shortHelp='Second Move Button')

class PlotFrame(wx.Frame):
    def __init__(self, parent, title, *args, **kwargs):
        wx.Frame.__init__(self, parent, title=title, *args, **kwargs)

        self.fig = mpl.figure.Figure((5,4))
        self.subplot1 = self.fig.add_subplot(111)
        self.canvas = FigureCanvasWxAgg(self, wx.ID_ANY, self.fig)

        self.toolbar = CustomToolbar(self.canvas)
        self.toolbar.Realize()

        sizer = wx.BoxSizer(wx.VERTICAL)
        sizer.Add(self.canvas, 1, wx.LEFT | wx.TOP | wx.GROW)
        sizer.Add(self.toolbar, 0, wx.GROW)

        self.SetSizer(sizer)
        self.Layout()
        self.Fit()
        self.CenterOnScreen()
        self.Show(True)

if __name__ == '__main__':
    app = wx.App(False)
    frame = PlotFrame(None, 'My Plot Frame')
    app.MainLoop()

Actual outcome
Actual outcome with 3.3:
actual_outcome

Expected outcome
Expected outcome (as with previous versions, using 3.2.2):
expected_outcome

Matplotlib version

  • Operating system: MacOS 10.14.6
  • Matplotlib version: 3.3.0
  • Matplotlib backend (print(matplotlib.get_backend())): WxAgg
  • Python version: 3.7.6
  • Other libraries: wxpython 4.0.4

Everything installed through conda. Matplotlib 3.2.2 installed from the main channel, 3.3.0 installed from conda-forge channel.
@jbhopkins
Copy link
Contributor Author

jbhopkins commented Aug 10, 2020 •
edited
Loading

There are a couple ways I've thought of this could be fixed, neither of them particularly hard. If someone tells me what the preferred option is, or suggests another option, I'm happy to do the coding and make a pull request.

Option 1:
Add a keyword argument to the NavigationToolbar2Wx class, something like 'show_mouse_pos=True'. If this is True (default), then show the cursor readout in the toolbar (current behavior). If it isn't, then don't add the cursor position to the toolbar. I've tested this and it's easy to implement.

Option 2:
Don't add the stretch spacer for the cursor position. If you have the cursor position just after the save button, and you save the tool id for the cursor readout StaticText in the self.wx_ids dictionary, someone could then programmatically remove the cursor readout using wx.ToolBar.DeleteTool. At least in my testing I couldn't remove the stretch spacer using this function, so this option would require that stretch spacer not be added. I've tested this and it's easy to implement.

I prefer option 1, because that provides a clean API for those who don't want the cursor readout. Option 2 would require anyone who wants to do this to go digging around in the backend code. However, I don't know if there are constraints on the API for these NavigationToolbars in general across backends that might make that a less appealing option.

@tacaswell tacaswell added this to the v3.3.1 milestone Aug 10, 2020
@tacaswell
Copy link
Member

Option 1 is better here, it gives you a clean way out and will match the qt5 toolbar

matplotlib/lib/matplotlib/backends/backend_qt5.py

Lines 654 to 655 in 7b56226

def __init__(self, canvas, parent, coordinates=True):
"""coordinates: should we show the coordinates on the right?"""

I do not disagree that show_mouse_pos is a more descriptive name, but for consistency we should match coordinates.

@QuLogic
Copy link
Member

QuLogic commented Aug 11, 2020

Note, we would like to release 3.3.1 in a day or two, if you could get that PR in soonish.

@jbhopkins jbhopkins mentioned this issue Aug 11, 2020
Merged
6 tasks
@QuLogic QuLogic linked a pull request Aug 11, 2020 that will close this issue
Fixes an issue where WxAgg NavigationToolbar2 broke custom toolbars #18219
Merged
6 tasks
@jameskeaveney jameskeaveney mentioned this issue Sep 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GUI: wx
Projects
None yet
Milestone
v3.3.1
Development

Successfully merging a pull request may close this issue.

Fixes an issue where WxAgg NavigationToolbar2 broke custom toolbars jbhopkins/matplotlib
3 participants
@tacaswell @QuLogic @jbhopkins