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 739836 - Improve sidebar and dialog ui
Improve sidebar and dialog ui
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
git master
Other Linux
: Normal enhancement
: ---
Assigned To: Evince Maintainers
Evince Maintainers
: 741819 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-11-09 06:59 UTC by Trinh Anh Ngoc
Modified: 2015-01-10 21:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (7.72 KB, patch)
2014-11-09 06:59 UTC, Trinh Anh Ngoc
none Details | Review
Screenshots (46.88 KB, image/png)
2014-11-09 07:00 UTC, Trinh Anh Ngoc
  Details
Screenshots (103.39 KB, image/png)
2014-11-09 07:01 UTC, Trinh Anh Ngoc
  Details
Screenshots (71.81 KB, image/png)
2014-11-09 07:02 UTC, Trinh Anh Ngoc
  Details
Screenshots (69.28 KB, image/png)
2014-11-09 07:09 UTC, Trinh Anh Ngoc
  Details
Screenshots (68.37 KB, image/png)
2014-11-09 07:10 UTC, Trinh Anh Ngoc
  Details
Patch (7.75 KB, patch)
2014-11-09 07:11 UTC, Trinh Anh Ngoc
none Details | Review
Patch (7.90 KB, patch)
2014-11-09 07:50 UTC, Trinh Anh Ngoc
reviewed Details | Review
Annotation list without top margin (51.76 KB, image/png)
2014-11-16 12:14 UTC, Trinh Anh Ngoc
  Details
Annotation list with top margin (51.81 KB, image/png)
2014-11-16 12:15 UTC, Trinh Anh Ngoc
  Details
Avoid borders collision in sidebar (809 bytes, patch)
2014-11-23 16:54 UTC, Trinh Anh Ngoc
committed Details | Review
Add margin to some areas in sidebar (3.33 KB, patch)
2014-11-23 16:55 UTC, Trinh Anh Ngoc
committed Details | Review
Set relief-none to annotation tool item (890 bytes, patch)
2014-11-23 16:56 UTC, Trinh Anh Ngoc
committed Details | Review
Use linked style for bookmark tool button group (1.96 KB, patch)
2014-11-23 16:57 UTC, Trinh Anh Ngoc
committed Details | Review
Use symbolic icon in sidebar (2.30 KB, patch)
2014-11-23 16:58 UTC, Trinh Anh Ngoc
committed Details | Review

Description Trinh Anh Ngoc 2014-11-09 06:59:26 UTC
Created attachment 290256 [details] [review]
Patch

Adjust margin, frame,... in sidebar.
Simplify properties dialog.
Comment 1 Trinh Anh Ngoc 2014-11-09 07:00:44 UTC
Created attachment 290257 [details]
Screenshots
Comment 2 Trinh Anh Ngoc 2014-11-09 07:01:35 UTC
Created attachment 290258 [details]
Screenshots
Comment 3 Trinh Anh Ngoc 2014-11-09 07:02:08 UTC
Created attachment 290259 [details]
Screenshots
Comment 4 Trinh Anh Ngoc 2014-11-09 07:09:38 UTC
Created attachment 290260 [details]
Screenshots
Comment 5 Trinh Anh Ngoc 2014-11-09 07:10:24 UTC
Created attachment 290261 [details]
Screenshots
Comment 6 Trinh Anh Ngoc 2014-11-09 07:11:22 UTC
Created attachment 290262 [details] [review]
Patch

Correct indent
Comment 7 Trinh Anh Ngoc 2014-11-09 07:50:30 UTC
Created attachment 290263 [details] [review]
Patch

Style FindSidebar
Comment 8 Carlos Garcia Campos 2014-11-16 09:36:13 UTC
Review of attachment 290263 [details] [review]:

Thanks for the patch. I think the patch could be split into several smaller patches, because it actually changes different things. I would also appreciate an explanation in the commit message so that the numbers don't look arbitrary.

::: shell/ev-properties-dialog.c
@@ +127,3 @@
+	
+	headerbar = gtk_header_bar_new ();
+	gtk_header_bar_set_title (GTK_HEADER_BAR (headerbar), "Properties");

Note that "Properties" is a translatable string, it should be marked for translation. But this is not actually needed, we can simply use "use-header-bar" property and the dialog will use a header and use the window title as headerbar title. I've just pushed these changes to git master using the use-header-bar property instead.

::: shell/ev-sidebar-annotations.c
@@ +131,3 @@
 	loading_model = ev_sidebar_annotations_create_simple_model (_("Loading…"));
 	ev_annots->priv->tree_view = gtk_tree_view_new_with_model (loading_model);
+	gtk_widget_set_margin_top (ev_annots->priv->tree_view, 2);

Why 2? and why adding the margin in general?

@@ +208,2 @@
 	item = gtk_toggle_tool_button_new ();
+	gtk_tool_button_set_icon_name (GTK_TOOL_BUTTON (item), "document-new-symbolic");

So there are three different improvements here, one is adding the margins, another is using relief-none for the tool palette item, and then using symbolic icons. I think we should also use a symbolic icon for the close button in the sidebar.

::: shell/evince.css
@@ +39,3 @@
     border-style: solid;
     border-width: 1px;
+    border-radius: 0;

This looks like an unrelated fix, I've just pushed it to git master too.

@@ +73,3 @@
+    border-width: 0;
+    border-top-width: 1px;
+}

Would it be possible to move here all other margin changes? why are some of the changes made in the CSS and some others in the code?
Comment 9 Trinh Anh Ngoc 2014-11-16 12:13:09 UTC
Thank you for the review. This is first time i make patch for GNOME.

1. Thank for commit headerbar support in dialog.
2. "2px" look nice for me. See screenshot for detail.
3. I tried to use margin in css but it doesn't work for me. I don't know why.
Therefore, i must do hard code :(
Comment 10 Trinh Anh Ngoc 2014-11-16 12:14:22 UTC
Created attachment 290788 [details]
Annotation list without top margin
Comment 11 Trinh Anh Ngoc 2014-11-16 12:15:00 UTC
Created attachment 290789 [details]
Annotation list with top margin
Comment 12 Lapo Calamandrei 2014-11-16 12:36:17 UTC
Carlos, some changes are in the code since we can't set margins in the css.
Comment 13 Trinh Anh Ngoc 2014-11-23 16:49:38 UTC
Hi. I made new patches for this.
Comment 14 Trinh Anh Ngoc 2014-11-23 16:54:48 UTC
Created attachment 291311 [details] [review]
Avoid borders collision in sidebar
Comment 15 Trinh Anh Ngoc 2014-11-23 16:55:56 UTC
Created attachment 291312 [details] [review]
Add margin to some areas in sidebar
Comment 16 Trinh Anh Ngoc 2014-11-23 16:56:59 UTC
Created attachment 291313 [details] [review]
Set relief-none to annotation tool item
Comment 17 Trinh Anh Ngoc 2014-11-23 16:57:52 UTC
Created attachment 291314 [details] [review]
Use linked style for bookmark tool button group
Comment 18 Trinh Anh Ngoc 2014-11-23 16:58:55 UTC
Created attachment 291315 [details] [review]
Use symbolic icon in sidebar
Comment 19 Carlos Garcia Campos 2014-11-29 11:44:37 UTC
Comment on attachment 291311 [details] [review]
Avoid borders collision in sidebar

Looks good. thanks!
Comment 20 Carlos Garcia Campos 2014-11-29 11:45:12 UTC
Comment on attachment 291312 [details] [review]
Add margin to some areas in sidebar

I don't see a big different, but looks good in any case.
Comment 21 Carlos Garcia Campos 2014-11-29 11:45:35 UTC
Comment on attachment 291313 [details] [review]
Set relief-none to annotation tool item

Ok.
Comment 22 Carlos Garcia Campos 2014-11-29 11:45:57 UTC
Comment on attachment 291314 [details] [review]
Use linked style for bookmark tool button group

Ok
Comment 23 Carlos Garcia Campos 2014-11-29 11:46:20 UTC
Comment on attachment 291315 [details] [review]
Use symbolic icon in sidebar

Looks much better, thanks!
Comment 24 Carlos Garcia Campos 2014-11-29 11:46:44 UTC
Pushed all the patches to git master, thank you!
Comment 25 Robert Roth 2015-01-10 21:44:28 UTC
*** Bug 741819 has been marked as a duplicate of this bug. ***