Add key shortcut label #285
Conversation
|
|
|
Hey, thanks and sorry for the long delay. I want to review this but just don't fine the time :( Hope to get back to Peek next week. |
|
No problem, no rush it took me three months to get around to doing this! :) |
|
@phw Have you got a chance to take a look? |
There was a problem hiding this comment.
Thanks a lot for this, it is working really well, also with the resizing! And sorry for the late reply again, but I wanted to give this a proper testing before merge and just struggled finding the time.
Only a smaller code change and some minor styling. Please let me know if you have the time to do the changes or if I should take care of it.
src/ui/application-window.vala
Outdated
| } | ||
|
|
||
| if (!is_recording) { | ||
| var shortcut =Application.get_app_settings(); | ||
| string keys =shortcut.get_string("keybinding-toggle-recording"); |
There was a problem hiding this comment.
You should be able to replace the custom handling of the shortcut string by using Gtk's builtin functions for that:
string keys = shortcut.get_string ("keybinding-toggle-recording");
uint accelerator_key;
Gdk.ModifierType accelerator_mods;
Gtk.accelerator_parse (keys, out accelerator_key, out accelerator_mods);
var shortcut_hint = Gtk.accelerator_get_label (accelerator_key, accelerator_mods);
shortcut_label.set_text ("Start/Stop: " + shortcut_hint);
See also shortcut-label.vala where I also used this to display the shortcut on older Gtk versions.
There was a problem hiding this comment.
well that makes it easier thanks!
src/ui/application-window.vala
Outdated
| } | ||
|
|
||
| if (!is_recording) { | ||
| var shortcut =Application.get_app_settings(); |
There was a problem hiding this comment.
Please make sure to have a space before and after =. Also Peek commonly uses a space before function parantheses. That's not a style I normally use, but most other Vala apps I saw used that, and I thought it would make sense to follow commonly used coding style ;)
There was a problem hiding this comment.
oops.. that is one thing I really need to work on
|
Thanks for the review, I am glad it worked as it is supposed to. I should get around to it today. |
|
Ok pushed the changes.. another note I changed this line
to
on one of my desktops running Mint 18.3 get_window_width was not working as intended .. any thoughts on that.. on my arch machine no issues at all. But with Mint area.width is working. |
Was this with Cinnamon or Mate? You could try to add some debug output to see what values get_window_width and area.width actually have. Not sure about this, but I'll merge it for now. Doesn't make a big difference anyway :) |
|
** (peek:26573): DEBUG: application-window.vala:345: area.width: 298 That is interesting.. get_window_width is always 48px larger on linux mint |
#234
I added a shortcut label to display when the program is started and the window has moved, also it will scale down when the window reaches a smaller size, it will scale as soon as the record button is hidden.