wxGUI: Statusbar settings as a part of Map Display Settings #2153

lindakarlovska · 2022-02-03T10:10:12Z

This PR creates several radiobuttons in Map Display settings that control which widget will be shown in statusbar.

This PR needs to wait for some refactoring that has to be done for mapdisp/statusbar.py file.

petrasovaa · 2022-02-03T14:23:51Z

Show everywhere seems redundant. Main slot settings is unclear, maybe Displayed content/information?

lindakarlovska · 2022-02-03T17:09:53Z

Show everywhere seems redundant. Main slot settings is unclear, maybe Displayed content/information?

Yes, that would be better.

lindakarlovska · 2022-02-03T17:21:07Z

Show everywhere seems redundant. Main slot settings is unclear, maybe Displayed content/information?

Yes, that would be better.

Maybe, we should also adapt terms Display extent, Computational region and Computational extent. I think they are quite confusing. Display extent is called Display Geometry, but if I understand it well it should display Display resolution. And Region extent vs. Computational region extent... I am not sure at all what is the difference between them apart from the fact that computational region displays two extra information. What do your think @petrasovaa ? Do you have some ideas for better naming?
We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

petrasovaa · 2022-02-06T04:40:08Z

We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

We could use: Display resolution, Display extent, Computational region

petrasovaa · 2022-02-06T04:52:58Z

gui/wxpython/mapdisp/properties.py

+    def __init__(self, parent, mapWindowProperties, value, label, style=0):
+        PropertyItem.__init__(self, mapWindowProperties)
+        self.value = value
+        self.widget = wx.RadioButton(


This should be RadioBox, that should simplify a lot of the code below.

petrasovaa · 2022-02-06T04:54:58Z

gui/wxpython/mapdisp/properties.py

+    coordinates = (1,)
+    mapscale = (2,)
+    compRegion = (3,)
+    goTo = (4,)
+    compExtent = (5,)
+    displayGeometry = 6


Why tuples here? And the last one is int?

Interesting. I did not make it, that had to be caused by Black.

gui/wxpython/mapwin/base.py

lindakarlovska · 2022-02-07T15:54:05Z

We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

We could use: Display resolution, Display extent, Computational region

I was thinking about it more and I like a "canvas" word. It seems to me a bit clearer than display but the criticism is welcome. :-)

Now it looks as follows:

veroandreo · 2022-02-07T20:16:06Z

That would be a new word for GRASS, no? What would it refer to: the map display region, right? Why call it Geographical?

Also, what would canvas resolution refer to? Currently only the computational region option in the status bar shows resolution in between parenthesis. Is there a way to have a map display resolution different than that of the computational region?

We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

We could use: Display resolution, Display extent, Computational region

I was thinking about it more and I like a "canvas" word. It seems to me a bit clearer than display but the criticism is welcome. :-)

Now it looks as follows:

That would be a new word for GRASS, no? It refers to the "map display region/extent", right? I think the term is used in QGIS (at least I've seen it when exporting maps from there) so users might be familiar with it, but to my knowledge, it is new to GRASS. I wonder if this will have then further implications in terms of GRASS terminology? If we use canvas here, should we also change "Map display" to canvas everywhere? Also, why call it Geographical? Will there be other kind of canvas?

Currently only the computational region option in the status bar shows resolution in between parenthesis.
What would canvas resolution refer to? Is there a way to have/set a map display resolution different than that of the computational region?

I'm thinking out loud here, apologies if these are silly questions

petrasovaa · 2022-02-08T04:16:29Z

We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

We could use: Display resolution, Display extent, Computational region

I was thinking about it more and I like a "canvas" word. It seems to me a bit clearer than display but the criticism is welcome. :-)

Now it looks as follows:

I would still go with my suggestion, as Vero points out, canvas is not a term we use and I don't think it particularly helps here.

I thought about merging the Display extent and Display resolution into Display region, which would be more consistent with Computational region, but if we would do that, that would be a separate PR.

petrasovaa · 2022-02-08T04:20:23Z

What would canvas resolution refer to? Is there a way to have/set a map display resolution different than that of the computational region?

By default the rendering (display) resolution is different than computational, it's based on the extent and display size, e.g. (n - s) / height in px.

lindakarlovska · 2022-02-09T15:12:01Z

That would be a new word for GRASS, no? What would it refer to: the map display region, right? Why call it Geographical?

Also, what would canvas resolution refer to? Currently only the computational region option in the status bar shows resolution in between parenthesis. Is there a way to have a map display resolution different than that of the computational region?

I think you hit a nail on its head, @veroandreo :-) That "Display resolution" is exactly the point when it becomes confusing. If I get it right it is given in pixels whereas Display Extent is given in coordinates. So, that's the reason why I suggested adding 'Geographical' before a word 'canvas extent' .... I agree that the term 'canvas' is too big intervention but let's try to think about this issue a bit. For example, I haven't found anywhere in the docs the explanation of what Display resolution actually means. Maybe it would help to add 'Map' and have 'Map Display resolution'. Or we could have 'Map Display info', 'Map Display technical info'..... anyway, I think we probably need to explain it more.

We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

We could use: Display resolution, Display extent, Computational region

I was thinking about it more and I like a "canvas" word. It seems to me a bit clearer than display but the criticism is welcome. :-)
Now it looks as follows:

That would be a new word for GRASS, no? It refers to the "map display region/extent", right? I think the term is used in QGIS (at least I've seen it when exporting maps from there) so users might be familiar with it, but to my knowledge, it is new to GRASS. I wonder if this will have then further implications in terms of GRASS terminology? If we use canvas here, should we also change "Map display" to canvas everywhere? Also, why call it Geographical? Will there be other kind of canvas?

Currently only the computational region option in the status bar shows resolution in between parenthesis. What would canvas resolution refer to? Is there a way to have/set a map display resolution different than that of the computational region?

I'm thinking out loud here, apologies if these are silly questions

lindakarlovska · 2022-02-09T15:15:58Z

We could probably use Display resolution, Region extent and Computational region extent. But I think it is still confusing.

We could use: Display resolution, Display extent, Computational region

I was thinking about it more and I like a "canvas" word. It seems to me a bit clearer than display but the criticism is welcome. :-)
Now it looks as follows:

I would still go with my suggestion, as Vero points out, canvas is not a term we use and I don't think it particularly helps here.

I thought about merging the Display extent and Display resolution into Display region, which would be more consistent with Computational region, but if we would do that, that would be a separate PR.

I am not sure if it helps. We can try it but I think we might need more to improve the documentation..

wenzeslaus

I think the two columns should not be there. It should be just one column.

I went through Preferences/Settings in several applications (Gimp, Inkscape, Spyder, QtCreator, VSCodium) and found two columns only in QtCreator in settings for Debugger and for Vim, so clearly something extreme. And even there it was for more than 10 items and they were checkboxes and exclusive choice radio buttons, so it is questionable if that even applies here.

lindakarlovska · 2022-02-10T17:52:29Z

I think the two columns should not be there. It should be just one column.

I went through Preferences/Settings in several applications (Gimp, Inkscape, Spyder, QtCreator, VSCodium) and found two columns only in QtCreator in settings for Debugger and for Vim, so clearly something extreme. And even there it was for more than 10 items and they were checkboxes and exclusive choice radio buttons, so it is questionable if that even applies here.

We can make it as one column, I am okay with that. :-).

petrasovaa · 2022-02-11T18:15:01Z

I don't like the mode thing. Not sure if i don't overlook something, but perhaps creating a new class inheriting from RBShowInStatusbar and override e.g., a _getOptions method (containing the labels) would be better, or?

lindakarlovska · 2022-02-22T21:07:02Z

The 3D/2D stuff is missing. We need to adapt ShowStatusbarChoiceItemsByClass and HideStatusbarChoiceItemsByClass. I think It should emit the signal which shows/hides buttons in the Map Display settings radiobox. The Combobox had reacted as well on 2D/3D switching so we should probably preserve that thought.

I was thinking about it a bit more and we do not probably need any signals, the dialog does not have to response dynamically... What do you think @petrasovaa ?
Btw, it is still not finished I need to test other Map Display apps, hopefully, tomorrow, it will be ready for review. :-)

petrasovaa · 2022-02-23T14:51:03Z

I was thinking about it a bit more and we do not probably need any signals, the dialog does not have to response dynamically... What do you think @petrasovaa ? Btw, it is still not finished I need to test other Map Display apps, hopefully, tomorrow, it will be ready for review. :-)

I agree, let's keep it simple.

lindakarlovska · 2022-02-24T12:54:09Z

I was thinking about it a bit more and we do not probably need any signals, the dialog does not have to response dynamically... What do you think @petrasovaa ? Btw, it is still not finished I need to test other Map Display apps, hopefully, tomorrow, it will be ready for review. :-)

I agree, let's keep it simple.

@petrasovaa I think it is ready to merge. I think disabling items is probably more readable than hiding them so I changed that. I think the code is also simpler as we do not need to change the layout of the radiobox either.
So it looks as follows now:
2D (basic):

3D (two items in settings disabled):

GCP apps (as Image2Target and others):

veroandreo · 2022-02-24T13:53:19Z

@petrasovaa I think it is ready to merge. I think disabling items is probably more readable than hiding them so I changed that. I think the code is also simpler as we do not need to change the layout of the radiobox either. So it looks as follows now:
2D (basic):

Given that it says Display content above, is it needed to write display again in the Display extent and Display geometry items? (same comment applies to those below)

3D (two items in settings disabled):

GCP apps (as Image2Target and others):

Would it be possible to write Number instead of No.?

wenzeslaus · 2022-02-24T14:27:40Z

Given that it says Display content above, is it needed to write display again in the Display extent and Display geometry items? (same comment applies to those below)

I think we have to keep display there. It is the display extent we are showing, not the computational region extent.

Would it be possible to write Number instead of No.?

Yes, please! But perhaps the label is somewhat incomplete anyway. Go to GCP number... or Pan to GCP by number?

veroandreo · 2022-02-24T14:37:25Z

Given that it says Display content above, is it needed to write display again in the Display extent and Display geometry items? (same comment applies to those below)

I think we have to keep display there. It is the display extent we are showing, not the computational region extent.

Now I get it, it refers to the map display (I read it as the verb display and not the noun referring to map display)... I'd suggest then to use Map display extent and Map display geometry to be crystal clear

Would it be possible to write Number instead of No.?

Yes, please! But perhaps the label is somewhat incomplete anyway. Go to GCP number... or Pan to GCP by number?

lindakarlovska · 2022-02-24T16:16:17Z

Given that it says Display content above, is it needed to write display again in the Display extent and Display geometry items? (same comment applies to those below)

I think we have to keep display there. It is the display extent we are showing, not the computational region extent.

Now I get it, it refers to the map display (I read it as the verb display and not the noun referring to map display)... I'd suggest then to use Map display extent and Map display geometry to be crystal clear

Would it be possible to write Number instead of No.?

Yes, please! But perhaps the label is somewhat incomplete anyway. Go to GCP number... or Pan to GCP by number?

I agree with you :-) Adding "Map" could help to understand those concepts. I previously suggested renaming the No. to number but we needed to focus on the implementation more so I removed that then.. Now it is a good time to discuss it again and change labels if needed. I think "Pan to GCP by number" is also good. I was thinking about "Focus GCP by number". but @wenzeslaus 's proposal is probably better.

gui/wxpython/mapdisp/statusbar.py

petrasovaa · 2022-02-28T05:11:34Z

I attached a diff with my changes, I didn't want to apply it directly since it's not completely finished and you may have your changes locally.
sbdiff.txt

The changes are adding the context menu in statusbar. Currently the item in the context menu is not always selected correctly and there may be other issues, but this is what I have now.

…o Map Display Settings (OSGeo#2153) Creates several radiobuttons in Map Display settings that control which item will be shown in statusbar.

lindakarlovska marked this pull request as draft February 3, 2022 10:10

lindakarlovska added the GUI wxGUI related label Feb 3, 2022

lindakarlovska added this to the 8.2.0 milestone Feb 3, 2022

lindakarlovska added the enhancement New feature or request label Feb 3, 2022

petrasovaa reviewed Feb 6, 2022

View reviewed changes

gui/wxpython/mapwin/base.py Outdated Show resolved Hide resolved

wenzeslaus reviewed Feb 9, 2022

View reviewed changes

lindakarlovska force-pushed the wxGUI-redesign-statusbar-widgets-settings branch from 0319e19 to 1027eb1 Compare February 11, 2022 15:44

sbmanager as a parameter, in progress

f5e5abc

lindakarlovska force-pushed the wxGUI-redesign-statusbar-widgets-settings branch from e78aea6 to f5e5abc Compare February 17, 2022 16:09

lindakladivova added 7 commits February 17, 2022 10:11

remove files pushed by mistake

bdc0fc8

make label more readable

1cd77e6

signal is connected in RBShowInStatusbar

8236966

edit label

a789e0a

switching statusbar modes through mapdisplay settings

f9ceef9

black and simplification

f1fc70b

small edits

2b0cf98

lindakladivova added 2 commits February 22, 2022 15:10

list comprehension

6569b12

black, signal removed

37bcf2b

lindakladivova added 4 commits February 23, 2022 09:02

desc, problem with other map disp apps

2cb5667

Update statusbar from radiobox, SetMode just sets the mode

d4ec49f

edit descriptions

31cbdd4

fix combobox in preferences dialog

eb25021

lindakarlovska marked this pull request as ready for review February 23, 2022 16:35

lindakarlovska requested a review from petrasovaa February 23, 2022 16:35

lindakladivova added 2 commits February 24, 2022 06:16

items disabled rather than hidden

dab55b6

black

0b18dab

petrasovaa requested changes Feb 28, 2022

View reviewed changes

gui/wxpython/mapdisp/statusbar.py Outdated Show resolved Hide resolved

lindakladivova added 3 commits March 1, 2022 12:23

keep the previous code for Show() method

4db0b45

GCP labels changed

60fc78e

parameter names changed

61da223

lindakarlovska requested a review from petrasovaa March 1, 2022 19:00

petrasovaa approved these changes Mar 2, 2022

View reviewed changes

petrasovaa merged commit 28a86c9 into OSGeo:main Mar 2, 2022

This was referenced Mar 2, 2022

[Feat] wxGUI: Redesigning Map Display status bar combo box into a new Map Display Settings dialog #2017

Closed

wxGUI: new context menu for statusbar settings #2251

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wxGUI: Statusbar settings as a part of Map Display Settings #2153

wxGUI: Statusbar settings as a part of Map Display Settings #2153

lindakarlovska commented Feb 3, 2022

petrasovaa commented Feb 3, 2022

lindakarlovska commented Feb 3, 2022 •

edited

Loading

lindakarlovska commented Feb 3, 2022 •

edited

Loading

petrasovaa commented Feb 6, 2022

petrasovaa Feb 6, 2022

petrasovaa Feb 6, 2022

lindakarlovska Feb 7, 2022

lindakarlovska commented Feb 7, 2022

veroandreo commented Feb 7, 2022

petrasovaa commented Feb 8, 2022

petrasovaa commented Feb 8, 2022

lindakarlovska commented Feb 9, 2022

lindakarlovska commented Feb 9, 2022

wenzeslaus left a comment

lindakarlovska commented Feb 10, 2022 •

edited

Loading

petrasovaa commented Feb 11, 2022

lindakarlovska commented Feb 22, 2022

petrasovaa commented Feb 23, 2022 •

edited

Loading

lindakarlovska commented Feb 24, 2022 •

edited

Loading

veroandreo commented Feb 24, 2022

wenzeslaus commented Feb 24, 2022

veroandreo commented Feb 24, 2022

lindakarlovska commented Feb 24, 2022

petrasovaa commented Feb 28, 2022

wxGUI: Statusbar settings as a part of Map Display Settings #2153

wxGUI: Statusbar settings as a part of Map Display Settings #2153

Conversation

lindakarlovska commented Feb 3, 2022

petrasovaa commented Feb 3, 2022

lindakarlovska commented Feb 3, 2022 • edited Loading

lindakarlovska commented Feb 3, 2022 • edited Loading

petrasovaa commented Feb 6, 2022

petrasovaa Feb 6, 2022

Choose a reason for hiding this comment

petrasovaa Feb 6, 2022

Choose a reason for hiding this comment

lindakarlovska Feb 7, 2022

Choose a reason for hiding this comment

lindakarlovska commented Feb 7, 2022

veroandreo commented Feb 7, 2022

petrasovaa commented Feb 8, 2022

petrasovaa commented Feb 8, 2022

lindakarlovska commented Feb 9, 2022

lindakarlovska commented Feb 9, 2022

wenzeslaus left a comment

Choose a reason for hiding this comment

lindakarlovska commented Feb 10, 2022 • edited Loading

petrasovaa commented Feb 11, 2022

lindakarlovska commented Feb 22, 2022

petrasovaa commented Feb 23, 2022 • edited Loading

lindakarlovska commented Feb 24, 2022 • edited Loading

veroandreo commented Feb 24, 2022

wenzeslaus commented Feb 24, 2022

veroandreo commented Feb 24, 2022

lindakarlovska commented Feb 24, 2022

petrasovaa commented Feb 28, 2022

lindakarlovska commented Feb 3, 2022 •

edited

Loading

lindakarlovska commented Feb 3, 2022 •

edited

Loading

lindakarlovska commented Feb 10, 2022 •

edited

Loading

petrasovaa commented Feb 23, 2022 •

edited

Loading

lindakarlovska commented Feb 24, 2022 •

edited

Loading