-
-
Notifications
You must be signed in to change notification settings - Fork 362
wxGUI: Projection checkbox from Map Display statusbar moved to Map Display Settings #2087
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: Projection checkbox from Map Display statusbar moved to Map Display Settings #2087
Conversation
gui/wxpython/mapdisp/properties.py
Outdated
else: | ||
self.widget.SetLabel(self.defaultLabel) | ||
|
||
# disable long help |
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.
Not sure how to deal with this method. Seems like quite dependent on mapframe. I have noticed that it is quite common in statusbar.py (SbItem, SbMapScale, SbGoTo and SbProjection), however we need to get rid of map display dependency. So maybe put it into giface?
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.
What is it even doing? Doesn't look very important.
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.
Seems to me the same. As far as I was able to find out it sets the help to a toolbar item. E.g. if enabled it sets a tool long help something like: WindowIDRef: -31309 Zoom to default or saved region, save to named region, .... If not enabled the help is just WindowIDRef: -31309.
gui/wxpython/mapwin/base.py
Outdated
@@ -95,6 +95,23 @@ def alignExtent(self, value): | |||
self.alignExtentChanged.emit(value=value) | |||
|
|||
|
|||
class StatusBarProperties(object): |
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.
Seems this could be part of MapWindowProperties, no?
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.
Well.. I had the feeling that it should be separated because status bar properties do not influence a map window itself in any way. But it is probably early to decide, we can divide it later on if needed.
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 assume moving the projection selection from GUI settings would go into another PR?
:return: widget or None if doesn't exist | ||
""" | ||
return self.widget | ||
|
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.
Any reason this class is missing all the property functions and does not inherit from PropertyItem?
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 am a bit unsure about _connect and _disconnect methods. They are not used for projection class but they may be called (if we let projection inherit from PropertyItem). Not sure if it is not confusing. For me not, but not sure if it is the way how it should be correctly implemented.
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.
Well, I can share the solution with the inheritance I made...
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 don't see why it should behave/be implemented differently, so I changed that based on the other properties, but please test.
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.
You are right, it can be implemented in the same way. I was probably confused by signals. I tested the changes you made and it works fine.
Yes, that's a plan. |
…splay Settings (OSGeo#2087) Co-authored-by: Anna Petrasova <[email protected]>
…splay Settings (OSGeo#2087) Co-authored-by: Anna Petrasova <[email protected]>
…splay Settings (OSGeo#2087) Co-authored-by: Anna Petrasova <[email protected]>
This PR creates a new Statusbar page in Map Display settings and move there "Use defined projection checkbox".
