Skip to content

wxGUI/toolbars: fix toolbar tools labels #1147

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
Merged
merged 47 commits into from
Mar 25, 2022
Merged

wxGUI/toolbars: fix toolbar tools labels #1147

merged 47 commits into from
Mar 25, 2022

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented Dec 4, 2020

Fixes #1133.

So far, I've only fixed the labels for the Map Display toolbar tools.

wxgui_mapdisplay_toolbar_tools_labels_def_exp

@tmszi tmszi added bug Something isn't working GUI wxGUI related labels Dec 4, 2020
@tmszi tmszi marked this pull request as draft December 4, 2020 12:00
wenzeslaus
wenzeslaus requested changes Dec 14, 2020
View reviewed changes
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

It works for me as expected (that is without the icons for me) and the code looks fine, but I think the labels need to be little more better. I don't think they should be as long as tooltips. They should be more like button texts, but they should be more than just keywords, so probably "Zoom to region" and "Zoom to extent" or something like that (the rest look fine).

@neteler neteler added this to the 8.0.1 milestone Dec 9, 2021
@tmszi tmszi force-pushed the fix-wxgui-toolbar-widget-tool-labels branch from d497b92 to 347f5ae Compare January 26, 2022 09:28
@tmszi tmszi marked this pull request as ready for review January 26, 2022 11:36
veroandreo
veroandreo requested changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

I added some minor comments, they have to do mostly with consistency. Thanks @tmszi :)

@tmszi tmszi requested a review from veroandreo January 28, 2022 14:13
veroandreo
veroandreo reviewed Jan 28, 2022
View reviewed changes
@veroandreo
Copy link
Contributor

veroandreo commented Jan 28, 2022 •
edited
Loading

Just tested again, and found that if I shrink the screen enough, I see this:

Screenshot_2022-01-28_15-55-06

some of the icons do not appear. I see this both with single-window and split window modes.

@tmszi
Copy link
Member Author

tmszi commented Jan 28, 2022

Just tested again, and found that if I shrink the screen enough, I see this:
some of the icons do not appear. I see this both with single-window and split window modes.

Yes you are right, because toolbar tool e.g Zoom out is wx.ITEM_CHECK type, for a checkable tool (such tool stays pressed after it had been toggled). It is default behavior I'm not changed it with this PR.

grass/gui/wxpython/mapdisp/toolbars.py

Line 178 in 1c300f6

("zoomOut", BaseIcons["zoomOut"], self.parent.OnZoomOut, wx.ITEM_CHECK),

mapdisplay_zoom_out_toolbar_tool

@veroandreo
Copy link
Contributor

Just tested again, and found that if I shrink the screen enough, I see this:
some of the icons do not appear. I see this both with single-window and split window modes.

Yes you are right, because toolbar tool e.g Zoom out is wx.ITEM_CHECK type, for a checkable tool (such tool stays pressed after it had been toggled). It is default behavior I'm not changed it with this PR.

grass/gui/wxpython/mapdisp/toolbars.py

Line 178 in 1c300f6

("zoomOut", BaseIcons["zoomOut"], self.parent.OnZoomOut, wx.ITEM_CHECK),

mapdisplay_zoom_out_toolbar_tool

Good! Thanks for the explanation :) I was not sure if it was actually related or not. +1 for merging from my side

@ninsbl
Copy link
Member

ninsbl commented Feb 20, 2022

If there is an RC2 for 8.0.1, can this make it into the milestone? Or should we bump up to 8.0.2?

@neteler
Copy link
Member

neteler commented Feb 20, 2022

"This branch has conflicts that must be resolved"

In the first place the PR needs to be rebased.

@tmszi tmszi modified the milestones: 8.0.1, 8.2.0 Feb 20, 2022
@tmszi tmszi force-pushed the fix-wxgui-toolbar-widget-tool-labels branch from 409dd58 to 9933f71 Compare February 20, 2022 16:02
@tmszi
Copy link
Member Author

tmszi commented Feb 20, 2022

@ninsbl @neteler Rebase + tested again from my side.

@neteler neteler requested a review from veroandreo February 20, 2022 16:21
@neteler
Copy link
Member

neteler commented Mar 25, 2022

@tmszi may this be merged and backported? We are getting close to the releases.

@tmszi tmszi merged commit 7d79d82 into OSGeo:main Mar 25, 2022
@neteler
Copy link
Member

neteler commented Mar 25, 2022

I tried to cherry-pick backport this but too many merge conflicts arise...
Apparently another PR needs to be backported, too?

@wenzeslaus
Copy link
Member

This is a large change touching many files and many lines while not fixing any critical bugs, so let's not backport it.

@tmszi tmszi deleted the fix-wxgui-toolbar-widget-tool-labels branch March 26, 2022 02:59
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@tmszi @ninsbl
wxGUI/toolbars: fix toolbar tools labels when toolbar width is shrink…
52f8a51 
…ed and toolbar menu is created (OSGeo#1147)

* wxGUI/animation
* wxGUI/datacatalog
* wxGUI/example/toolbar
* wxGUI/gcp
* wxGUI/gmodeler
* wxGUI/gui_core/pyedit
* wxGUI/gui_core/simplelmgr
* wxGUI/gui_core/toolbars
* wxGUI/iclass
* wxGUI/image2target
* wxGUI/iscatt
* wxGUI/lmgr
* wxGUI/lmgr/toolbars
* wxGUI/mapdisp
* wxGUI/mapdisplay
* wxGUI/mapswipe
* wxGUI/modules/histogram
* wxGUI/photo2image
* wxGUI/psmap
* wxGUI/rdigit
* wxGUI/rlisetup
* wxGUI/vdigit
* wxGUI/vnet
* wxGUI/wxplot/histogram
* wxGUI/wxplot/profile
* wxGUI/wxplot/scatter
* wxGUIgui/gui_core/simplelmgr
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@tmszi @ninsbl
wxGUI/toolbars: fix toolbar tools labels when toolbar width is shrink…
8028c5f 
…ed and toolbar menu is created (OSGeo#1147)

* wxGUI/animation
* wxGUI/datacatalog
* wxGUI/example/toolbar
* wxGUI/gcp
* wxGUI/gmodeler
* wxGUI/gui_core/pyedit
* wxGUI/gui_core/simplelmgr
* wxGUI/gui_core/toolbars
* wxGUI/iclass
* wxGUI/image2target
* wxGUI/iscatt
* wxGUI/lmgr
* wxGUI/lmgr/toolbars
* wxGUI/mapdisp
* wxGUI/mapdisplay
* wxGUI/mapswipe
* wxGUI/modules/histogram
* wxGUI/photo2image
* wxGUI/psmap
* wxGUI/rdigit
* wxGUI/rlisetup
* wxGUI/vdigit
* wxGUI/vnet
* wxGUI/wxplot/histogram
* wxGUI/wxplot/profile
* wxGUI/wxplot/scatter
* wxGUIgui/gui_core/simplelmgr
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@tmszi @neteler
wxGUI/toolbars: fix toolbar tools labels when toolbar width is shrink…
ca49546 
…ed and toolbar menu is created (OSGeo#1147)

* wxGUI/animation
* wxGUI/datacatalog
* wxGUI/example/toolbar
* wxGUI/gcp
* wxGUI/gmodeler
* wxGUI/gui_core/pyedit
* wxGUI/gui_core/simplelmgr
* wxGUI/gui_core/toolbars
* wxGUI/iclass
* wxGUI/image2target
* wxGUI/iscatt
* wxGUI/lmgr
* wxGUI/lmgr/toolbars
* wxGUI/mapdisp
* wxGUI/mapdisplay
* wxGUI/mapswipe
* wxGUI/modules/histogram
* wxGUI/photo2image
* wxGUI/psmap
* wxGUI/rdigit
* wxGUI/rlisetup
* wxGUI/vdigit
* wxGUI/vnet
* wxGUI/wxplot/histogram
* wxGUI/wxplot/profile
* wxGUI/wxplot/scatter
* wxGUIgui/gui_core/simplelmgr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wenzeslaus wenzeslaus wenzeslaus approved these changes

@petrasovaa petrasovaa petrasovaa approved these changes

@veroandreo veroandreo veroandreo approved these changes

@neteler neteler Awaiting requested review from neteler

Assignees
No one assigned
Labels
bug Something isn't working GUI wxGUI related
Projects
None yet
Milestone
8.2.0
Development

Successfully merging this pull request may close these issues.

[Bug] Narrow toolbar shows IDs instead of labels
6 participants
@tmszi @veroandreo @ninsbl @neteler @wenzeslaus @petrasovaa