*** Bug 604958 has been marked as a duplicate of this bug. ***

Review of attachment 150052 [details] [review]:

Hi Maxim, this is a good start!  I think we need some design before doing too much more work on this though; how exactly the user interface should look and function isn't entirely clear to me.

I'm tempted for now actually to take the old gnome-panel code and either move it into gnome-settings-daemon or do the work to have a child process in gnome-shell.

It might be worth breaking out the CSS changes into a separate patch though?

Created attachment 151634 [details] [review]
Port runDialog to css.

Bind <Ctrl>+<Tab> for run in terminal.

(In reply to comment #3)
> Bind <Ctrl>+<Tab> for run in terminal.

you mean <Ctrl>+<Return>

Created attachment 151886 [details] [review]
Corrected patch

Fixed typo

Thanks for updating the patch!  So I have two high level concerns here:

* As I mentioned before the UI really needs design (visual and interaction).  I really don't like the list of apps on the left at the moment.  This depends somewhat on what we do for more-applications, especially more-applications-search.

Any other opinions on this?

* It makes alt-f2 noticeably slower to come up the first time; not sure where the blame lies, but we do read a lot of application data, create a lot of ClutterActors etc.  Will need profiling.

Created attachment 154601 [details] [review]
Port runDialog to css

Merge with HEAD.

Review of attachment 154601 [details] [review]:

Looks good and gets us one stop closer to port everything to CSS ;)

There are some issues that need fixing (one of them might be actually a bug in St.Label), rest is minor though.

::: data/theme/gnome-shell.css
@@ +759,3 @@
+    background-image: url("dialog-error.svg");
+    width: 36px;
+    height: 36px;

The error icon (24x24px) was centered in a 36x36px box, you do stretch the icon to be 36x36 sized here (i.e it is bigger that it is used to be). 

Which looks somehow odd ...

::: js/ui/runDialog.js
@@ +207,3 @@
         global.stage.add_actor(this._group);
 
+        this._boxH = new St.Bin({ style_class: 'run-dialog-box',

I am not sure the name _boxH makes much sense anymore, I'd just call it _box

@@ +227,1 @@
+        this._entry = entry.clutter_text;

Should probably be renamed to _entryText

@@ +230,1 @@
+        this._errorBox = new St.BoxLayout({});

You don't need to pass {} here just do ()

@@ +236,1 @@
+        this._errorBox.add(errorIcon, {});

this._errorBox.add(errorIcon) should be enough.

@@ +239,3 @@
 
+        this._errorMessage = new St.Label({ style_class: 'run-dialog-error-label'});
+        this._errorMessage.clutter_text.line_wrap = true;

This somehow does not work as expected the text does not get wrapped but truncated.

foo ...

instead of

foo
bar

Which results into only a part of the error message being actually visible.

@@ +332,2 @@
                 this._errorBox.show();
+                this._boxH.style_changed();

Why is that needed?

Created attachment 154742 [details] [review]
Port runDialog to css

> This somehow does not work as expected the text does not get wrapped but
> truncated.

Strange.. It work fine for me...(Clutter 1.0.8.1)

> Why is that needed?

Without this, message will show with delay.

> The error icon (24x24px) was centered in a 36x36px box, you do stretch the icon
> to be 36x36 sized here (i.e it is bigger that it is used to be).

It was icon with size 36x36. Now(with same css style), image centered.

Created attachment 154743 [details]
Error message without patch applied

Created attachment 154744 [details]
Error message with the patch applied

(In reply to comment #9)
> Created an attachment (id=154742) [details] [review]
> Port runDialog to css
> 
> > This somehow does not work as expected the text does not get wrapped but
> > truncated.
> 
> Strange.. It work fine for me...(Clutter 1.0.8.1)

Odd, it doesn't here (see screenshots) ... using jhbuild with the clutter-1.0 branch.

> > Why is that needed?
> 
> Without this, message will show with delay.

Well removing it does not make any difference here, but even so wouldn't queue_relayout() be better suited?

> > The error icon (24x24px) was centered in a 36x36px box, you do stretch the icon
> > to be 36x36 sized here (i.e it is bigger that it is used to be).
> 
> It was icon with size 36x36. Now(with same css style), image centered.

This looks fine now.

Created attachment 154767 [details] [review]
Port runDialog to css

Review of attachment 154767 [details] [review]:

Looks good now, as discussed on IRC we go with this way instead of wrapping.

Review of attachment 154767 [details] [review]:

Looks good now, as discussed on IRC we go with this way instead of wrapping.

Created attachment 154783 [details] [review]
Use Lightbox in runDialog

Review of attachment 154783 [details] [review]:

It might make sense to split the "port lightbox to st/css" part from the "use lightbox in run dialog" part. If you want to commit the patch as-is, you should at least update the commit message - right now it skips over the lightbox part.

Something like:
Port Lightbox to CSS and use it in runDialog

Make Lightbox stylable and add a common style. Use it in runDialog.

Looks fixed to me.