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 location wizard: EPSG page #670

Merged
merged 6 commits into from
Jun 13, 2020

Conversation

lindakarlovska
Copy link
Contributor

@lindakarlovska lindakarlovska commented May 24, 2020

Path to usr/share/proj/epsg was removed. Search and EPSG code text inputs were merged into one search input. Link to epsg.io was added. The layout was adapted using several box sizers instead of grid sizer.

Before:
Screenshot from 2020-05-24 13-46-49

Version 1:
Screenshot from 2020-06-05 07-26-31

The motion of events is following (unincorporated yet in Version 1):

  • searching in a list on the fly
  • choose automatically the first row and permit "next" button
  • if epsg code is empty, nothing is selected and "next" button is not active
  • change informative URL according to a query string

Version 2:
Screenshot from 2020-06-06 05-24-32

Already incorporated:

  • searching in a list on the fly
  • if epsg code is empty, nothing is selected and "next" button is not active
  • change informative URL according to a query string

I have a problem that when selecting epsg code from a table, it sometimes happens that neither the query string nor the final epsg code mentioned in the summary page changes. Also I am not sure how to incorporate choosing the automatically the first row in the list.

Version 3 (Layout enhancement):
Screenshot from 2020-06-07 05-10-50

@lindakarlovska lindakarlovska marked this pull request as draft May 24, 2020 18:50
@lucadelu
Copy link
Contributor

I'm not sure to remove completely the capability to use local epsg file. There could be specific coordinate system in local epsg file that are not in epsg.io

I suggest to change "Link to epsg.io:" in "Source:" with default value "epsg.io" with a button "Local epsg file" (or "Browse") to select the epsg file and change the source.

What do you think?

@mlennert
Copy link
Contributor

I'm not sure to remove completely the capability to use local epsg file. There could be specific coordinate system in local epsg file that are not in epsg.io

I suggest to change "Link to epsg.io:" in "Source:" with default value "epsg.io" with a button "Local epsg file" (or "Browse") to select the epsg file and change the source.

What do you think?

The idea here is not to remove the use of the local epsg file. It is still used. The idea is that in 95% of cases users do not have to change this and that in the 5% of cases where it has to be changed, users know what they are doing and can simply set the relevant environment variable.

@mlennert
Copy link
Contributor

Path to usr/share/proj/epsg was removed. Search and EPSG code text inputs were merged into one search input. Link to epsg.io was added. This link responds to the selection of EPSG code.

What do you mean by "responds to the selection of EPSG code" ?

Is the idea that if the user wants to search for all projection systems relevant for Ghana that she has to go to epsg.io, search there and then copy over the epsg code ?

I think we should avoid dependency on an internet connection, and so I would like to keep the possibility to search with keywords and not only epsg codes. I thought the idea was to just combine everything into one 'search' field where the user could enter either epsg codes or keywords ?

@landam landam added enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related labels May 25, 2020
@petrasovaa
Copy link
Contributor

I reviewed the page layout and suggest the following changes (diff against the last state):

--- a/gui/wxpython/location_wizard/wizard.py
+++ b/gui/wxpython/location_wizard/wizard.py
@@ -1553,8 +1553,7 @@ class EPSGPage(TitledPage):
 
         # epsg.io hyperlink
         self.tlink = HyperlinkCtrl(
-            self, id=wx.ID_ANY, 
-            size=(200, -1),
+            self, id=wx.ID_ANY,
             label="epsg.io",
             url="epsg.io")
         self.tlink.SetNormalColour(
@@ -1566,26 +1565,19 @@ class EPSGPage(TitledPage):
         
         # layout       
         searchBoxSizer.Add(self.lcode, proportion=0,
-                       flag=wx.ALIGN_LEFT |
-                       wx.ALIGN_CENTER_VERTICAL |
+                       flag=wx.ALIGN_CENTER_VERTICAL |
                        wx.ALL, border=5)
         searchBoxSizer.Add(self.searchb, proportion=1,
-                       flag=wx.ALIGN_LEFT |
-                       wx.ALIGN_CENTER_VERTICAL |
-                       wx.ALL, border=5)
+                       flag=wx.ALL, border=5)
         epsglistBoxSizer.Add(self.epsglist, proportion=1,
-                       flag=wx.ALIGN_LEFT |
-                       wx.ALIGN_CENTER_VERTICAL |
-                       wx.EXPAND |
+                       flag=wx.EXPAND |
                        wx.ALL, border=5)
-        informationBoxSizer.AddStretchSpacer(10) 
+        informationBoxSizer.AddStretchSpacer(1) 
         informationBoxSizer.Add(self.llink,  proportion=0,
-                       flag=wx.ALIGN_RIGHT |
-                       wx.ALIGN_CENTER_VERTICAL |
+                       flag=wx.ALIGN_CENTER_VERTICAL |
                        wx.ALL, border=5)
-        informationBoxSizer.Add(self.tlink, proportion=1,
-                       flag=wx.ALIGN_RIGHT |
-                       wx.ALIGN_CENTER_VERTICAL |
+        informationBoxSizer.Add(self.tlink, proportion=0,
+                       flag=wx.ALIGN_CENTER_VERTICAL |
                        wx.ALL, border=5)

Mostly the problem is with the flags, some of the combinations are throwing errors with latest wxPython. My suggestion for future is to not use wx.ALIGN_... and add them only when you see the need, rather then to put them everywhere 'just in case'. Also typically we don't specify fixed sizes of widgets.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

Some small improvements to the layout:

@@ -1568,21 +1568,21 @@ class EPSGPage(TitledPage):
                        flag=wx.ALIGN_CENTER_VERTICAL |
                        wx.ALL, border=5)
         searchBoxSizer.Add(self.searchb, proportion=1,
-                       flag=wx.ALL, border=5)
+                       flag=wx.ALL | wx.EXPAND, border=5)
         epsglistBoxSizer.Add(self.epsglist, proportion=1,
                        flag=wx.EXPAND |
                        wx.ALL, border=5)
         informationBoxSizer.AddStretchSpacer(1) 
         informationBoxSizer.Add(self.llink,  proportion=0,
                        flag=wx.ALIGN_CENTER_VERTICAL |
-                       wx.ALL, border=5)
-        informationBoxSizer.Add(self.tlink, proportion=1,
-                       flag=wx.ALIGN_CENTER_VERTICAL |
-                       wx.ALL, border=5)
+                       wx.RIGHT, border=5)
+        informationBoxSizer.Add(self.tlink, proportion=0,
+                       flag=wx.ALIGN_CENTER_VERTICAL)
         
         self.sizer.Add(searchBoxSizer, proportion=0, flag=wx.EXPAND)
         self.sizer.Add(epsglistBoxSizer, proportion=1, flag=wx.EXPAND)
-        self.sizer.Add(informationBoxSizer, proportion=0, flag=wx.EXPAND)
+        self.sizer.Add(informationBoxSizer, proportion=0,
+                       flag=wx.EXPAND | wx.TOP, border=5)

More importantly, the selection of the item in list needs to be fixed. We talked about automatic selection of the item after users starts to search (first in the list for simplicity, but we may want to do something more complex regarding preferring direct epsg match). The search box should not be updated upon selection in the list.

gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
gui/wxpython/location_wizard/wizard.py Outdated Show resolved Hide resolved
@petrasovaa petrasovaa marked this pull request as ready for review June 12, 2020 18:17
@petrasovaa petrasovaa self-requested a review June 12, 2020 18:17
@landam landam self-requested a review June 12, 2020 19:44
@petrasovaa petrasovaa merged commit 19ea788 into OSGeo:master Jun 13, 2020
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants