After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 702585 - Set buttons arrow icons according to locale's text direction
Set buttons arrow icons according to locale's text direction
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
3.9.x
Other Linux
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-06-18 17:48 UTC by Yosef Or Boczko
Modified: 2013-06-24 06:47 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Set buttons arrow icons according to locale's text direction (1.57 KB, patch)
2013-06-18 17:48 UTC, Yosef Or Boczko 		none Details | Review
Set button arrow icons according to locale's text direction (1.59 KB, patch)
2013-06-19 06:32 UTC, Yosef Or Boczko 		committed Details | Review

Description Yosef Or Boczko 2013-06-18 17:48:34 UTC 
Created attachment 247191 [details] [review]
Set buttons arrow icons according to locale's text direction

See patch.
Comment 1 Christian Persch 2013-06-18 22:12:15 UTC 
Review of attachment 247191 [details] [review]:

::: shell/ev-history-action-widget.c
@@ +212,3 @@
+        gboolean rtl;
+
+        rtl = (gtk_widget_get_default_direction () == GTK_TEXT_DIR_RTL);

Why not use gtk_widget_get_direction() on the actual widget?

@@ +224,3 @@
         switch (action_button) {
         case EV_HISTORY_ACTION_BUTTON_BACK:
+                icon_name = rtl ? "go-previous-rtl-symbolic" : "go-previous-symbolic";

What are those 'rtl' icon names? Why not simply use go-next-symbolic as the rtl variant for go-previous-symbolic and vv below?
Comment 2 Yosef Or Boczko 2013-06-19 06:32:37 UTC 
Created attachment 247229 [details] [review]
Set button arrow icons according to locale's text direction
Comment 3 Yosef Or Boczko 2013-06-19 06:34:47 UTC 
(In reply to comment #1)
> Review of attachment 247191 [details] [review]:
> 
> ::: shell/ev-history-action-widget.c
> @@ +212,3 @@
> +        gboolean rtl;
> +
> +        rtl = (gtk_widget_get_default_direction () == GTK_TEXT_DIR_RTL);
> 
> Why not use gtk_widget_get_direction() on the actual widget?
> 
> @@ +224,3 @@
>          switch (action_button) {
>          case EV_HISTORY_ACTION_BUTTON_BACK:
> +                icon_name = rtl ? "go-previous-rtl-symbolic" :
> "go-previous-symbolic";
> 
> What are those 'rtl' icon names? Why not simply use go-next-symbolic as the rtl
> variant for go-previous-symbolic and vv below?

Symbols 'rtl' meant exactly this bug.
Comment 4 Carlos Garcia Campos 2013-06-19 16:05:59 UTC 
Comment on attachment 247229 [details] [review]
Set button arrow icons according to locale's text direction

Pushed to git master, thanks!
Comment 5 Yosef Or Boczko 2013-06-22 18:11:35 UTC 
Pleas, push also to git gnome-3-8 branch.
Comment 6 Carlos Garcia Campos 2013-06-23 06:34:15 UTC 
I guess this is not considered a UI change, no? because UI is supposed to be frozen in stable branch.
Comment 7 Yosef Or Boczko 2013-06-23 06:56:09 UTC 
No, this is not considered a UI change.
Comment 8 Carlos Garcia Campos 2013-06-24 06:47:34 UTC 
Ok, pushed to gnome-3-8 branch too.