Merge lp:~tigrangab/slingshot/rtl_fix into lp:~elementary-pantheon/slingshot/trunk

[Tigran Gabrielyan]

Needs this granite branch to work correctly: https://code.launchpad.net/~tigrangab/granite/popover_fix/+merge/184027 There's a bug in arrow placement if it needs to be top right.

[Andrea Basso (voluntatefaber)]

I'm afraid that's really not enoguh, RTL goes way deeper than that, whereas your patch could solve the positioning problem, things like page-scrolling and keyboard shortcuts are still hardcoded as in a LTR prospective.

[Tigran Gabrielyan (tigrangab)]

Wasn't aware there were more issues. I'll install a RTL language to actually test it.

[Tigran Gabrielyan (tigrangab)]

Can you check it now and make sure I didn't miss anything and it works correctly?

[Andrea Basso (voluntatefaber)]

I'm currently not at home, so I unfortunately cannot test it.

Just looking at the code, though, are you sure keyboard shortcuts works fine, especially in turning pages?

[Tigran Gabrielyan (tigrangab)]

No :) I was doing that at 2AM, I'm working on it.

[Tigran Gabrielyan (tigrangab)]

Shouldn't the first page in RTL still be full?

https://launchpadlibrarian.net/147514797/applications.png That's the first page but only half are full. Is that intended or another RTL bug in slingshot?

[Andrea Basso (voluntatefaber)]

I'd say that's definitely a bug. And also Slingshot position, I guess you
appear in top-left corner...

[Tigran Gabrielyan (tigrangab)]

Please test this when you get a chance. If something doesn't work as expected, please let me know how it should work. Thanks.

[Andrea Basso (voluntatefaber)]

I'll do that as soon as I get back :)
Il giorno 06/set/2013 08:59, "Tigran Gabrielyan" <email address hidden> ha
scritto:

> Please test this when you get a chance. If something doesn't work as
> expected, please let me know how it should work. Thanks.
> --
> https://code.launchpad.net/~tigrangab/slingshot/rtl_fix/+merge/184029
> Your team elementary Pantheon team is requested to review the proposed
> merge of lp:~tigrangab/slingshot/rtl_fix into lp:slingshot.
>

[David Gomes (davidgomes)]

I need someone to test this or teach me how to. Should I change my language to Hebrew or something?

[Tigran Gabrielyan (tigrangab)]

I left a note on the PopOver merge request about it. Add setDefaultDirection(Gtk.TextDirection.RTL); in the constructor of SlingshotView.vala. You'll still have English but it will behave RTL-ly

[David Gomes (davidgomes)]

Remove diff line 236, 73. Add newline after diff line 49. I tested and it worked, good job.

review: Needs Fixing

[David Gomes (davidgomes)]

This is DEPENDENT on a Granite bug fix which HAS NOT been released on Luna yet. So we can only merge this to stable PPA when Granite gets a push to the stable PPA with that bug fix.

[Sergey "Shnatsel" Davidoff (shnatsel)]

Can we merge this now? At least as an Isis-only fix?

Unmerged revisions

381. By Tigran Gabrielyan on 2013-09-06

Fix RTL key navigation

380. By Tigran Gabrielyan on 2013-09-05

Fix keyboard and scroll navigation for RTL

379. By Tigran Gabrielyan on 2013-09-05

Show slingshot on right side of screen if language is RTL

=== modified file 'src/SlingshotView.vala'
--- src/SlingshotView.vala	2013-09-04 19:08:08 +0000
+++ src/SlingshotView.vala	2013-09-06 06:54:15 +0000
@@ -61,6 +61,9 @@
         private int search_view_position = 0;
         private Modality modality;
 
+        // Used for RTL support
+        private int direction = 1;
+
         // Sizes
         public int columns {
             get {
@@ -111,6 +114,10 @@
             setup_ui ();
             connect_signals ();
 
+            if (get_default_direction () == Gtk.TextDirection.RTL) {
+                direction = -1;
+            }
+
             debug ("Apps loaded");
 
         }
@@ -281,17 +288,19 @@
 
         }
 
-        private void reposition (bool show=true) {
-
+        private void reposition (bool show = true) {
             debug("Repositioning");
-            
+
+            var direction = get_default_direction ();
             Gdk.Rectangle monitor_dimensions, app_launcher_pos;
             screen.get_monitor_geometry (this.screen.get_primary_monitor(), out monitor_dimensions);
-            app_launcher_pos = Gdk.Rectangle () { x = monitor_dimensions.x,
-                                                  y = monitor_dimensions.y,
-                                                  width = 100,
-                                                  height = 30
-                                                 };
+
+            app_launcher_pos = Gdk.Rectangle () {
+                x = (direction == Gtk.TextDirection.RTL) ? monitor_dimensions.width : monitor_dimensions.x,
+                y = monitor_dimensions.y,
+                width = 100,
+                height = 30
+            };
             move_to_rect (app_launcher_pos, show);
         }
 
@@ -391,15 +400,17 @@
                 case "Left":
                     if (modality == Modality.NORMAL_VIEW) {
                         if (event.state == Gdk.ModifierType.SHIFT_MASK) // Shift + Left
-                            page_switcher.set_active (page_switcher.active - 1);
+                            page_switcher.set_active (page_switcher.active - direction);
                         else
-                            normal_move_focus (-1, 0);
+                            normal_move_focus (-direction, 0);
                     } else if (modality == Modality.CATEGORY_VIEW) {
                         if (event.state == Gdk.ModifierType.SHIFT_MASK) // Shift + Left
-                            category_view.switcher.set_active (category_view.switcher.active - 1);
-                        else if (!searchbar.has_focus) {//the user has already selected an AppEntry
-                            category_move_focus (-1, 0);
-                        }
+                            category_view.switcher.set_active (category_view.switcher.active - direction);
+                        else if (searchbar.has_focus && direction == -1) // there's no AppEntry selected, the user is switching category
+                            top_left_focus ();
+                        else
+                            category_move_focus (-direction, 0);
+
                     } else
                         return base.key_press_event (event);
                     break;
@@ -407,16 +418,16 @@
                 case "Right":
                     if (modality == Modality.NORMAL_VIEW) {
                         if (event.state == Gdk.ModifierType.SHIFT_MASK) // Shift + Right
-                            page_switcher.set_active (page_switcher.active + 1);
+                            page_switcher.set_active (page_switcher.active + direction);
                         else
-                            normal_move_focus (+1, 0);
+                            normal_move_focus (direction, 0);
                     } else if (modality == Modality.CATEGORY_VIEW) {
                         if (event.state == Gdk.ModifierType.SHIFT_MASK) // Shift + Right
-                            category_view.switcher.set_active (category_view.switcher.active + 1);
-                        else if (searchbar.has_focus) // there's no AppEntry selected, the user is switching category
+                            category_view.switcher.set_active (category_view.switcher.active + direction);
+                        else if (searchbar.has_focus && direction == 1) // there's no AppEntry selected, the user is switching category
                             top_left_focus ();
                         else //the user has already selected an AppEntry
-                            category_move_focus (+1, 0);
+                            category_move_focus (direction, 0);
                     } else {
                         return base.key_press_event (event);
                     }
@@ -463,7 +474,7 @@
 
                 case "Page_Up":
                     if (modality == Modality.NORMAL_VIEW) {
-                        page_switcher.set_active (page_switcher.active - 1);
+                        page_switcher.set_active (page_switcher.active - direction);
                         if (page_switcher.active != 0) // we don't wanna lose focus if we don't actually change page
                             searchbar.grab_focus (); // this is because otherwise focus isn't the current page
                     } else if (modality == Modality.CATEGORY_VIEW) {
@@ -474,7 +485,7 @@
 
                 case "Page_Down":
                     if (modality == Modality.NORMAL_VIEW) {
-                        page_switcher.set_active (page_switcher.active + 1);
+                        page_switcher.set_active (page_switcher.active + direction);
                         if (page_switcher.active != grid_view.get_n_pages () - 1) // we don't wanna lose focus if we don't actually change page
                             searchbar.grab_focus (); //this is because otherwise focus isn't the current page
                     } else if (modality == Modality.CATEGORY_VIEW) {
@@ -542,32 +553,33 @@
 
         }
 
+        private void scroll_page (int direction) {
+            if (modality == Modality.NORMAL_VIEW) {
+                page_switcher.set_active (page_switcher.active + direction);
+            } else if (modality == Modality.SEARCH_VIEW) {
+                if (direction == this.direction) {
+                    search_view_up ();
+                } else {
+                    search_view_down ();
+                }
+            } else {
+                category_view.switcher.set_active (category_view.switcher.active + direction);
+            }
+        }
+
         public override bool scroll_event (EventScroll event) {
-
             switch (event.direction.to_string ()) {
                 case "GDK_SCROLL_UP":
                 case "GDK_SCROLL_LEFT":
-                    if (modality == Modality.NORMAL_VIEW)
-                        page_switcher.set_active (page_switcher.active - 1);
-                    else if (modality == Modality.SEARCH_VIEW)
-                        search_view_up ();
-                    else
-                        category_view.switcher.set_active (category_view.switcher.active - 1);
+                    scroll_page (-direction);
                     break;
                 case "GDK_SCROLL_DOWN":
                 case "GDK_SCROLL_RIGHT":
-                    if (modality == Modality.NORMAL_VIEW)
-                        page_switcher.set_active (page_switcher.active + 1);
-                    else if (modality == Modality.SEARCH_VIEW)
-                        search_view_down ();
-                    else
-                        category_view.switcher.set_active (category_view.switcher.active + 1);
+                    scroll_page (direction);
                     break;
-
             }
 
             return false;
-
         }
 
         public void show_slingshot () {
@@ -590,34 +602,40 @@
             Wnck.Screen.get_default ().force_update ();
             if (w != null)
                 w.activate (Gdk.x11_get_server_time (this.get_window ()));
-		}
+        }
 
         private void move_page (int step) {
-        
             debug ("Moving: step = " + step.to_string ());
-        
+
             if (step == 0)
                 return;
-            if (step < 0 && current_position >= 0) //Left border
-                return;
-            if (step > 0 && (-current_position) >= ((grid_view.get_n_pages () - 1) * grid_view.get_page_columns () * 130)) //Right border
-                return;
-            
+
+            int max_position = (grid_view.get_n_pages () - 1) * grid_view.get_page_columns () * 130;
+
+            if (step < 0) {
+                if ((direction == 1 && current_position >= 0) || (direction == -1 && current_position <= -max_position)) {
+                    return;
+                }
+            } else {
+                if ((direction == 1 && current_position <= -max_position) || (direction == -1 && current_position >= 0)) {
+                    return;
+                }
+            }
+
             int count = 0;
-            int increment = -step*130*columns/10;
-            Timeout.add (30/columns, () => {
+            int increment = -step * 130 * columns / 10 * direction;
 
+            Timeout.add (30 / columns, () => {
                 if (count >= 10) {
-                    current_position += -step*130*columns - 10*increment; //We adjust to end of the page
+                    current_position += -step * 130 * columns - 10 * increment * direction; //We adjust to end of the page
                     view_manager.move (grid_view, current_position, 0);
                     return false;
                 }
-                    
+
                 current_position += increment;
                 view_manager.move (grid_view, current_position, 0);
                 count++;
                 return true;
-                
             }, Priority.DEFAULT_IDLE);
         }
 
@@ -744,10 +762,17 @@
                 grid_view.append (app_entry);
                 app_entry.show_all ();
             }
-                            
-            view_manager.move (grid_view, 0, 0);
+
             current_position = 0;
 
+            if (get_default_direction () == Gtk.TextDirection.RTL) {
+                int width;
+                view_manager.get_size_request (out width, null);
+                current_position = (grid_view.get_n_pages () - 1) * width * -1;
+            }
+
+            view_manager.move (grid_view, current_position, 0);
+
         }
 
         private void read_settings (bool first_start = false, bool check_columns = true, bool check_rows = true) {