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 701966 - Implementation the new 3.10 design
Implementation the new 3.10 design
Status: RESOLVED DUPLICATE of bug 704752
Product: devhelp
Classification: Applications
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: devhelp-maint
devhelp-maint
: 700004 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-06-10 22:57 UTC by Yosef Or Boczko
Modified: 2013-07-24 10:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Port to GtkHeaderBar (2.19 KB, patch)
2013-06-10 22:57 UTC, Yosef Or Boczko
needs-work Details | Review
Make headerbar a titlebar (1.31 KB, patch)
2013-06-10 22:58 UTC, Yosef Or Boczko
needs-work Details | Review
Use GtkButton subclasses instead of GdHeaderButton ones (5.12 KB, patch)
2013-06-10 22:58 UTC, Yosef Or Boczko
needs-work Details | Review
Remove libgd submodule (2.36 KB, patch)
2013-06-10 22:58 UTC, Yosef Or Boczko
needs-work Details | Review
Bump required GTK+ (552 bytes, patch)
2013-06-10 22:59 UTC, Yosef Or Boczko
none Details | Review
DevHelp new 3.10 design (screenshot) (214.35 KB, image/png)
2013-06-11 10:53 UTC, Yosef Or Boczko
  Details
DevHelp new 3.10 design (Screenshot, Maximize) (142.81 KB, image/png)
2013-06-11 10:58 UTC, Yosef Or Boczko
  Details
Use GtkButton subclasses instead of GdHeaderButton ones (no empty lines only with whitespaces are added) (5.15 KB, patch)
2013-06-11 12:11 UTC, Yosef Or Boczko
none Details | Review
Make headerbar a titlebar (whitespace needed before the open parenthesis) (1.31 KB, patch)
2013-06-11 12:15 UTC, Yosef Or Boczko
needs-work Details | Review
Port to GtkHeaderBar and Bump required GTK+ (2.52 KB, patch)
2013-06-11 14:27 UTC, Yosef Or Boczko
none Details | Review
Port to GtkHeaderBar and bump required GTK+ (2.65 KB, patch)
2013-06-11 15:12 UTC, Yosef Or Boczko
none Details | Review
Make headerbar a titlebar (1.31 KB, patch)
2013-06-11 15:14 UTC, Yosef Or Boczko
none Details | Review
Use GtkButton subclasses instead of GdHeaderButton ones (5.32 KB, patch)
2013-06-11 15:17 UTC, Yosef Or Boczko
needs-work Details | Review
Remove libgd submodule (2.10 KB, patch)
2013-06-11 15:20 UTC, Yosef Or Boczko
needs-work Details | Review
Use GtkButton subclasses instead of GdHeaderButton ones (whitespace needed before the open parenthesis) (5.34 KB, patch)
2013-06-11 15:50 UTC, Yosef Or Boczko
none Details | Review
Remove libgd submodule (2.29 KB, patch)
2013-06-11 15:57 UTC, Yosef Or Boczko
none Details | Review
Port to GtkHeaderBar and bump required GTK+ (2.65 KB, patch)
2013-06-30 16:06 UTC, Yosef Or Boczko
committed Details | Review
Use GtkButton subclasses instead of GdHeaderButton ones and fix arrow icons in RTL locales (4.78 KB, patch)
2013-06-30 16:07 UTC, Yosef Or Boczko
committed Details | Review
Remove libgd submodule (2.29 KB, patch)
2013-06-30 16:07 UTC, Yosef Or Boczko
committed Details | Review
Make headerbar a title and add close button (2.56 KB, patch)
2013-06-30 16:24 UTC, Yosef Or Boczko
none Details | Review
Before the changes (102.17 KB, image/png)
2013-06-30 16:27 UTC, Yosef Or Boczko
  Details
After the changes (without close button) (102.10 KB, image/png)
2013-06-30 16:28 UTC, Yosef Or Boczko
  Details
After the changes (with close button) (102.80 KB, image/png)
2013-06-30 16:29 UTC, Yosef Or Boczko
  Details
Fix a tooltip of forward_button (1.02 KB, patch)
2013-06-30 19:23 UTC, Yosef Or Boczko
committed Details | Review

Description Yosef Or Boczko 2013-06-10 22:57:02 UTC
See https://live.gnome.org/Design/Whiteboards/CloseButton.

Another moment I will add patches.
Comment 1 Yosef Or Boczko 2013-06-10 22:57:47 UTC
Created attachment 246455 [details] [review]
Port to GtkHeaderBar
Comment 2 Yosef Or Boczko 2013-06-10 22:58:08 UTC
Created attachment 246456 [details] [review]
Make headerbar a titlebar
Comment 3 Yosef Or Boczko 2013-06-10 22:58:27 UTC
Created attachment 246457 [details] [review]
Use GtkButton subclasses instead of GdHeaderButton ones
Comment 4 Yosef Or Boczko 2013-06-10 22:58:43 UTC
Created attachment 246458 [details] [review]
Remove libgd submodule
Comment 5 Yosef Or Boczko 2013-06-10 22:59:16 UTC
Created attachment 246459 [details] [review]
Bump required GTK+
Comment 6 Aleksander Morgado 2013-06-11 07:38:45 UTC
Screenshot maybe?
Comment 7 Yosef Or Boczko 2013-06-11 10:53:03 UTC
Created attachment 246493 [details]
DevHelp new 3.10 design (screenshot)
Comment 8 Yosef Or Boczko 2013-06-11 10:58:53 UTC
Created attachment 246494 [details]
DevHelp new 3.10 design (Screenshot, Maximize)
Comment 9 Aleksander Morgado 2013-06-11 11:16:06 UTC
Review of attachment 246458 [details] [review]:

Would be good to say in the commit log why it is being removed.

Also, autogen.sh complaints about the missing libgd submodule:
  [aleksander@ares devhelp]$ ./autogen.sh 
  /usr/bin/gnome-autogen.sh
  No submodule mapping found in .gitmodules for path 'libgd'

Probably needs to be removed from the git config as well.
Comment 10 Aleksander Morgado 2013-06-11 11:17:01 UTC
Review of attachment 246457 [details] [review]:

Please make sure that no empty lines only with whitespaces are added; got 4 reported when applying this patch.
Comment 11 Aleksander Morgado 2013-06-11 11:17:42 UTC
Review of attachment 246456 [details] [review]:

::: src/dh-window.c
@@ +693,3 @@
 
+        /* Make headerbar a titlebar */
+        gtk_window_set_titlebar(GTK_WINDOW(window), priv->header_bar);

Whitespace needed before the open parenthesis
Comment 12 Aleksander Morgado 2013-06-11 11:19:40 UTC
Comment on attachment 246455 [details] [review]
Port to GtkHeaderBar

I'd merge the GTK+ bump commit into this one, as GtkHeaderBar already needs the new GTK+ version. Also, explicitly say in the commit message that GTK+ is being bumped and to which version.
Comment 13 Aleksander Morgado 2013-06-11 11:26:58 UTC
*** Bug 700004 has been marked as a duplicate of this bug. ***
Comment 14 Yosef Or Boczko 2013-06-11 12:11:06 UTC
Created attachment 246504 [details] [review]
Use GtkButton subclasses instead of GdHeaderButton ones (no empty lines only with whitespaces are added)
Comment 15 Yosef Or Boczko 2013-06-11 12:15:12 UTC
Created attachment 246505 [details] [review]
Make headerbar a titlebar (whitespace needed before the open parenthesis)
Comment 16 Yosef Or Boczko 2013-06-11 12:18:36 UTC
That's good enough?
Comment 17 Aleksander Morgado 2013-06-11 14:17:44 UTC
(In reply to comment #16)
> That's good enough?

Please also merge the GtkHeaderBar and gtk+ bump in the same commit; and see what happens with the libgd warning during autogen.
Comment 18 Yosef Or Boczko 2013-06-11 14:27:29 UTC
Created attachment 246533 [details] [review]
Port to GtkHeaderBar and Bump required GTK+
Comment 19 Aleksander Morgado 2013-06-11 14:52:24 UTC
Review of attachment 246505 [details] [review]:

This patch now doesn't apply on top of the new "Port to GtkHeaderBar and Bump required GTK+" one :/
Comment 20 Aleksander Morgado 2013-06-11 14:55:38 UTC
Review of attachment 246505 [details] [review]:

Actually, also make sure that you add a whitespace before each open parenthesis in this patch.
Comment 21 Aleksander Morgado 2013-06-11 14:57:31 UTC
Sorry for all this review loop, but changing patches independently instead of updating them all together has these things :)

Could you please fix these last things and upload all the patches again? Also, please make sure you mark as "obsolete" patches that are being updated with new ones.
Comment 22 Yosef Or Boczko 2013-06-11 15:12:42 UTC
Created attachment 246536 [details] [review]
Port to GtkHeaderBar and bump required GTK+
Comment 23 Yosef Or Boczko 2013-06-11 15:14:47 UTC
Created attachment 246537 [details] [review]
Make headerbar a titlebar
Comment 24 Yosef Or Boczko 2013-06-11 15:17:51 UTC
Created attachment 246538 [details] [review]
Use GtkButton subclasses instead of GdHeaderButton ones
Comment 25 Yosef Or Boczko 2013-06-11 15:20:58 UTC
Created attachment 246539 [details] [review]
Remove libgd submodule
Comment 26 Yosef Or Boczko 2013-06-11 15:21:53 UTC
I uploaded the revised amendments.
Comment 27 Aleksander Morgado 2013-06-11 15:44:12 UTC
(In reply to comment #26)
> I uploaded the revised amendments.

Please mark as obsolete the patches that no longer should be considered. Go to the Attachments section below, click on "Details" for each of those patches, and then mark the "obsolete" checkbox there. Thanks!
Comment 28 Aleksander Morgado 2013-06-11 15:45:10 UTC
Review of attachment 246538 [details] [review]:

Still multiple whitespaces needs to get added in this patch... :/

::: src/dh-window.c
@@ +675,3 @@
+        }
+
+        back_button = gtk_button_new();

Whitespace needed before parenthesis...

@@ +676,3 @@
+
+        back_button = gtk_button_new();
+        back_image = gtk_image_new_from_icon_name(prev_icon, GTK_ICON_SIZE_MENU);

Whitespace needed before parenthesis...

@@ +677,3 @@
+        back_button = gtk_button_new();
+        back_image = gtk_image_new_from_icon_name(prev_icon, GTK_ICON_SIZE_MENU);
+        gtk_button_set_image(GTK_BUTTON(back_button), back_image);

Whitespace needed before parenthesis...

Really lots of these still to get fixed in this same patch...
Comment 29 Aleksander Morgado 2013-06-11 15:49:34 UTC
Review of attachment 246539 [details] [review]:

There's still some LIBGD_INIT(.. in configure.ac
Comment 30 Yosef Or Boczko 2013-06-11 15:50:17 UTC
Created attachment 246540 [details] [review]
Use GtkButton subclasses instead of GdHeaderButton ones (whitespace needed before the open parenthesis)
Comment 31 Yosef Or Boczko 2013-06-11 15:57:55 UTC
Created attachment 246544 [details] [review]
Remove libgd submodule
Comment 32 Frederic Peters 2013-06-11 16:06:10 UTC
I asked on #gnome-design and the stuff in Design/Whiteboards is not yet appropriate for implementation; I'd keep the close button changes out of the bug report for the moment.
Comment 33 Yosef Or Boczko 2013-06-11 16:20:47 UTC
(In reply to comment #32)
> I asked on #gnome-design and the stuff in Design/Whiteboards is not yet
> appropriate for implementation; I'd keep the close button changes out of the
> bug report for the moment.

Are you sure?
It has been implemented in several applications.

See:
Baobab: https://git.gnome.org/browse/baobab/commit/?id=fe327ebe89c7e86c12766be71c6d376695d2dc2b

gitg: https://git.gnome.org/browse/gitg/commit/?id=87650faead3cc1d7ff2370472b2e5f38a4568996

GNOME Weather: https://git.gnome.org/browse/gnome-weather/commit/?id=2bb7bee1c10df02b453f4eab4b181d28e4152230

GNOME Clocks: https://git.gnome.org/browse/gnome-clocks/commit/?id=8662e90d2ae3c1b8ea030111b967ff3d8deca74c
Comment 34 Yosef Or Boczko 2013-06-12 21:29:41 UTC
You do not push the changes?
Comment 35 Frederic Peters 2013-06-12 21:40:14 UTC
I'd like to have designer input first. Adding the ui-review keyword. You could also try pinging in #gnome-design.
Comment 36 Yosef Or Boczko 2013-06-19 06:42:44 UTC
I think you can implement this design.
It has been applied enough applications (gitg, gnome-clocks, gnome-weather, baobab) so it will be designed for implementation.
Comment 37 Evgeny Bobkin 2013-06-30 13:35:04 UTC
(In reply to comment #30)
> Created an attachment (id=246540) [details] [review]
> Use GtkButton subclasses instead of GdHeaderButton ones (whitespace needed
> before the open parenthesis)

I think once you port buttons form libgd in the same patch you should add context classes like it is done in baobab gnome-clocks etc. otherwise buttons will not look the same way is was in libgd, take a look for example here https://git.gnome.org/browse/gnome-clocks/tree/src/window.vala#n82

close_button.get_style_context ().add_class ("image-button");

moreover for a separator align is missing: separator.valign = Gtk.Align.FILL;

I would suggest that there are screenshots for comparison before and after the change of porting from libgd, there should be no visible change.
Comment 38 Yosef Or Boczko 2013-06-30 16:06:12 UTC
Created attachment 248088 [details] [review]
Port to GtkHeaderBar and bump required GTK+
Comment 39 Yosef Or Boczko 2013-06-30 16:07:07 UTC
Created attachment 248089 [details] [review]
Use GtkButton subclasses instead of GdHeaderButton ones and fix arrow icons in RTL locales
Comment 40 Yosef Or Boczko 2013-06-30 16:07:45 UTC
Created attachment 248090 [details] [review]
Remove libgd submodule
Comment 41 Yosef Or Boczko 2013-06-30 16:24:36 UTC
Created attachment 248091 [details] [review]
Make headerbar a title and add close button

You can wait with this patch.
The quilt codes, you can push.
Comment 42 Yosef Or Boczko 2013-06-30 16:27:21 UTC
Created attachment 248092 [details]
Before the changes
Comment 43 Yosef Or Boczko 2013-06-30 16:28:14 UTC
Created attachment 248093 [details]
After the changes (without close button)
Comment 44 Yosef Or Boczko 2013-06-30 16:29:28 UTC
Created attachment 248094 [details]
After the changes (with close button)
Comment 45 Yosef Or Boczko 2013-06-30 19:23:19 UTC
Created attachment 248100 [details] [review]
Fix a tooltip of forward_button

Error of copy/paste.
Comment 46 Yosef Or Boczko 2013-07-01 14:48:55 UTC
I can push the patches ?
(without the patch 'Make headerbar a title and add close button')
Comment 47 Aleksander Morgado 2013-07-01 17:14:17 UTC
Talked to aday in IRC and he said that we should probably wait to add the close button stuff, as that is not clearly defined yet. So I'll merge all the patches except for the one implementing the close button change.
Comment 48 Yosef Or Boczko 2013-07-01 17:15:58 UTC
I can push the patches.
I got access to git.
Comment 49 Yosef Or Boczko 2013-07-01 17:21:53 UTC
I pushed the patches (without the patch 'Make headerbar a title and add close button').
Comment 50 Aleksander Morgado 2013-07-01 17:38:45 UTC
(In reply to comment #49)
> I pushed the patches (without the patch 'Make headerbar a title and add close
> button').

:( I was going to fixup several whitespace issues that I found and fix the format of the commit logs before doing that.
Comment 51 Aleksander Morgado 2013-07-01 17:43:20 UTC
(In reply to comment #50)
> (In reply to comment #49)
> > I pushed the patches (without the patch 'Make headerbar a title and add close
> > button').
> 
> :( I was going to fixup several whitespace issues that I found and fix the
> format of the commit logs before doing that.

In general, you shouldn't push the patches yourself, unless someone reviewed the patch and added the "accepted-commit-now" tag in the patch; please don't do that again :) I also wanted to re-test the patches myself before pushing them, maybe I should have said that explicitly, sorry for not saying it.

Anyway, for the patches that you pushed, now please mark them as 'committed' in this bugreport.
Comment 52 Aleksander Morgado 2013-07-01 17:45:39 UTC
(In reply to comment #45)
> Created an attachment (id=248100) [details] [review]
> Fix a tooltip of forward_button
> 
> Error of copy/paste.

You didn't push this one, did you? Or did you fixed-it-up with some other commit?
Comment 53 Yosef Or Boczko 2013-07-01 17:53:15 UTC
(In reply to comment #51)
> (In reply to comment #50)
> > (In reply to comment #49)
> > > I pushed the patches (without the patch 'Make headerbar a title and add close
> > > button').
> > 
> > :( I was going to fixup several whitespace issues that I found and fix the
> > format of the commit logs before doing that.
> 
> In general, you shouldn't push the patches yourself, unless someone reviewed
> the patch and added the "accepted-commit-now" tag in the patch; please don't do
> that again :) I also wanted to re-test the patches myself before pushing them,
> maybe I should have said that explicitly, sorry for not saying it.

Just a few hours ago I received a access permissions to git.
The wiki does not say when you can push changes.

> Anyway, for the patches that you pushed, now please mark them as 'committed' in
> this bugreport.

How to do it?

(In reply to comment #52)
> (In reply to comment #45)
> > Created an attachment (id=248100) [details] [review] [details] [review]
> > Fix a tooltip of forward_button
> > 
> > Error of copy/paste.
> 
> You didn't push this one, did you? Or did you fixed-it-up with some other
> commit?

I fixed-it-up with some other commit (with the 'Use GtkButton 
subclasses instead of GdHeaderButton ones' patch).
Comment 54 Aleksander Morgado 2013-07-02 06:06:59 UTC
> > > > I pushed the patches (without the patch 'Make headerbar a title and add close
> > > > button').
> > > 
> > > :( I was going to fixup several whitespace issues that I found and fix the
> > > format of the commit logs before doing that.
> > 
> > In general, you shouldn't push the patches yourself, unless someone reviewed
> > the patch and added the "accepted-commit-now" tag in the patch; please don't do
> > that again :) I also wanted to re-test the patches myself before pushing them,
> > maybe I should have said that explicitly, sorry for not saying it.
> 
> Just a few hours ago I received a access permissions to git.
> The wiki does not say when you can push changes.
> 

In general, you shouldn't push changes to a project in which you are not a maintainer, *unless* someone explicitly asks you to do so. This explicit permission is usually through the 'accepted-commit-now' tag in bugzilla per-patch, or may be less official, like a "go on" in IRC. Having commit rights don't give you the right to push whatever you want, you still should go through the maintainers filter. But don't worry, you'll get used to it :)

Also, note, that even if *I* was the one pushing *your* patch, the authorship in git will still be yours. So if someone tells you he's going to push your patches, just forget about it, you shouldn't go on and push them yourself, you don't need to worry. Specially because maintainers will usually re-review long patches, re-compile and re-test before really pushing them, unless they are simple enough to let the patch creator push them (flagging them with accepted-commit-now).

> > Anyway, for the patches that you pushed, now please mark them as 'committed' in
> > this bugreport.
> 
> How to do it?
> 

Go to the list of patches below, and click in "Details" for each of those already pushed. Then, you'll be able to select "committed" in the combobox for the "status". 

Each patch has a status, and there are different status values. Take a look at them and get used to them; as you'll see them used widely through gnome bugzilla.
Comment 55 Yosef Or Boczko 2013-07-02 07:22:16 UTC
(In reply to comment #54)
> > > > > I pushed the patches (without the patch 'Make headerbar a title and add close
> > > > > button').
> > > > 
> > > > :( I was going to fixup several whitespace issues that I found and fix the
> > > > format of the commit logs before doing that.
> > > 
> > > In general, you shouldn't push the patches yourself, unless someone reviewed
> > > the patch and added the "accepted-commit-now" tag in the patch; please don't do
> > > that again :) I also wanted to re-test the patches myself before pushing them,
> > > maybe I should have said that explicitly, sorry for not saying it.
> > 
> > Just a few hours ago I received a access permissions to git.
> > The wiki does not say when you can push changes.
> > 
> 
> In general, you shouldn't push changes to a project in which you are not a
> maintainer, *unless* someone explicitly asks you to do so. This explicit
> permission is usually through the 'accepted-commit-now' tag in bugzilla
> per-patch, or may be less official, like a "go on" in IRC. Having commit rights
> don't give you the right to push whatever you want, you still should go through
> the maintainers filter. But don't worry, you'll get used to it :)
> 
> Also, note, that even if *I* was the one pushing *your* patch, the authorship
> in git will still be yours. So if someone tells you he's going to push your
> patches, just forget about it, you shouldn't go on and push them yourself, you
> don't need to worry. Specially because maintainers will usually re-review long
> patches, re-compile and re-test before really pushing them, unless they are
> simple enough to let the patch creator push them (flagging them with
> accepted-commit-now).

Thanks for the explanation. I searched such an explanation in the wiki, and not found.
Without knowing the rules, I lost git access pretty quick.

> > > Anyway, for the patches that you pushed, now please mark them as 'committed' in
> > > this bugreport.
> > 
> > How to do it?
> > 
> 
> Go to the list of patches below, and click in "Details" for each of those
> already pushed. Then, you'll be able to select "committed" in the combobox for
> the "status". 
> 
> Each patch has a status, and there are different status values. Take a look at
> them and get used to them; as you'll see them used widely through gnome
> bugzilla.

I do not see combobox.
Should permissions for it?
Comment 56 Aleksander Morgado 2013-07-02 07:43:12 UTC
> 
> > > > Anyway, for the patches that you pushed, now please mark them as 'committed' in
> > > > this bugreport.
> > > 
> > > How to do it?
> > > 
> > 
> > Go to the list of patches below, and click in "Details" for each of those
> > already pushed. Then, you'll be able to select "committed" in the combobox for
> > the "status". 
> > 
> > Each patch has a status, and there are different status values. Take a look at
> > them and get used to them; as you'll see them used widely through gnome
> > bugzilla.
> 
> I do not see combobox.
> Should permissions for it?

Ahh... that may have been it. You're listed just as reporter, not as developer. In general, if you have git access, you should also be marked as developer in bugzilla. You should ask for that to the one who gave you git access. I'll mark them myself as commited.
Comment 57 Aleksander Morgado 2013-07-24 10:21:52 UTC
Nacho has been working also in the close button thing, plus some other things, see bug 704752. Will close this bug as duplicate of that one, as those patches will get merged before the UI freeze.

*** This bug has been marked as a duplicate of bug 704752 ***