GNOME Bugzilla – Bug 702408
Broken RTL handling for back button icon
Last modified: 2013-06-22 18:41:16 UTC
See patch. Pleas push the patch also to branch gnome-3-8.
Created attachment 246976 [details] [review] Set button arrow icons according to locale's text direction
Review of attachment 246976 [details] [review]: ::: src/baobab-window.vala @@ +186,3 @@ set_ui_state (home_page, false); + if(Gtk.Widget.get_default_direction() == Gtk.TextDirection.RTL) Here you should just use get_direction () == Gtk.TextDirection.RTL, right? And put a space before open parentheses, after "if" and after "get_default_direction" and all the others. @@ +187,3 @@ + if(Gtk.Widget.get_default_direction() == Gtk.TextDirection.RTL) + { In our coding style the open curly brace is on the same row of the "if" statement @@ +189,3 @@ + { + Gtk.Image back_button_image = back_button.get_child() as Gtk.Image; + back_button_image.set_from_icon_name("go-previous-rtl-symbolic", Gtk.IconSize.MENU); Use the property assignment instead of the setter, i.e. just do button_image.icon_name = "go-previous-rtl-symbolic". Since we cannot use the .UI file to automatically set the image here, I think it would be clearer to have if (get_direction () == Gtk.TextDirection.LTR) { image.icon_name = "go-previous-symbolic"; } else { image.icon_name = "go-previous-rtl-symbolic"; } and remove the icon_name property in the .UI Also, maybe move the code near the top of the constructor, like after setup_treview() or so.
I guess we also need to change the direction of the animations
Created attachment 246980 [details] [review] Set button arrow icons according to locale's text direction
Review of attachment 246980 [details] [review]: Better, now replace "Gtk.Image" with "var" in type definition and you're done! As for changing animation direction, I think here we need different patches for master and the gnome-3.8 branch. Yosef could you take care of this, too?
(In reply to comment #5) > As for changing animation direction, I think here we need different patches for > master and the gnome-3.8 branch. Yosef could you take care of this, too? Actually, it seems that master does already the right thing, so we only need a fix for gnome-3.8
Created attachment 247047 [details] [review] Set button arrow icons according to locale's text direction
Comment on attachment 247047 [details] [review] Set button arrow icons according to locale's text direction Attachment 247047 [details] pushed as eb58d8c - Set button arrow icons according to locale's text direction
Created attachment 247074 [details] [review] Part of previous commit forgotten