GNOME Bugzilla – Bug 768529
Mouse & Touchpad panel doesn't fit low resolution screens
Last modified: 2016-10-24 08:43:13 UTC
As the title says, I can't use this panel in a 720x480 screen. See following patch.
Created attachment 331013 [details] [review] mouse: adapt to small screens mouse: adapt to small screens When running on a low resolution screen, the hardcoded margins seem to be too much and ends up making the window too big. Fix that my setting the halign property rather than margins, and reducing the height request.
Before and after screenshots would be useful. And please don't assign yourself to bugs, thanks.
There aren't any UI changes, except that the panel is 20px shorter on height. Sorry about the self-assignment, I didn't notice that when attaching the patch.
Review of attachment 331013 [details] [review]: > When running on a low resolution screen, the > hardcoded margins seem to be too much and > ends up making the window too big. > > Fix that my setting the halign property rather > than margins, and reducing the height request. Can you split up the 2 changes as well? ::: panels/mouse/gnome-mouse-properties.ui @@ +22,3 @@ <property name="shadow_type">none</property> <property name="hscrollbar_policy">never</property> + <property name="height_request">440</property> Wouldn't it be easier not to have a height request? It would/should keep the size it has in the main window.
Created attachment 331050 [details] [review] mouse: set max-content-width on scrolledwindows Instead of using only hardcoded height requests, it's better if we give more flexibility for the content to grow up to a certain ammount of pixeld.
Created attachment 331051 [details] [review] mouse: center horizontally using halign property Instead of relying on hardcoded margins to place the content in the middle of the panel, it's better and more reliable to use the horizontal alignment property of GtkWidget.
Created attachment 331052 [details] [review] mouse: remove arbitrary height request The test frame widget size is better handled by the scrolled window's max-content-width property introduced earlier than an arbitrary height request.
Review of attachment 331050 [details] [review]: > a certain ammount of pixeld. "amount" and "pixels"
Review of attachment 331050 [details] [review]: Subject: [PATCH] mouse: set max-content-width on scrolledwindows And you're setting the height, not the width.
Review of attachment 331051 [details] [review]: What's stopping the widget from taking the whole width available without margins? Would be good to explain in the commit message.
Review of attachment 331052 [details] [review]: Sure.
Created attachment 331091 [details] [review] mouse: set max-content-height on scrolledwindows (In reply to Bastien Nocera from comment #8) > Review of attachment 331050 [details] [review] [review]: > > "amount" and "pixels" Done.
Created attachment 331092 [details] [review] mouse: center horizontally using halign property (In reply to Bastien Nocera from comment #10) > Review of attachment 331051 [details] [review] [review]: > > What's stopping the widget from taking the whole width available without > margins? > Would be good to explain in the commit message. I rewrote the entire commit message. See below: The Mouse & Touchpad panel has a horizontally centered list, which is centered pixel-counting the list width and hardcoded margins. This approach has various issues. It resizes the window needlessly when e.g. the font changes the size, dpi or family. This is specially visible when dealing with low resolution screens, where the hardcoded margins are too much to fit a 720x480 screen with the Large Font accessibility setting on. Fix that by removing the margins and setting the horizontal alignment of the list to center. Since the list itself doesn't expand to fill the available space, there won't be any user- visible changes except that the panel is now able to scale down.
Created attachment 331093 [details] [review] mouse: remove arbitrary height request No changes, just to keep patch ordering in Bugzilla.
Review of attachment 331091 [details] [review]: Also mention that you're changing the maximum content height for the panel in the commit message. Looks fine other than that.
Review of attachment 331092 [details] [review]: Great commit message :)
Review of attachment 331093 [details] [review]: Replace "earlier" with the commit ID before pushing please.
Created attachment 331095 [details] [review] mouse: set max-content-height on scrolledwindows Done.
Created attachment 331096 [details] [review] mouse: center horizontally using halign property Thanks.
Created attachment 331097 [details] [review] mouse: remove arbitrary height request Done.
Thanks for the reviews. Attachment 331095 [details] pushed as 62f1f6b - mouse: set max-content-height on scrolledwindows Attachment 331096 [details] pushed as 6c677bb - mouse: center horizontally using halign property Attachment 331097 [details] pushed as a58f9dd - mouse: remove arbitrary height request
Created attachment 338313 [details] screenshow of the new mouse/touchpad panel size (In reply to Georges Basile Stavracas Neto from comment #21) > Attachment 331097 [details] pushed as a58f9dd - mouse: remove arbitrary > height request this one introduced what is IMO an UI regression. On my laptop, the panel now cuts off just below the mouse panel. I spent about an hour trying to debug why the touchpad panel doesn't show up anymore, wondering why the GTK debugger shows it as visible, etc. until I accidentally realised I could scroll down and it's there. There are no scroll bars on the side to hint at that, touch I eventually noticed that the dotted line on the bottom is supposed to signal this. Either way, the cutoff is at the worst point (see attachment). If parts of the touchpad frame would be visible then the scrolling would be more obvious. The commit I marked above is part of a series, but just reverting the last one is enough to restore the old size (which obviously goes against this bug, but I'll just handwave around that :)
We already have other bugs to look into that.