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

fix Sidebar to resize better #512

Merged
merged 1 commit into from Feb 8, 2018

Conversation

prculley
Copy link
Contributor

@prculley prculley commented Dec 27, 2017

Fixes #10334 (or at least helps) Filter sidebar loses RH controls when adjusting sidebar width down.
Issue #10161 Difficult to expand Sidebar on Windows after it is shrunk to nothing.

The sidebar is currently set to be the smaller of 25% of SCREEN width or 400 pix, whatever is smaller. For large screens (particularly multi-monitor), you end up with a 400 pix wide sidebar, no matter what size the Gramps window is.

As the bug reporter noted, if you try to shrink the sidebar, you lose the right hand portion of it.
The undersized sidebar also did not have a bottom scroll bar to allow the user to scroll to see all of it.

I think it would be better to allow the sidebar contents to shrink with the sidebar to their 'natural' minimum. It should help a lot with the reporter's issue since the natural minimum is a lot smaller than 400pix.

Further, if you shrink the sidebar to nothing, it becomes very hard to expand it again #10161. So I further suggest limiting the sidebar minimum size to its overall natural minimum. That way it cannot be eliminated completely, but the remaining amount is pretty obviously too small and less likely to just hide the sidebar right hand side contents. I note that the results are a bit different on Windows and Ubuntu Linux, probably due to different CSS. The user can always eliminate the sidebar via the 'View' menu.

With these changes, an undersized sidebar will also have a scroll bar at the bottom of the sidebar.

I also set up the default sidebar size to be dependent on the window size. This only applies for first run until user adjusts manually. Tested and looks OK for Gramps windows at 800, 1280, and 1920 wide (delete ".gramps/gramps50/People_personview.ini" etc. to test).

@prculley prculley added the bug label Dec 27, 2017
@codecov-io
Copy link

codecov-io commented Dec 27, 2017

Codecov Report

Merging #512 into maintenance/gramps50 will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@                   Coverage Diff                    @@
##           maintenance/gramps50     #512      +/-   ##
========================================================
- Coverage                 42.36%   42.35%   -0.01%     
========================================================
  Files                      1064     1064              
  Lines                    141972   141983      +11     
========================================================
  Hits                      60140    60140              
- Misses                    81832    81843      +11
Impacted Files Coverage Δ
gramps/gui/views/pageview.py 0% <0%> (ø) ⬆️
gramps/gui/widgets/grampletbar.py 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f4a114...d3ed320. Read the comment docs.

@Nick-Hall Nick-Hall self-assigned this Dec 31, 2017
@Nick-Hall
Copy link
Member

Preventing the sidebar shrinking to zero width is useful. However, I don't like the sidebar changing width when resizing the main window.

Most users will want the sidebar to be the preferred width of the gramplets it contains, so that a scrollbar is not necessary. For most gramplets this will be about 400px, but this will vary depending on font size and translations. There is a special case for small screens because a 400px wide sidebar looks silly on a 800x600 display.

We also want the user to have the ability to set the sidebar to a fixed width. See feature request 7116.

@prculley
Copy link
Contributor Author

prculley commented Jan 7, 2018

@Nick-Hall I fixed it so the sidebar does not resize with the main window. That was actually an oversight on my part from earlier experimentation.

The viewport (with the scrollbar) has always been there, it was originally over-ridden by the forced sidebar minimum size of 400px. I have left the change for minimum size to be 'natural', as that seem to me to be the best way to reduce the incidence of cut off right hand elements when manually resized less than 400px. As a consequence, the scrollbar will reappear if the size is manually reduced below the natural size.
And the user who doesn't feel a need for 400px width can get by nicely with less (at least for filter and some other Gramplets).

Note that a new user still gets a 400px wide sidebar (if his screen is large enough) until he manually resizes, thereafter his manual setting 'sticks' per your code for saving of position across sessions.

@Nick-Hall
Copy link
Member

This is an improvement, but the default width is now too small.

For example, on a 1280 width screen, the default sidebar width before this PR was 320, which allowed all the filters to be displayed without scrollbars. Now the default size requires a scrollbar for most sidebars and a new user would probably want to resize these.

The initial main window size is small so that it will fit on a 800x600 display. Using the screen size is probably better then the initial window size for calculations. Alternatively, perhaps we could consider using the preferred width of the child widget in the scrollbar instead?

@prculley prculley force-pushed the bug10334 branch 2 times, most recently from 99b4737 to 9804092 Compare January 8, 2018 22:31
@prculley
Copy link
Contributor Author

prculley commented Jan 8, 2018

@Nick-Hall So I spent some time figuring out how to allocate the 'natural' widget size to the sidebar for the initial start.
It looks ok on my systems (see image below 800px wide). If it takes a 320px sidebar to avoid a scrollbar on your system then there must be some considerable changes from CSS or somewhere, on my systems it was ~230px.

Note that this is still approximate, since I cannot figure out how to determine the width of the overall pane, as it has not been realized yet.
I don't think using a screen size as a basis would be a good idea, for instance, on my system the 'screen' width is 2880px wide as reported by Gtk, with dual monitors, on initial start Gramps appears on my primary 1280 wide monitor.
sidebar

@Nick-Hall
Copy link
Member

@prculley Yes. There are significant differences between different themes and translations. The default value of 400px was chosen so that scrollbars were not required for most common themes and translations. We also discussed the idea of a laptop mode for small screens. However, this was rejected in favour of a small tweak based on screen width.

The way that displays, screens and monitors are handled probably changed for Gtk3. I seem to remember that a display could have multiple screens. There still appears to be a concept of a primary monitor. The initial window size is the same for all resolutions (775x500), so it doesn't give an indication of screen size.

@lordemannd
Copy link
Contributor

lordemannd commented Jan 27, 2018

@prculley i wanted to see if this also solved #0010216, unfortunately I just updated to the end of gramps50 branch, it looks like there is now some conflict with these files, can you give guidance on how can I combine these changes?

@prculley
Copy link
Contributor Author

@lordemannd I rebased to the current Gramps50, it should apply cleanly now.

@lordemannd
Copy link
Contributor

lordemannd commented Jan 27, 2018

@prculley, thank you for the rebase, I have tested it and yes it solves #0010216 for me on Windows. I like it.

@adamvazquez82
Copy link

@prculley This is wonderful and better than the current behavior 🥇

@Nick-Hall
Copy link
Member

@prculley The new functionality is good, but the initial width is still too small on a default Gnome desktop. We must make the inital width big enough to avoid scrollbars on all but the lowest supported screen resolutions. I attach a screenshot so that you can see the problem.

filter

Once this issue is solved I will happy to merge this PR.

@prculley
Copy link
Contributor Author

@Nick-Hall Would you be so kind as to take screen grabs of the whole Gramps window as it comes up in the default startup (no gramps.ini and no personview.ini). I would also appreciate a grab with the sidebar adjusted so that it shows everything.

I note that with the CSS in your system, the elements are so large that a user will have to use a vertical scroll bar to see the whole filter.

Attached is a screenshot of Gramps as it is for Ubuntu 16.04 (I guess this is Unity, not quite Gnome). What distro are you using? I guess I have to get that set up in a VM too.

I'd love to make the initial size computed, but since widgets are not all realized at the point where I'm setting it, I have to make guesses on appropriate values. Maybe I should find a good place to come back and resize after everything is done?
ubuntu

@Nick-Hall
Copy link
Member

This is the initial screen:
initial

This is what it looks like when resized to the minimum size without scrollbars:
resized

I am using Arch Linux and Gnome with the default Adwaita theme.

I notice that the ScrolledWindow class now has a set_propagate_natural_width method. This appears to be what we need, but unfortunately it is only available in Gtk 3.22 and above. Perhaps we could get the same result be overloading the preferred size methods?

@lordemannd
Copy link
Contributor

lordemannd commented Jan 29, 2018

On debian, I notice a difference between the side bar on flat People view and the Grouped People view, are they completely separate views?

@prculley
Copy link
Contributor Author

prculley commented Jan 29, 2018

@Nick-Hall clearly something weird is going on here. According to my calculations based on the screen caps, the sidebar should have been fine.

Nick, if you would be willing to set a breakpoint at gui.views.pageview.py lin 170 and tell me what ch_width, width, and pos values are, I would appreciate it. I am downloading the Arch Linux distro but it is going to take a while with my lousy internet, so I probably won't be able to try debugging there for another couple of days.

@Nick-Hall
Copy link
Member

@lordemannd Yes, they are two separate views.

@Nick-Hall
Copy link
Member

@prculley I'll have a look at this again tomorrow.

@prculley
Copy link
Contributor Author

prculley commented Jan 29, 2018

I found that the Arch Linux loaded faster than expected; only to find it is extremely raw, no GUI at all. I have no idea how long it will take me to figure out what to do to get it running...

I already had a Debian downloaded that I never had used, so I tried that. Debian 9 with Gnome, default theme (Tweak tool says Adwaita), updated and loaded with Gramps50 branch with this PR's patches. Initial startup looks good to me.
debian

@lordemannd
Copy link
Contributor

lordemannd commented Jan 30, 2018

On debian, after going back to current gramps50 branch and adding in the sidebar changes, then deleting all .ini files, and restarting Gramps, it looks vary similar to Windows.
Only difference is that on Linux, the sidebar content doesn't fit in the default width and uses left/right scroll bar. The gramplet control arrow in upper right is visible without scrolling.

screenshot from 2018-01-29 16-17-11

@wroldwiedbwe
Copy link

I found that the Arch Linux loaded faster than expected; only to find it is extremely raw, no GUI at all.

Yes that's right Arch Linux is very much build it yourself from the ground up you probably want to try one of the derivatives like Antergos or Manjaro.

@Nick-Hall
Copy link
Member

Arch Linux provides good documentation, including installation instructions for Gnome. Try:

sudo pacman -S gnome

@Nick-Hall
Copy link
Member

@prculley Some debugging information as requested:

pos = 410
width = 667
ch_width = 257

The width value looks about 50px too large.

@prculley
Copy link
Contributor Author

prculley commented Feb 8, 2018

@Nick-Hall Another try. Based on you debug information, it appears that on your system that gtk is returning a bad? value for the screen size. Presumably because your system has a different version of gtk where the allocated value was not yet valid.

I decided to quit trying to estimate the size of the hpane containing the main section and sidebar and to wait on a signal indicating it had been realized/mapped/was safe to draw on. Advantage, no more estimates. I did add a few pixels to leave room for the vertical scroll-bar.

@lordemannd
Copy link
Contributor

looks good to me on windows 10 and linux/debian

Fixes #10334
Issue #10161
@Nick-Hall Nick-Hall merged commit d3ed320 into gramps-project:maintenance/gramps50 Feb 8, 2018
@Nick-Hall
Copy link
Member

@prculley Thanks. It works now.

The way Gtk works does sometimes change. Arch uses the latest Gtk version, which is why it is good for testing. I also test with Ubuntu 14.04 which uses the oldest Gtk version we currently support.

@prculley prculley deleted the bug10334 branch February 9, 2018 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants