GNOME Bugzilla – Bug 653819
The search bar doesn't remember its size
Last modified: 2013-05-22 22:25:42 UTC
This report was originally filled at https://bugs.launchpad.net/ubuntu/+source/banshee/+bug/803928 1Search_box_normal.png is the normal size of the search bar. If you resize it (2Search_box_longer_before_restart.png) and then restart banshee, you will notice that the search bar returned to its normal size (3Search_box_after_restart.png). Attached screenshots: [1]https://bugs.launchpad.net/ubuntu/+source/banshee/+bug/803928/+attachment/2185431/+files/1Search_box_normal.png [2]https://bugs.launchpad.net/ubuntu/+source/banshee/+bug/803928/+attachment/2185432/+files/2Search_box_longer_before_restart.png [3]https://bugs.launchpad.net/ubuntu/+source/banshee/+bug/803928/+attachment/2185433/+files/3Search_box_after_restart.png
Created attachment 243090 [details] [review] Patch - search bar remembers its size I'm attaching a patch that solves this problem. I tested it a bit.
Hey Tomasz, thanks for the patch! A question about it: > @@ -170,7 +170,15 @@ namespace Nereid > search_entry_align = new Alignment (1.0f, 0.5f, 0f, 0f); > var box = new HBox () { Spacing = 2 }; > var grabber = new GrabHandle (); > - grabber.ControlWidthOf (view_container.SearchEntry, 150, 350, false); > + SearchEntry search_entry = view_container.SearchEntry; > + grabber.ControlWidthOf (search_entry, 150, 350, false); > + > + int search_entry_width = SearchEntryWidth.Get (); > + search_entry.SetSizeRequest(search_entry_width, -1); // -1 indicates no resizing If there's no resizing, do we need the call at all? Maybe it still works without this. > + search_entry.SizeAllocated+= (o, a) => { > + SearchEntryWidth.Set (search_entry.Allocation.Width); > + }; > + Why do we need a new SearchEntry instance? Wouldn't the patch work just by having the new delegate subscribed to SizeAllocated event, but reusing the view_container.SearchEntry object?
Actually, those were 2 questions :)
Thank you for feedback, Andrés. I'll fix that at the weekend.
> > @@ -170,7 +170,15 @@ namespace Nereid > > search_entry_align = new Alignment (1.0f, 0.5f, 0f, 0f); > > var box = new HBox () { Spacing = 2 }; > > var grabber = new GrabHandle (); > > - grabber.ControlWidthOf (view_container.SearchEntry, 150, 350, false); > > + SearchEntry search_entry = view_container.SearchEntry; > > + grabber.ControlWidthOf (search_entry, 150, 350, false); > > + > > + int search_entry_width = SearchEntryWidth.Get (); > > + search_entry.SetSizeRequest(search_entry_width, -1); // -1 indicates no resizing > > If there's no resizing, do we need the call at all? Maybe it still works > without this. I think that no call can be removed - it is the only line in the code which sets width that is stored in settings. Maybe my comment is misleading and should be improved (e.g "// height should be preserved, so second parameter is -1") > > + search_entry.SizeAllocated+= (o, a) => { > > + SearchEntryWidth.Set (search_entry.Allocation.Width); > > + }; > > + > > Why do we need a new SearchEntry instance? Wouldn't the patch work just by > having the new delegate subscribed to SizeAllocated event, but reusing the > view_container.SearchEntry object? Could you explain what you mean? search_entry variable is set to view_container.SearchEntry (see: line 173).
Comment on attachment 243090 [details] [review] Patch - search bar remembers its size Hey Tomasz, sorry for the long delay to reply! See inline: (In reply to comment #5) > > > @@ -170,7 +170,15 @@ namespace Nereid > > > search_entry_align = new Alignment (1.0f, 0.5f, 0f, 0f); > > > var box = new HBox () { Spacing = 2 }; > > > var grabber = new GrabHandle (); > > > - grabber.ControlWidthOf (view_container.SearchEntry, 150, 350, false); > > > + SearchEntry search_entry = view_container.SearchEntry; > > > + grabber.ControlWidthOf (search_entry, 150, 350, false); > > > + > > > + int search_entry_width = SearchEntryWidth.Get (); > > > + search_entry.SetSizeRequest(search_entry_width, -1); // -1 indicates no resizing > > > > If there's no resizing, do we need the call at all? Maybe it still works > > without this. > > I think that no call can be removed - it is the only line in the code which > sets width that is stored in settings. Maybe my comment is misleading and > should be improved (e.g "// height should be preserved, so second parameter is > -1") Ok, I've changed the comment to better reflect this. > > > > + search_entry.SizeAllocated+= (o, a) => { > > > + SearchEntryWidth.Set (search_entry.Allocation.Width); > > > + }; > > > + > > > > Why do we need a new SearchEntry instance? Wouldn't the patch work just by > > having the new delegate subscribed to SizeAllocated event, but reusing the > > view_container.SearchEntry object? > > Could you explain what you mean? > search_entry variable is set to view_container.SearchEntry (see: line 173). Of course, you're right! An oversight on my part sorry. So, I've committed the patch (with your name) after I changed some things very slightly: - Modified comment as you adviced above. - Used var instead of SearchEntry, to have a bit less verbose code. - Added a space between SizeAllocated and += operator, and other space after SetSizeRequest and before the parenthesis (slight violations of the coding guidelines :) ). - Improved the commit message (look at it so you start seeing the patterns we use): https://git.gnome.org/browse/banshee/commit/?id=4931c7fc1e6ecf242c7d0de661b226efcc37be46 So, thanks so much for your contribution!
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.