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

wxGUI: Fix behaviour of top Single-Window GUI toolbars #2568

Merged
merged 7 commits into from
Oct 28, 2022

Conversation

lindakarlovska
Copy link
Contributor

Fixes the non-standard behavior of top Single-Window GUI toolbars.

Bug:

  • bigger icons when undocking and docking the toolbar (takes into consideration that one does not drop a toolbar during that process)
    bigger icons
    after redocking all toolbars were resized

  • no toolbar name displayed after undocking
    no-name

After fix:
Now it inherits from AuiToolbar. This approach corresponds to the usage of aui agw library which is used for drawing the main window frame.

After conversion to AuiToolbar:

  • sizes of toolbars are still the same
    toolbar

  • toolbar name is displayed after undock
    worrkspace_toolbar

Potentially this PR could also fix some other related bugs (probably different depending on OS) caused by inheriting from the inappropriate wx.Toolbar class.

@lindakarlovska lindakarlovska added bug Something isn't working GUI wxGUI related labels Sep 2, 2022
@lindakarlovska lindakarlovska self-assigned this Sep 2, 2022
@lindakarlovska lindakarlovska added this to the 8.3.0 milestone Sep 2, 2022
@lindakarlovska lindakarlovska marked this pull request as draft September 2, 2022 17:22
@lindakarlovska lindakarlovska removed the request for review from petrasovaa September 2, 2022 17:22
@lindakarlovska lindakarlovska marked this pull request as ready for review September 4, 2022 07:38
Comment on lines 33 to 37
try:
from agw import aui
except ImportError:
import wx.lib.agw.aui as aui

Copy link
Contributor

Choose a reason for hiding this comment

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

Please check current import style and update if needed

Copy link
Contributor Author

@lindakarlovska lindakarlovska Sep 20, 2022

Choose a reason for hiding this comment

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

I checked in wxPython Demo version 4.1.1. and it should be used like that

"""Provides handling for aui.AuiToolBar widget"""

def _defineTool(self, name=None, icon=None, handler=None, item=wx.ITEM_NORMAL):
"""Define tool"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Explain why it needs to be different

Copy link
Contributor Author

@lindakarlovska lindakarlovska Sep 21, 2022

Choose a reason for hiding this comment

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

The problem is that AUI does not have InsertTool.. If I get it right, it is quite a new method in wx which is probably the reason. Look at https://www.geeksforgeeks.org/wxpython-inserttool-function-in-wx-toolbar/.

* fix import
* CreateTool doesn't need to be duplicated
* reduce the number of relay methods, rest is handled by getattr
@petrasovaa
Copy link
Contributor

I will merge this to get it tested more, could @nilason or @tmszi test it on Mac and Windows? The basic test is if the GUI launches at all, then it's worth testing undocking and docking the main toolbar, and switching to 3D and back.

It should be backported, but only after sufficient testing.

@petrasovaa petrasovaa merged commit 8c2239e into OSGeo:main Oct 28, 2022
@tmszi
Copy link
Member

tmszi commented Oct 28, 2022

I will merge this to get it tested more, could @nilason or @tmszi test it on Mac and Windows? The basic test is if the GUI launches at all, then it's worth testing undocking and docking the main toolbar, and switching to 3D and back.

Successfully tested on the MS Windows.

wxPython 4.1.1 msw (phoenix) wxWidgets 3.1.5 and version 4.2.0 msw (phoenix) wxWidgets 3.2.1

@nilason
Copy link
Contributor

nilason commented Oct 28, 2022

Tested on Mac with:
Python 3.10.7 (v3.10.7:6cc6b13308, Sep 5 2022, 14:02:52) [Clang 13.0.0 (clang-1300.0.29.30)]
4.2.0 osx-cocoa (phoenix) wxWidgets 3.2.0

Most things works smoothly and as expected. Actually a very pleasant experience, good work!!

But switching to and fro 3D mode causes a glitch:

01

02

03

Secondly, initially the main buttons are not visible:

04

...resizing the panel frame causes a redraw, which fixes that:

05

Finally, the Layer tool bar, starts displaced, and you also see a Map Display tab (it is also to be found where it should be in the middle pane):

06

after a window resize (just enough to initialise a redraw) they come to the right place but the tool bar seems too high:

07

@tmszi
Copy link
Member

tmszi commented Oct 28, 2022

Secondly, initially the main buttons are not visible:

04

...resizing the panel frame causes a redraw, which fixes that:

05

The same on MS Windows OS. I missed it the first time I tested it.

@petrasovaa
Copy link
Contributor

Thanks! Any idea if these issues were there before this was merged?

@nilason
Copy link
Contributor

nilason commented Oct 28, 2022

Thanks! Any idea if these issues were there before this was merged?

The last one was there already before, i.e.:

06

@lindakarlovska
Copy link
Contributor Author

I will merge this to get it tested more, could @nilason or @tmszi test it on Mac and Windows? The basic test is if the GUI launches at all, then it's worth testing undocking and docking the main toolbar, and switching to 3D and back.

It should be backported, but only after sufficient testing.

@petrasovaa Anna, thank you for your testing and looking into this PR more deeply. I did not manage to react earlier, I am sorry.
Thank also @nilason and @tmszi for testing!

petrasovaa added a commit that referenced this pull request Nov 11, 2022
Use agw.aui toolbar as the main toolbar to avoid strange behavior. This requires restructuring Toolbar classes. 

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
Use agw.aui toolbar as the main toolbar to avoid strange behavior. This requires restructuring Toolbar classes. 

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
marisn pushed a commit to marisn/grass that referenced this pull request Jun 2, 2023
Use agw.aui toolbar as the main toolbar to avoid strange behavior. This requires restructuring Toolbar classes. 

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
Use agw.aui toolbar as the main toolbar to avoid strange behavior. This requires restructuring Toolbar classes. 

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working GUI wxGUI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants