Merge lp:~haggai-eran/unity/rtl-menu-popup into lp:unity

Proposed by Haggai Eran on 2011-08-15
Status: Rejected
Rejected by: Stephen M. Webb on 2013-01-30
Proposed branch: lp:~haggai-eran/unity/rtl-menu-popup
Merge into: lp:unity
Diff against target: 92 lines (+44/-2) 3 files modified
To merge this branch: bzr merge lp:~haggai-eran/unity/rtl-menu-popup
Reviewer Review Type Date Requested Status
Alex Launi (community) Needs Fixing on 2012-03-14
Neil J. Patel 2011-08-15 Pending
Review via email: mp+71541@code.launchpad.net

Description of the Change

Hi,

These patches should make the menus in the unity menu bar and in the indicators open to the left when using a right-to-left locale. In a right-to-left setting, the menus should open that way so that they are aligned to the right with the label of the menu or the indicator icon.

The patches change the unity service to show menus to the left of the coordinates given when using an RTL locale. It also changes unity panel to send the coordinates of the bottom right corner of the indicator label, instead of the bottom left. A similar patch was proposed for unity-2d at lp:~haggai-eran/unity-2d/4.0-rtl.

The branch depends on some changes I've proposed to the nux library, at lp:~haggai-eran/nux/rtl.

To post a comment you must log in.
Alex Launi (alexlauni) wrote :

Please add tests that prove this behaves correctly.

review: Needs Fixing
Haggai Eran (haggai-eran) wrote :

Hi Alex,

Thanks for your comment. I'm wondering what tests would be suitable. Most of the changes in this branch are visual: opening the menu to the left, instead of to the right. One change that seems more suitable to automated testing is the keyboard navigation change, which makes the right key navigate to the previous menu, instead of to the next menu. I could add a test that checks this. Should that test be under tests/unit/TestPanelService.cpp?

Alex Launi (alexlauni) wrote :

Sounds right

Stephen M. Webb (bregma) wrote :

Although this functional change is desirable, the merge proposal has grown stale.

I'm rejecting it for now. Please re-propose if you are still actively proposing it.

Haggai Eran (haggai-eran) wrote :

That's fine. Without the nux RTL patches these patches won't work anyway.

I don't have much time these days to work on updating these patches. If I ever will I'll re-propose.

Unmerged revisions

1374. By Haggai Eran on 2011-08-13

Correct navigation in the menu when using keyboard on a right-to-left locale.

1373. By Haggai Eran on 2011-08-13

Open popup menus from the bottom-right point when using a right-to-left locale.

1372. By Haggai Eran on 2011-08-13

Align popup menus to the left when in right-to-left locale.

Preview Diff

1=== modified file 'plugins/unityshell/src/PanelIndicatorObjectEntryView.cpp'
2--- plugins/unityshell/src/PanelIndicatorObjectEntryView.cpp 2011-08-03 11:37:28 +0000
3+++ plugins/unityshell/src/PanelIndicatorObjectEntryView.cpp 2011-08-15 10:35:29 +0000
4@@ -97,7 +97,10 @@
5 (proxy_->image_visible() && proxy_->image_sensitive())) &&
6 nux::GetEventButton(button_flags) != 2)
7 {
8- proxy_->ShowMenu(GetAbsoluteX() + 1, //cairo translation
9+ int left = GetAbsoluteX() + 1; //cairo translation
10+ if (GetDirection() == nux::RightToLeft)
11+ left = left + GetGeometry().width;
12+ proxy_->ShowMenu(left,
13 GetAbsoluteY() + PANEL_HEIGHT,
14 time(NULL),
15 nux::GetEventButton(button_flags));
16@@ -140,7 +143,10 @@
17
18 void PanelIndicatorObjectEntryView::Activate()
19 {
20- proxy_->ShowMenu(GetAbsoluteGeometry().x + 1, //cairo translation FIXME: Make this into one function
21+ int left = GetAbsoluteX() + 1; //cairo translation FIXME: Make this into one function
22+ if (GetDirection() == nux::RightToLeft)
23+ left = left + GetGeometry().width;
24+ proxy_->ShowMenu(left,
25 GetAbsoluteGeometry().y + PANEL_HEIGHT,
26 time(NULL),
27 1);
28
29=== modified file 'plugins/unityshell/src/unityshell.cpp'
30--- plugins/unityshell/src/unityshell.cpp 2011-08-11 07:58:12 +0000
31+++ plugins/unityshell/src/unityshell.cpp 2011-08-15 10:35:29 +0000
32@@ -137,6 +137,8 @@
33 StartupNotifyService::Default()->SetSnDisplay(screen->snDisplay(), screen->screenNum());
34
35 nux::NuxInitialize(0);
36+ nux::SetDefaultDirection(gtk_widget_get_default_direction() == GTK_TEXT_DIR_RTL ?
37+ nux::RightToLeft : nux::LeftToRight);
38 wt = nux::CreateFromForeignWindow(cScreen->output(),
39 glXGetCurrentContext(),
40 &UnityScreen::initUnity,
41
42=== modified file 'services/panel-service.c'
43--- services/panel-service.c 2011-07-29 07:15:54 +0000
44+++ services/panel-service.c 2011-08-15 10:35:29 +0000
45@@ -784,6 +784,25 @@
46
47 *x = priv->last_x;
48 *y = priv->last_y;
49+
50+ if (gtk_widget_get_direction(GTK_WIDGET(menu)) == GTK_TEXT_DIR_RTL)
51+ {
52+ GtkAllocation size;
53+ GtkBorder padding;
54+
55+ gtk_widget_get_allocation(GTK_WIDGET(menu), &size);
56+
57+ GtkStyleContext *context;
58+ GtkStateFlags state;
59+
60+ context = gtk_widget_get_style_context (GTK_WIDGET(menu));
61+ state = gtk_widget_get_state_flags (GTK_WIDGET(menu));
62+
63+ gtk_style_context_get_padding (context, state, &padding);
64+
65+ *x += padding.left - size.width + 1;
66+ }
67+
68 *push = TRUE;
69 }
70
71@@ -1009,6 +1028,21 @@
72 || direction == GTK_MENU_DIR_PREV)
73 return;
74
75+ // Switch directions for RTL.
76+ if (gtk_widget_get_direction(GTK_WIDGET(menu)) == GTK_TEXT_DIR_RTL)
77+ {
78+ switch (direction)
79+ {
80+ case GTK_MENU_DIR_CHILD:
81+ direction = GTK_MENU_DIR_PARENT;
82+ break;
83+ case GTK_MENU_DIR_PARENT:
84+ direction = GTK_MENU_DIR_CHILD;
85+ break;
86+ default: ;
87+ }
88+ }
89+
90 /* We don't want to distrupt going into submenus */
91 if (direction == GTK_MENU_DIR_CHILD)
92 {