-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Option to turn off progress bar and display text #59
base: master
Are you sure you want to change the base?
Conversation
Hey, looks pretty good, thanks for the contribution! Gonna look into it a bit more and then merge it. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, sorry for responding only now, I reviewed the changes and it looks good to me. Regarding the code formatting comments I made, I have to say that I would generally need to clean up at some point, so if you ignore those, I wouldn't mind.
The only thing that would be nice in addition is adding the an additional option to change the font like you did for the font size.
{ null } | ||
}; | ||
|
||
public AvizoClient() | ||
{ | ||
Object(application_id: "org.danb.avizo.client", | ||
flags: ApplicationFlags.HANDLES_COMMAND_LINE); | ||
flags: ApplicationFlags.HANDLES_COMMAND_LINE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow the indentation got broken here.
|
||
_service = Bus.get_proxy_sync(BusType.SESSION, "org.danb.avizo.service", | ||
"/org/danb/avizo/service"); | ||
"/org/danb/avizo/service"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here as well, but here it was broken before it seems.
|
||
public string text { get; set; } | ||
|
||
public int font_size {get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space before get
.
@@ -151,7 +158,7 @@ public class AvizoWindow : Gtk.Window | |||
} | |||
opacity += animation_sec_elapsed/fade_in; | |||
if (opacity > 1) opacity = 1; | |||
print("Fade in: %f\n", opacity); | |||
//print("Fade in: %f\n", opacity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you remove it intentionally or was it just while debugging?
@@ -165,7 +172,7 @@ public class AvizoWindow : Gtk.Window | |||
} | |||
opacity -= animation_sec_elapsed/fade_out; | |||
if (opacity < 0) opacity = 0; | |||
print("Fade out: %f\n", opacity); | |||
//print("Fade out: %f\n", opacity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question here.
@@ -194,7 +201,16 @@ public class AvizoWindow : Gtk.Window | |||
Gdk.cairo_set_source_rgba(ctx, background); | |||
draw_round_rect(ctx, border_width, border_width, _width - 2 * border_width, _height - 2 * border_width, border_radius - border_width); | |||
|
|||
Gdk.cairo_set_source_rgba(ctx, bar_bg_color); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many extra lines here.
width, | ||
height); | ||
} | ||
ctx.select_font_face ("Sans", Cairo.FontSlant.NORMAL, Cairo.FontWeight.BOLD); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to be able to choose the font as well? For the font size you added that option, too.
Cairo.TextExtents extents; | ||
ctx.text_extents(text, out extents); | ||
double text_x = _width*0.5-(extents.width/2 + extents.x_bearing); | ||
double text_y = _height*0.9; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation.
|
||
ctx.set_operator(Cairo.Operator.OVER); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra line not needed.
@@ -378,6 +418,11 @@ public class AvizoService : GLib.Object | |||
window.set_accept_focus(false); | |||
} | |||
|
|||
if(image_path!="") | |||
{ | |||
window.image_resource =""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation.
I think Related: |
Added 3 command line options:
Demonstration: