wxGUI location wizard: First page #646
Conversation
landam
added
enhancement
|
@lindakladivova Please provide screenshots before and after. It's self-explanatory description :-) |
There was a problem hiding this comment.
I would say the asterisk should go right after the label. Since there is GridBagSizer used, you may need to shift the edit fields one column to the right to get the asterisk there so that everything is aligned. Alternatively, the edit fields could be moved below the label like we have in forms.
| @@ -179,6 +179,11 @@ def __init__(self, wizard, parent, grassdatabase): | |||
| grass.legal_name, | |||
| self._nameValidationFailed)) | |||
| self.tlocTitle = self.MakeTextCtrl(size=(400, -1)) | |||
|
|
|||
| # text for required options | |||
| self.required_txt = self.MakeStaticText("*", size=(-1, -1)) | |||
There was a problem hiding this comment.
I don't think the required_txt needs to be attribute of the class (remove self).
petrasovaa
changed the title
|
We thought about it more and I think we should redo the DB part. The readonly textctrl is I think confusing (visually you can't see it's readonly at least on gtk). I think we can use statictext as before and move it below, it shouldn't be the first thing. Here is a poor drawing of what you could do. Notice also change in page title and Location Name could be just Name. The Change button is just if we realize we need to enable users to change the database (would be aligned with the asterisk). I think you could also go ahead and remove the checkboxes (that's in the roadmap). |
| self.tlocation.SetFocus() | ||
| self.tlocation.SetValidator( | ||
| GenericValidator( | ||
| grass.legal_name, | ||
| self._nameValidationFailed)) | ||
| self.tlocTitle = self.MakeTextCtrl(size=(400, -1)) | ||
|
|
||
| # text for required options | ||
| self.required_txt = self.MakeLabel("*") |
There was a problem hiding this comment.
As I mentioned, don't use self here if it's used only locally within the scope of the method, if you make it an attribute of the class it looks like it's being used elsewhere too, which is confusing.
| self.required_txt.SetToolTip(_("This option is required")) | ||
|
|
||
| # text for optional options | ||
| self.optional_txt = self.MakeLabel("(optional)") |
There was a problem hiding this comment.
Make it translatable: _("optional")
| self.optional_txt = self.MakeLabel("(optional)") | ||
| italics = wx.Font(10, wx.DEFAULT, wx.ITALIC, wx.NORMAL) | ||
| self.optional_txt.SetFont(italics) | ||
| self.optional_txt.SetForegroundColour("gray") |
There was a problem hiding this comment.
I would recommend here using system colors instead of 'gray': wx.SystemSettings.GetColour(wx.SYS_COLOUR_GRAYTEXT) because people have different window styles.
| @@ -171,14 +162,26 @@ def __init__(self, wizard, parent, grassdatabase): | |||
| self.locTitle = '' | |||
|
|
|||
| # text controls | |||
| self.tgisdbase = self.MakeStaticText(grassdatabase, size=(-1, -1)) | |||
| self.tlocation = self.MakeTextCtrl("newLocation", size=(300, -1)) | |||
| self.tgisdbase = self.MakeTextCtrl(grassdatabase, size=(400, -1), style = wx.TE_READONLY | wx.TRANSPARENT_WINDOW) | |||
There was a problem hiding this comment.
If you look at the code checks, the flake8 code quality check is failing on your PR because of spaces around =. It should be:
..., style=wx.TE_READONLY | wx.TRANSPARENT_WINDOW
see PEP8 for reference.
…change button added and page title and Location Name changed.
Another version: |
I think we should use only StaticText for the DB to communicate clearly that user doesn't need to edit it. |
Ok. I have changed that: |
lindakarlovska
changed the title
|
Looks good! Just one other small change, I noticed the summary page still has Location Title, so it would be good to change that to Description to sync the labels. |
Location wizard location name marked as required and the Project Location renamed to Location Name.
Sorry I do not have before. I have the one after :-) :
Here's the screenshot of the second version:
Here's the screenshot of the third version:
(The layout was a bit changed and also the Description was marked as optional. GIS data directory is still non-editable but made through wx.TE_READONLY method.)
Here's the screenshot of the fourth version:
I more like the TextCtrl than StaticText for Location Database. I find it more logical because the window is editable. What do you think?
The fifth version with StaticText:
