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.

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

wxGUI/PyShell: check for wxPython version #1160

Merged
merged 2 commits into from Dec 12, 2020
Merged

wxGUI/PyShell: check for wxPython version #1160

merged 2 commits into from Dec 12, 2020

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Dec 7, 2020

Add a wxPython version check on using useStockId parameter on (Py)Shell init call.

Fixes #1156

@nilason nilason mentioned this pull request Dec 7, 2020
Closed
@neteler
Copy link
Member

neteler commented Dec 7, 2020

Successfully tested on Linux (Fedora 33).

@infrastation
Copy link
Contributor

Would this be better?

        shellargs = dict(parent=self, id=wx.ID_ANY, introText=self.intro,
                         locals={'gs': grass, 'AddLayer': self.AddLayer})
        # useStockId should be False on macOS
        # useStockId did not exist before wxPython 4.0.2
        if sys.platform == "darwin" and CheckWxVersion([4, 0, 2]):
            shellargs['useStockId'] = False
        self.shell = PyShell(**shellargs)

@nilason
Copy link
Contributor Author

nilason commented Dec 7, 2020

Would this be better?

        shellargs = dict(parent=self, id=wx.ID_ANY, introText=self.intro,
                         locals={'gs': grass, 'AddLayer': self.AddLayer})
        # useStockId should be False on macOS
        # useStockId did not exist before wxPython 4.0.2
        if sys.platform == "darwin" and CheckWxVersion([4, 0, 2]):
            shellargs['useStockId'] = False
        self.shell = PyShell(**shellargs)

Nice, I'd say it would be. Updated PR with a Black formatted version of what you suggested. Thanks!

@wenzeslaus wenzeslaus merged commit 1ee82f0 into OSGeo:master Dec 12, 2020
@wenzeslaus wenzeslaus added backport_needed bug Something isn't working labels Dec 12, 2020
@wenzeslaus
Copy link
Member

Backport needed right after (together with) #818.

@nilason
Copy link
Contributor Author

nilason commented Dec 12, 2020

Thanks!

@neteler neteler added this to the 7.8.6 milestone Dec 13, 2020
@neteler
Copy link
Member

neteler commented Dec 13, 2020

#818 is so far scheduled for 7.8.6, so same milestone here.

If you want me to anticipate both for milestone 7.8.5 , let me know.

@nilason
Copy link
Contributor Author

nilason commented Dec 13, 2020

If you want me to anticipate both for milestone 7.8.5 , let me know.

No, 7.8.6 at the earliest for both. Thank you!

@nilason nilason deleted the fix-1156 branch December 21, 2020 11:14
@neteler
Copy link
Member

neteler commented Jan 4, 2021

If you want me to anticipate both for milestone 7.8.5 , let me know.

No, 7.8.6 at the earliest for both. Thank you!

Now we have the 7.8.6 milestone open: backport or postpone, @nilason ?

@nilason nilason mentioned this pull request Jan 4, 2021
Merged
@nilason
Copy link
Contributor Author

nilason commented Jan 4, 2021

Now we have the 7.8.6 milestone open: backport or postpone, @nilason ?

please see: #818 (comment)

@neteler
Copy link
Member

neteler commented Jan 4, 2021

Now we have the 7.8.6 milestone open: backport or postpone, @nilason ?

please see: #818 (comment)

May I leave the backporting of this and the related ticket(s) directly to you? ... may be easier :-)

@nilason
Copy link
Contributor Author

nilason commented Jan 4, 2021

May I leave the backporting of this and the related ticket(s) directly to you? ... may be easier :-)

I’m glad to give it go :-)

nilason added a commit that referenced this pull request Jan 5, 2021
@nilason
wxGUI: Fix key binding issues on mac (#818 #1160)
e7fded2 
Fixes #785 (backport of 5fd8c29)

The main problem with stock id buttons is that they all come with a key
binding. E.g. Close, Clear and Copy have the same key binding ctrl+c and
causes problems at least for mac. To remedy this issue we are using
subclassed versions of Button: ClearButton, CancelButton, ApplyButton).

I also made some effort to implement ESC to close dialog (attribute
table manager, mapcalc).

Note: for module dialogs (forms.py) and Map calculator I have replaced
ctrl+c to copy selected text, not to copy the whole command.

This also addresses a subsequent issue (#1156) introduced by 5fd8c29,
with an added check for wxPython version (backport of 1ee82f0).
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@nilason @ninsbl
wxGUI/PyShell: check for wxPython version (OSGeo#1160)
021d93b 
Fixes OSGeo#1156 (5fd8c29).
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@nilason @ninsbl
wxGUI/PyShell: check for wxPython version (OSGeo#1160)
66abc43 
Fixes OSGeo#1156 (5fd8c29).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrasovaa petrasovaa petrasovaa approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
7.8.6
Development

Successfully merging this pull request may close these issues.

[Bug] GUI fails to start since commit 5fd8c29
5 participants
@nilason @neteler @infrastation @wenzeslaus @petrasovaa