GNOME Bugzilla – Bug 593650
option for sidebar on right side of window
Last modified: 2018-05-22 13:37:33 UTC
Presently the sidebar can only be viewed on the left side of the evince window. Since it is used for navigation through a document, it is more ergonomic to have it on the right side of the window, right next to the scroll-bar, where the mouse may often be. So, it would be nice if there were a configuration option to have the sidebar / side-pane open up on the right side of the window.
Created attachment 271813 [details] [review] Patch to Evince sidebar position control *Please use --ignore-whitespace when applying the patch *Added menu items, accelerators to Set Sidebar position *Source can be found here - https://github.com/m4heshd/evince *First version - https://github.com/m4heshd/evince_org
(In reply to comment #1) Thank you for the patch, Mahesh! > *Please use --ignore-whitespace when applying the patch Why? I think it's better to removed whitespaces before submitting a patch. > *First version - https://github.com/m4heshd/evince_org What does the this repository contain? Why have you linked it here? If intend to continue working on Evince, then... 1. Contact gpoo [1] or Aakash Goenka [2] who was last summer's intern for Evince coding the Bookshelf View. 2. Fork evince from github.com/gnome/evince and then branch out to work on specific feature. Submit a link to this branch so you can receive continuous evaluation on work committed in this branch. [1] https://people.gnome.org/~gpoo/ [2] https://wiki.gnome.org/Outreach/SummerOfCode/2013/Projects/AakashGoenka_BookshelfTilingEvince)
(In reply to comment #2) > (In reply to comment #1) > > Thank you for the patch, Mahesh! My pleasure :) > > *Please use --ignore-whitespace when applying the patch > Why? I think it's better to removed whitespaces before submitting a patch. i don't know what's the matter. I removed all whitespaces when creating the patch. Somehow the "git apply" detects some few more. so --ignore-whitespace clears that for me. > > *First version - https://github.com/m4heshd/evince_org > > What does the this repository contain? Why have you linked it here? It's just the version when i did the cloning. I thought it'll be helpful.
(In reply to comment #3) > i don't know what's the matter. I removed all whitespaces when creating the > patch. Somehow the "git apply" detects some few more. so --ignore-whitespace > clears that for me. Try with "Remove whitespaces" plugin in Gedit.
(In reply to comment #4) > Try with "Remove whitespaces" plugin in Gedit. I'll definitely try that. Thank you for the advice.
Comment on attachment 271813 [details] [review] Patch to Evince sidebar position control From 4570959d0fc6788080398f1517fa6ac4a26e6672 Mon Sep 17 00:00:00 2001 From: Mahesh Bandara Wijerathna <m4heshd@gmail.com> Date: Mon, 17 Mar 2014 22:31:53 +0530 Subject: [PATCH] Apply the patch --- shell/ev-window.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++ shell/evince-ui.xml | 4 +++ 2 files changed, 86 insertions(+) diff --git a/shell/ev-window.c b/shell/ev-window.c index 978c55f..f45b933 100644 --- a/shell/ev-window.c +++ b/shell/ev-window.c @@ -387,6 +387,15 @@ static gchar *nautilus_sendto = NULL; G_DEFINE_TYPE (EvWindow, ev_window, GTK_TYPE_APPLICATION_WINDOW) +int w_w,w_w2; +void get_widget_size(GtkWidget *widget, GtkAllocation *allocation, void *data) { + w_w = allocation->width; +} + +void get_widget_size2(GtkWidget *widget, GtkAllocation *allocation, void *data) { + w_w2 = allocation->width; +} + static gdouble get_screen_dpi (EvWindow *window) { @@ -5135,6 +5144,64 @@ ev_window_view_sidebar_cb (GtkAction *action, EvWindow *ev_window) } static void +ev_window_view_sidebar_left (GtkAction *action, EvWindow *ev_window) +{ + int sb_w=w_w; + + g_object_ref (ev_window->priv->sidebar); + g_object_ref (ev_window->priv->view_box); + + + gtk_container_remove (GTK_PANED (ev_window->priv->hpaned),ev_window->priv->sidebar); + gtk_container_remove (GTK_PANED (ev_window->priv->hpaned),ev_window->priv->view_box); + + gtk_paned_pack1 (GTK_PANED (ev_window->priv->hpaned), + ev_window->priv->sidebar, FALSE, FALSE); + + gtk_widget_show (ev_window->priv->sidebar); + + gtk_paned_add2 (GTK_PANED (ev_window->priv->hpaned), + ev_window->priv->view_box); + + gtk_widget_show (ev_window->priv->view_box); + + gtk_paned_set_position (GTK_PANED (ev_window->priv->hpaned), sb_w); + g_object_unref (ev_window->priv->sidebar); + g_object_unref (ev_window->priv->view_box); + + +} + +static void +ev_window_view_sidebar_right (GtkAction *action, EvWindow *ev_window) +{ + int vb_w=w_w2; + + g_object_ref (ev_window->priv->sidebar); + g_object_ref (ev_window->priv->view_box); + + + gtk_container_remove (GTK_PANED (ev_window->priv->hpaned),ev_window->priv->sidebar); + gtk_container_remove (GTK_PANED (ev_window->priv->hpaned),ev_window->priv->view_box); + + gtk_paned_pack2 (GTK_PANED (ev_window->priv->hpaned), + ev_window->priv->sidebar, FALSE, FALSE); + + gtk_widget_show (ev_window->priv->sidebar); + + gtk_paned_add1 (GTK_PANED (ev_window->priv->hpaned), + ev_window->priv->view_box); + + gtk_widget_show (ev_window->priv->view_box); + + gtk_paned_set_position (GTK_PANED (ev_window->priv->hpaned), vb_w); + g_object_unref (ev_window->priv->sidebar); + g_object_unref (ev_window->priv->view_box); + + +} + +static void ev_window_sidebar_current_page_changed_cb (EvSidebar *ev_sidebar, GParamSpec *pspec, EvWindow *ev_window) @@ -6087,6 +6154,15 @@ static const GtkActionEntry entries[] = { N_("Reload the document"), G_CALLBACK (ev_window_cmd_view_reload) }, + /* Side Pane Controls*/ + { "PanePosition", NULL, N_("Se_t Side Pane Position") }, + { "PaneLeft", NULL, N_("_Left"), "<shift><control>L", + N_("Set Side Pane position to the Left"), + G_CALLBACK (ev_window_view_sidebar_left) }, + { "PaneRight", NULL, N_("_Right"), "<shift><control>R", + N_("Set Side Pane position to the Right"), + G_CALLBACK (ev_window_view_sidebar_right) }, + { "ViewAutoscroll", GTK_STOCK_MEDIA_PLAY, N_("Auto_scroll"), NULL, NULL, G_CALLBACK (ev_window_cmd_view_autoscroll) }, @@ -6176,6 +6252,7 @@ static const GtkActionEntry entries[] = { G_CALLBACK (ev_window_cmd_action_menu) }, { "F7", NULL, "", "F7", NULL, G_CALLBACK (ev_window_cmd_view_toggle_caret_navigation) }, + }; /* Toggle items */ @@ -7644,6 +7721,11 @@ ev_window_init (EvWindow *ev_window) ev_window->priv->view); /* Connect to model signals */ + + /* Get sizes of the widgets */ + g_signal_connect(ev_window->priv->sidebar, "size-allocate", G_CALLBACK(get_widget_size), NULL); + g_signal_connect(ev_window->priv->view_box, "size-allocate", G_CALLBACK(get_widget_size2), NULL); + g_signal_connect_swapped (ev_window->priv->model, "page-changed", G_CALLBACK (ev_window_page_changed_cb), diff --git a/shell/evince-ui.xml b/shell/evince-ui.xml index df9c9f9..e3cdd25 100644 --- a/shell/evince-ui.xml +++ b/shell/evince-ui.xml @@ -35,6 +35,10 @@ <menuitem name="ViewDualMenu" action="ViewDual"/> <separator/> <menuitem name="ViewSidebarMenu" action="ViewSidebar"/> + <menu name="SetPanePosition" action="PanePosition"> + <menuitem name="SetPaneLeft" action="PaneLeft"/> + <menuitem name="SetPaneRight" action="PaneRight"/> + </menu> <separator/> <menuitem name="ViewFullscreenMenu" action="ViewFullscreen"/> <menuitem name="ViewPresentationMenu" action="ViewPresentation"/> -- 1.7.9.5
Created attachment 272184 [details] [review] Sidebar Patch version 2 Re created the patch From a single commit because of the uncomfortability of reviewing the old patch.
Review of attachment 272184 [details] [review]: Remove the UI part. The location of the sidebar makes sense for different languages (Left-To-Right/Right-To-Left). But that should be detected on runtime. ::: shell/ev-window.c @@ +388,3 @@ G_DEFINE_TYPE (EvWindow, ev_window, GTK_TYPE_APPLICATION_WINDOW) +int w_w,w_w2; Don't use global variables @@ +5201,3 @@ + +} + This function are almost identical. Use a helper function that receives the orientation as an argument. @@ +6256,1 @@ }; Don't add this extra empty line @@ +7726,3 @@ + g_signal_connect(ev_window->priv->sidebar, "size-allocate", G_CALLBACK(get_widget_size), NULL); + g_signal_connect(ev_window->priv->view_box, "size-allocate", G_CALLBACK(get_widget_size2), NULL); + Instead, you can call the function when the sidebar is created.
The location of sidebar makes sense for switching between LTR and RTL. However, that could be detected automatically during runtime.
I agree that it is good to detect the *default* location at runtime, but to prevent the user from being able to choose which side s/he prefers simply because you prefer the left or right side would be unfortunate. Does having this feature be something which the user can choose (e.g. in some settings dialog) detract from evince somehow? The overall original idea of this bug-report was not to automatically accommodate users with LTR/RTL languages, but to allow each individual user to decide for him/herself what is desired. Another motivation was so that a choice could be made to locate the sidebar in the same place that the scroll-bar is, which can reduce the amount of mouse motion required when using evince.
(In reply to comment #10) > I agree that it is good to detect the *default* location at runtime, but to > prevent the user from being able to choose which side s/he prefers simply > because you prefer the left or right side would be unfortunate. Does having > this feature be something which the user can choose (e.g. in some settings > dialog) detract from evince somehow? The overall original idea of this > bug-report was not to automatically accommodate users with LTR/RTL languages, > but to allow each individual user to decide for him/herself what is desired. > Another motivation was so that a choice could be made to locate the sidebar in > the same place that the scroll-bar is, which can reduce the amount of mouse > motion required when using evince. That's exactly what i was gonna say. I'm so confused with gpoo's review. This patch has nothing to do about languages.
(In reply to comment #10) > I agree that it is good to detect the *default* location at runtime, but to > prevent the user from being able to choose which side s/he prefers simply > because you prefer the left or right side would be unfortunate. Does having > this feature be something which the user can choose (e.g. in some settings > dialog) detract from evince somehow? The overall original idea of this > bug-report was not to automatically accommodate users with LTR/RTL languages, > but to allow each individual user to decide for him/herself what is desired. > Another motivation was so that a choice could be made to locate the sidebar in > the same place that the scroll-bar is, which can reduce the amount of mouse > motion required when using evince. Historically Evince tries to get good defaults. If there is a good use case for something different, then it could be better to switch to something different. But you have to make a good use case. Making the option would make Evince inconsistent with other applications that use a sidebar, like Gedit, Nautilus, Anjuta, etc. So far, this bug only makes sense for RTL/LTR.
(In reply to comment #11) > (In reply to comment #10) > > I agree that it is good to detect the *default* location at runtime, but to > > prevent the user from being able to choose which side s/he prefers simply > > because you prefer the left or right side would be unfortunate. Does having > > this feature be something which the user can choose (e.g. in some settings > > dialog) detract from evince somehow? The overall original idea of this > > bug-report was not to automatically accommodate users with LTR/RTL languages, > > but to allow each individual user to decide for him/herself what is desired. > > Another motivation was so that a choice could be made to locate the sidebar in > > the same place that the scroll-bar is, which can reduce the amount of mouse > > motion required when using evince. > > That's exactly what i was gonna say. I'm so confused with gpoo's review. This > patch has nothing to do about languages. Then split the patch.
(In reply to comment #13) Hello gpoo. First thank you so much for the quick review. > Then split the patch. Please guide me through what to split and what are the wrongs in the patch. I haven't done anything to override the Evince UI defaults. Side pane was on left by default and it still is. I did add few menu items though.
(In reply to comment #14) > (In reply to comment #13) > [...] > > Then split the patch. > > Please guide me through what to split and what are the wrongs in the patch. I > haven't done anything to override the Evince UI defaults. Side pane was on left > by default and it still is. I did add few menu items though. Split the part related with UI from the actual location of the sidebar. That is, 2 patches. To be honest, the one of the UI is unlikely to land. But the other part is necessary for correct support of RTL languages.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/evince/issues/101.