Merge lp:~l-admin-3/pantheon-greeter/login-button-fix into lp:~elementary-pantheon/pantheon-greeter/trunk

[Marcus Wichelmann]

I have changed the style of the button again and added an icon to the button. It's not perfect, but anyway it looks much better.

Please don't merge before the change in the gtk-theme is merged.

[Raphael Isemann (teemperor)]

I think the better solution is that we ditch that ugly draw-call override here: http://bazaar.launchpad.net/~elementary-pantheon/pantheon-greeter/trunk/view/head:/src/LoginBox.vala#L178

instead of working around the draw-call optimization of GTK+ (it knows that it doesn't need to draw the background of the button because it has the same color as the parent-widget => the credentials area). Sadly we made a hacky solution to make it transparent so GTK+ has no idea that it actually needs to render the background.

The better solution would be to remove my old hack and do it in a sane way like setting the transparency of the credentials-area with a extra css class.

review: Needs Fixing (code and functionality)

[Marcus Wichelmann (l-admin-3)]

Okay... I hope I have understand that correct now.
So the background of the credentialsArea is full transparent and the wallpaper will show through. And because the button uses the background-color (the transparency) of the credentialsArea the background of the button shows a section of the wallpaper. An you mean, if the background-color of the credentialsArea will be set in a stylesheet the button wont be transparent any more..? Or do you mean, that the background of the button should be set to non-transparent, too?

Im new to Gtk and did not know, how it behaves exactly...

[Raphael Isemann (teemperor)]

I'm really bad at explaining, but i'll try anyway :)

The code didn't changed much since the old greeter which looked like that: http://fc02.deviantart.net/fs70/f/2012/128/4/9/rebound_of_pantheon_greeter_by_spiceofdesign-d4yv1m6.png
In short: White-grey loginbox with a white-grey button on the bottom-right. Let's assume that is how the current greeter looked and how we want it to draw it. Also let's call that "white-grey" just grey.

So we tell GTK to render a grey loginbox with a grey button on the bottom-right, and GTK does that exactly like a human would do with the least amount of drawing. It does it like that:
1. We have a white piece of paper
2. We draw the Loginbox first (because it is "lower" or other said further away from the eye of the user), now it looks like that: http://i.imgur.com/gljzb4X.png
3. Now we draw the grey button on the bottom right that it looks like that: http://i.imgur.com/lu2jrjt.png

I know that whoever needs to draw with these 3 steps, would only draw the black border on the step 3 and wouldn't take the grey pencil again and start drawing with a grey pencil on a grey surface just for the button.

The same thing does GTK, it only draws the border.

Now to the current greeter, the only thing that has changed is that we added a ugly hack to prevent GTK from drawing the loginbox.
Due to that the paper stays white on step 2. Also GTK never gets informed that it can't draw the loginbox as we just replace the draw-function with a function that does more or less nothing. So on step 3 GTK assumes that there is already a grey loginbox and continues to work as we discussed above: It only draws the border of the button as it "knows" that the background is already grey and it want to save time.

In the end the result it looks like like that: http://i.imgur.com/hXfrdBa.png

Now to the patches:

Your patch tells GTK that the button is not the default-grey but some different color. Due to that is has to draw the background of the button in step 3, which solves the problem.

The IMHO better patch removes the hack that prevents GTK from drawing the loginbox and instead tells GTK+ itself that the loginbox is transparent. Now GTK knows that it has to draw the whole button in step 3.

[Marcus Wichelmann (l-admin-3)]

Hello,

thanks for that very good explanation, and that you have spent so much time to help me understanding Gtk better. Now I have understood, what you mean, but why should Gtk draw a grey button? If I'm not mistaken there is a transparent background for the button definded in the stylesheet, so If you change the way, how the LoginBox is drawn, the Button will be transparent anyway. In the old loginScreen the button is grey-white, because it uses grey-white background of the loginBox. Now the background looks like the wallpaper and Gtk wouldnt draw the button-background, not only because the button is grey-white too, but because there isn't any background color defined.

Of course, it is much better, to set the transparency of the loginBox using a stylesheet, but Im not shure, that that will solve the indication problem.

I'm sorry, that I can answer only now, I have reinstalled my PC. (My daily build for development isn't very stable, because, it isn't 'stable'.. :D)

[Raphael Isemann (teemperor)]

Ok, i'm confused now. I actually thought we had the same issue with the button as i described above and that looks the greeter currently looks like this: https://lh3.googleusercontent.com/WNaUkfeIrF-l8QgRNxssp5df3ysrqGtNaASCRlrcvVjM_me-sgub7n6Eajkh4ZCcXWJAQw2I8YU=w1896-h844

And i think this patch is for this problem with the transparent button? If it isn't, i talked about a different thing and you have to explain what this patch is for.

If it is, i'm confused as this line in the current source

> login_btn.get_style_context ().add_class ("suggested-action");

already "fixes" the issue by giving the button a different color with the elementary theme. At least that is the same as the "hacky fix" i described above.

So, all in all:

* The bug is already fixed for me in the current source that we have.
* I think someone patched around and fixed it while i was gone for the last 2 months, as i can remove the hacky function-override and it still works. I don't know why but i'm too busy to review all past commits.

What we do now:

* I reject this patch as the issue is already fixed on my build. If the problem still happens on your system (make sure you are updated!) we have to look into that.

* If you want to just add the icon to the button then remove this line from your patch
> login_btn.get_style_context ().add_class ("non-transparent");
and then comment here that you just want to add the icon and you need design-feedback.

review: Needs Information

[Marcus Wichelmann (l-admin-3)]

Hello,

yes, this request is my second merge request for this bug.
First the button was transparent and hard to read, so I have written the temporaly one-lin-fix, that you can find in the current source-code.

Link to my first merge-request:
https://code.launchpad.net/~l-admin-3/pantheon-greeter/login-button-fix/+merge/229259

That makes the button looks blue and so it is readable, but this solution isn't very good, so I have rewritten that fix using an own style-class, that fits.

So this fix you found is my previous fix, that makes it only readable.

This Merge Request is the real fix, and it is the continue of the last request. Im sorry, I did not know, that you havent seen that.

But design-feedback will be also good!

[Raphael Isemann (teemperor)]

I see.

Hmm, i'm probably blind but i can't find where we use a invisible button with just a border defined in the CSS. I mean, there has to be a reason it is transparent in the first place as every other button isn't transparent. If you can find me the reason why it is transparent (so, the line in a CSS file for example) i'll approve whatever is necessary to fix that with highest priority.

Thanks for the good work so far, it's really appreciated and i wish i had a exactly clue why it is still broken. I still assume it is the draw-function as this is what i have noted down here.

[Marcus Wichelmann (l-admin-3)]

All default buttons are transparent. So a button on a white window and a button on a grey window are looking different. The color of the button will adapt the background-color of the parent object.

Example:
http://bazaar.launchpad.net/~elementary-design/egtk/4.x/view/head:/gtk-3.0/gtk-widgets.css#L639

In this line is the definition for a transparent background-transition to a light-transparent black.

[Raphael Isemann (teemperor)]

Oh, wow, i didn't knew that our buttons are transparent by default. Well, 100% my bad, i'll make sure we get this and the prerequisite merged ASAP and good job!

*Versinkt in Schmach*

[Marcus Wichelmann (l-admin-3)]

OK, very thanks!
Im anxious... :)

*freu*

[Marcus Wichelmann (l-admin-3)]

Whats happened to this request? It will be cool, if this inappropriate blue button will be replaced before Freya Beta 2.

[Raphael Isemann (teemperor)]

The branch for eGTK isn't merged yet, so i can't merge this here. That branch was in limbo because we weren't sure if there is not a better way to solve this problem in CSS. But if we haven't found a better solution yet, we probably will never find one.

Anyway, I pinged Dan a few minutes ago that he shall take a look at the theme-branch (and, if possible, approve it), so i can merge this here.

[Raphael Isemann (teemperor)]

Ok, i think Dan and i came to the conclusion that we need a extra-CSS for the greeter. Than we can put your CSS there and everything is fine. The reason is that we need quite some more extra-classes and we would have to flood the elementary-theme with pantheon-greeter specific stuff for that.
I´ll update the greeter that it now uses a greeter-specific CSS and we merge your CSS-branch and this branch here both into the greeter.

Unmerged revisions

276. By Marcus Wichelmann on 2014-08-02

Changed the style of the login-button and added an icon.

=== modified file 'src/CredentialsArea.vala'
--- src/CredentialsArea.vala	2014-08-01 16:46:19 +0000
+++ src/CredentialsArea.vala	2014-08-02 12:39:06 +0000
@@ -130,14 +130,21 @@
     public GuestLogin (LoginOption user) {
         base (user);
 
-        login_btn = new Button.with_label (_("Login"));
+        var btn_image = new Image ();
+        btn_image.set_from_icon_name ("go-jump-symbolic", IconSize.BUTTON);
+
+        login_btn = new Button.with_label (_("New session"));
         login_btn.clicked.connect (() => {
             request_login ();
         });
 
         login_btn.margin_top = 52;
 
-        login_btn.get_style_context ().add_class ("suggested-action");
+        login_btn.get_style_context ().add_class ("non-transparent");
+
+        login_btn.set_image (btn_image);
+        login_btn.set_image_position (PositionType.RIGHT);
+        login_btn.set_always_show_image (true);
 
         attach (login_btn, 0, 1, 1, 1);
     }
@@ -155,4 +162,4 @@
     public override void pass_focus () {
     }
 
-}
+}
\ No newline at end of file