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/nviz: fix showing scrollbars on the Data, Appearance, Analysis page (tab) #3250

Merged
merged 3 commits into from
May 2, 2024

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented Nov 19, 2023

Fixes #3224. Successfully tested on the macOS, MS Windows, GNU/Linux OS.

Screenshots

macOS

wxGUI NVIZ show scrollbars on the macOS

MS Windows

wxGUI NVIZ show scrollbars on the MS Windows OS

Additional context

Solution require complete different approach and contains one non-standard workaround. I found that the vertical scrollbar is showed on notebook pages that contain the FoldPanelBar widget, not because the standard SetVirtualSize() method is called (is not working as expected), but because the nested scrolled panel with the horizontal scrollbar contains the wx.Choice widget and after adding all the notebook pages it is called the UpdatePages() method, in which the SetItems() method is called for the wx.Choice widget, which among other things, ensures the display of a vertical scroll bar (this applies to notebook page Data and Appearance, but the vertical scrollbar is not displayed for Analysis page because it does not contain the wx.Choce widget).

For this reason, I added a hidden wx.Choice widget to each notebook page that contains a FoldPanelBar widget on the first FoldPanel widget which contains a horizontal scrollbar. After adding all the notebook pages, the hidden wx.Choice widgets SetItem() method is called, which ensures recalculation of the ScrolledPanel window size with a vertical scrollbar and displays it.

  1. Data notebook page -> ScrolledPanel with vertical scrollbar -> FoldPanelBar -> Surface FoldPanel-> ScrolledPanel with horizontal scrollbar -> hidden Choice widget

  2. Appearance notebook page -> ScrolledPanel with vertical scrollbar -> FoldPanelBar -> Lighting FoldPanel -> ScrolledPanel with horizontal scrollbar -> hidden Choice widget

  3. Analysis notebook page -> ScrolledPanel with vertical scrollbar -> FoldPanelBar -> Cutting FoldPanel -> ScrolledPanel with horizontal scrollbar -> hidden Choice widget

I couldn't find a better cleaner solution.

Could @cmbarton and @nilason test it and confirm expected behavior on macOS, please?

@tmszi tmszi added bug Something isn't working GUI wxGUI related macOS macOS specific backport to 8.3 labels Nov 19, 2023
@tmszi tmszi added this to the 8.4.0 milestone Nov 19, 2023
@tmszi tmszi self-assigned this Nov 19, 2023
@petrasovaa
Copy link
Contributor

Thanks for the analysis, but looking at the changes I am worried this is getting out of hand. The code is already complicated and this relies on undocumented behavior. How about just dropping this problematic widget altogether, and use standard tabs (at the botom) instead of the FoldPanelBar? The motivation was to avoid second row of tabs at the bottom, but at this point, I think maintainable code with standard widgets is more important for me. Any thoughts?

@nilason
Copy link
Contributor

nilason commented Nov 20, 2023

I'd also would like to thank you @tmszi for the efforts you put into this (including setting up a VM!)
I can confirm, it does work as intended!

I also felt/feel a bit unsure on the amount of code needed (?) for this. I'm ambivalent. Double set of tabs doesn't sound too compelling from a user perspective...

Another thing, the whole panel takes up so much horizontal space, nearly a third of my monitors, there's room for improvement on the layout on this.

@tmszi
Copy link
Member Author

tmszi commented Nov 27, 2023

Thanks for the analysis, but looking at the changes I am worried this is getting out of hand. The code is already complicated and this relies on undocumented behavior. How about just dropping this problematic widget altogether, and use standard tabs (at the botom) instead of the FoldPanelBar? The motivation was to avoid second row of tabs at the bottom, but at this point, I think maintainable code with standard widgets is more important for me. Any thoughts?

I tried to refactor it using standard AlwaysShowScrollbars() method , the code is now maintainable without the non-standard/undocumented parts.

@cmbarton
Copy link
Contributor

I strongly support more easily maintainable code, especially because GRASS depends on volunteers who often need to rotate in and out of coding.

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.

I don't want to block this, since it seems to fix a bug. Larger interface changes are needed to address this properly anyway.
Note I didn't actually tested this myself.

@github-actions github-actions bot added the Python Related code is in Python label Apr 26, 2024
@cmbarton
Copy link
Contributor

I don't want to block this, since it seems to fix a bug. Larger interface changes are needed to address this properly anyway. Note I didn't actually tested this myself.

Agreed. I have tested. It does help the main window but is needed for other NVIZ windows too. Beyond that, as I noted, the main problem is that the NVIZ interface is simply too large and complicated to fit into a small pane in the single window interface. It would be a better solution to just have it launch in an independent window.

@petrasovaa petrasovaa merged commit 943de90 into OSGeo:main May 2, 2024
26 checks passed
HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working GUI wxGUI related macOS macOS specific Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] wxGUI/nviz: tabs/pages with foldpanels are broken on Mac
5 participants