GNOME Bugzilla – Bug 704217
Make headerbar as titlebar and add close button
Last modified: 2013-08-18 20:25:08 UTC
I'm not sure about the design, so the patches may vary.
Created attachment 249148 [details] [review] shell: Added parameter a padding for cc_shell_embed_widget_in_header
Created attachment 249150 [details] [review] shell: Make headerbar as titlebar and add close button
Created attachment 249151 [details] [review] shell: move the search entry to left of the headerbar there isn't enough space between the close button to title.
Created attachment 249152 [details] Screenshot - Settings: overview
Created attachment 249153 [details] Screenshot - Settings: Region & language
(In reply to comment #1) > Created an attachment (id=249148) [details] [review] > shell: Added parameter a padding for cc_shell_embed_widget_in_header this for bug #703974#c6 and 703974#c9. "I wonder if we should compensate for that with a right margin" and "The right padding problem is also a blocker".
It is not intended for implementation.
Created attachment 249195 [details] [review] shell: Add class style "titlebar" to the headerbar for the csd Now you can try with GTK_CSD=1.
Created attachment 249196 [details] Screenshot - Settings: overview with CSD gnome-screenshot delete the background - it's probably a bug.
Created attachment 249197 [details] Screenshot - Settings: Region & language with CSD
Reopening. Allan Day said me that close button is for 3.10.
Created attachment 251413 [details] [review] shell: Make headerbar as titlebar and add close button
Created attachment 251414 [details] [review] shell: create search bar and move to him the search entry (I ask Allan Day on this)
Created attachment 251415 [details] Screenshot - Settings: overview (run with GTK_CSD=1)
Created attachment 251416 [details] Screenshot - Settings: search page
Created attachment 251417 [details] Screenshot - Settings: Region & language
Created attachment 251428 [details] [review] shell: Fix showing search bar in search from command line
Created attachment 251429 [details] [review] shell: Enable to hide the search bar with Escape
Created attachment 251430 [details] [review] shell: Focus search entry just if serach bar is showing
Created attachment 251431 [details] [review] shell: create search bar and move to him the search entry
Created attachment 251432 [details] [review] shell: create search bar and move to him the search entry
Review of attachment 251413 [details] [review]: Please change the commit title: "shell: Set the headerbar as our titlebar and add a close button" As pointed on IRC we don't want the subtitle and the main view should be labeled as "All Settings".
Review of attachment 251432 [details] [review]: Please change the commit title: "shell: Use a search bar for the search entry" ::: shell/cc-window.c @@ +237,3 @@ gtk_entry_set_text (GTK_ENTRY (priv->search_entry), ""); + if (gtk_search_bar_get_search_mode (GTK_SEARCH_BAR (self->priv->search_bar))) + gtk_widget_grab_focus (priv->search_entry); This makes us lose the "just start typing" search. We need some extra code in window_key_press_event to start the search mode in that case. @@ +1450,3 @@ "titlebar"); + /* priv button */ You mean "previous button", right? @@ +1473,3 @@ + gtk_widget_set_valign (button, GTK_ALIGN_CENTER); + gtk_style_context_add_class (gtk_widget_get_style_context (button), + "image-button"); No trailing whitespace, please
Created attachment 252144 [details] [review] shell: Set the headerbar as our titlebar and add a close button
Created attachment 252154 [details] [review] shell: Use a search bar for the search entry
Review of attachment 252144 [details] [review]: Almost fine. ::: shell/cc-window.c @@ +1465,3 @@ gtk_container_add (GTK_CONTAINER (priv->top_right_box), priv->lock_button); + + gtk_widget_show_all (priv->top_right_box); This works but looks weird. See below. @@ +1480,3 @@ create_header (self); + gtk_window_set_titlebar (GTK_WINDOW (self), priv->header); + gtk_header_bar_set_title (GTK_HEADER_BAR (priv->header), _(DEFAULT_WINDOW_TITLE)); This seems like the correct place to do the show_all() call on the headerbar just like below we do a show_all() on the box. @@ +1539,2 @@ "resizable", TRUE, "title", _(DEFAULT_WINDOW_TITLE), This string should remain "Settings" since that's the name in the .desktop file and thus what shows up in gnome-shell's app picker and dash but the window title is what shows up in the overview and in alt+tab and we don't want them to be different.
Created attachment 252156 [details] [review] shell: Set the headerbar as our titlebar and add a close button
Created attachment 252157 [details] [review] shell: Set the headerbar as our titlebar and add a close button
Created attachment 252158 [details] [review] shell: Use a search bar for the search entry
Review of attachment 252157 [details] [review]: Looks good now
Review of attachment 252158 [details] [review]: You can push after fixing that nit below. ::: shell/cc-window.c @@ +1466,3 @@ + gtk_widget_set_valign (priv->search_button, GTK_ALIGN_CENTER); + gtk_style_context_add_class (gtk_widget_get_style_context (priv->search_button), + "image-button"); trailing white space! yuck
Review of attachment 252157 [details] [review]: pushed as 9de9364ba5ab28381f62fc41468381baff7a733c - shell: Set the headerbar as our titlebar and add a close button
Review of attachment 252158 [details] [review]: pushed as ef995c072897d6a36fc2173b79cfa6347badc6d0 - shell: Use a search bar for the search entry