-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
Format popover #207
Format popover #207
Changes from 26 commits
c798771
56d78f3
cf4cd7c
25a3ad4
7c17680
b2cc7e0
4f15596
7e1f7a6
20f9806
eaac4d6
cf92f4f
8159452
1abbd16
562d2df
67e051e
00bd103
7f5c9e9
31b95e2
afa8b3d
27e96c5
8eaf362
db6ca8e
18409ad
d24dc6a
4e5d3a1
16dc234
5d7b0e4
56fc03c
98a4500
3e0310b
f901664
d1d8688
a69647d
042198e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,8 @@ using Gtk; | |
using Cairo; | ||
using Peek.Recording; | ||
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
namespace Peek.Ui { | ||
|
||
[GtkTemplate (ui = "/com/uploadedlobster/peek/application-window.ui")] | ||
|
@@ -41,7 +43,13 @@ namespace Peek.Ui { | |
|
||
[GtkChild] | ||
private Button stop_button; | ||
|
||
|
||
[GtkChild] | ||
private Popover pop_format; | ||
|
||
[GtkChild] | ||
private MenuButton pop_format_menu; | ||
|
||
[GtkChild] | ||
private Label size_indicator; | ||
|
||
|
@@ -56,10 +64,10 @@ namespace Peek.Ui { | |
private File out_file; | ||
private RecordingArea active_recording_area; | ||
private string stop_button_label; | ||
|
||
private signal void recording_finished (); | ||
|
||
private GLib.Settings settings; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
|
||
private const int SMALL_WINDOW_SIZE = 300; | ||
|
||
|
@@ -88,6 +96,8 @@ namespace Peek.Ui { | |
this.in_file = null; | ||
leave_recording_state (); | ||
}); | ||
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These blank lines were not there before so I think they should be removed. Same below. |
||
application.toggle_recording.connect (toggle_recording); | ||
|
||
|
@@ -141,7 +151,7 @@ namespace Peek.Ui { | |
settings.bind ("persist-save-folder", | ||
this, "save_folder", | ||
SettingsBindFlags.DEFAULT); | ||
|
||
// Configure window | ||
this.set_keep_above (true); | ||
this.load_geometry (); | ||
|
@@ -157,6 +167,7 @@ namespace Peek.Ui { | |
// is configured that way. | ||
this.set_close_button_position (); | ||
} | ||
|
||
|
||
public void hide_headerbar () { | ||
this.get_style_context ().add_class ("headerbar-hidden"); | ||
|
@@ -212,6 +223,25 @@ namespace Peek.Ui { | |
|
||
return false; | ||
} | ||
//set format | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing blank line. |
||
private string get_format_name (string format) { | ||
switch (format) { | ||
case OUTPUT_FORMAT_APNG: return _("APNG"); | ||
case OUTPUT_FORMAT_GIF: return _("GIF"); | ||
case OUTPUT_FORMAT_MP4: return _("MP4"); | ||
case OUTPUT_FORMAT_WEBM: return _("WebM"); | ||
default: return ""; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation. |
||
} | ||
|
||
private void select_format (string format) { | ||
recorder.config.output_format = format; | ||
var format_name = get_format_name (format); | ||
record_button.set_label (_("Record as %s").printf (format_name)); | ||
pop_format.hide (); | ||
} | ||
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too many blank lines. |
||
[GtkCallback] | ||
public void on_window_screen_changed (Gdk.Screen? previous_screen) { | ||
|
@@ -283,6 +313,26 @@ namespace Peek.Ui { | |
} | ||
} | ||
} | ||
|
||
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too many blank lines. |
||
[GtkCallback] | ||
private void on_gif_button_clicked (Button source) { | ||
select_format("gif"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation. |
||
[GtkCallback] | ||
private void on_apng_button_clicked (Button source) { | ||
select_format("apng"); | ||
} | ||
[GtkCallback] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this functionality is repeating for all buttons (and maybe there will be even more in the future) I would wrap it in a select_format () helper function, something like this:
That's easier to extend and change in the future. |
||
private void on_webm_button_clicked (Button source) { | ||
select_format("webm"); | ||
} | ||
[GtkCallback] | ||
private void on_mp4_button_clicked (Button source) { | ||
select_format("mp4"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code duplication screams for some kind of unification where the format is a parameter, but I don't know if that's possible when using an XML UI definition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, if you find a more elegant solution let me know, we could use radiobuttons and group them that is the only option I can think of at the moment. |
||
|
||
[GtkCallback] | ||
private void on_record_button_clicked (Button source) { | ||
|
@@ -368,6 +418,7 @@ namespace Peek.Ui { | |
if (!is_recording) { | ||
is_recording = true; | ||
size_indicator.opacity = 0.0; | ||
pop_format_menu.hide(); | ||
record_button.hide (); | ||
if (get_window_width () >= SMALL_WINDOW_SIZE) { | ||
stop_button.set_label (stop_button_label); | ||
|
@@ -386,6 +437,7 @@ namespace Peek.Ui { | |
is_recording = false; | ||
is_postprocessing = false; | ||
stop_button.hide (); | ||
pop_format_menu.show(); | ||
record_button.show (); | ||
unfreeze_window_size (); | ||
|
||
|
@@ -410,10 +462,24 @@ namespace Peek.Ui { | |
|
||
private void update_input_shape () { | ||
// Set an input shape so that the recording view is not clickable | ||
|
||
|
||
var window_region = create_region_from_widget (recording_view.get_toplevel ()); | ||
var recording_view_region = create_region_from_widget (recording_view); | ||
window_region.subtract (recording_view_region); | ||
|
||
//popover menu | ||
if (pop_format.visible) { | ||
var pop_style=pop_format.get_style_context(); | ||
if ( DesktopIntegration.get_theme_name() == "Ambiance" ){ | ||
pop_style.add_class(Gtk.STYLE_CLASS_TITLEBAR); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if we add this for all themes? Does it do any harm? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does not always behave correctly I have tested with over 10 themes and it is really hit and miss, The only reason I have it set for ambiance because the default for the popover menu was white and it just looked horrible. But now we have an easy way to make changes for specific themes :) |
||
} | ||
|
||
var pop_format_region = create_region_from_widget (pop_format); | ||
window_region.union (pop_format_region); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation. |
||
|
||
|
||
// The fallback app menu overlaps the recording area | ||
var fallback_app_menu = get_fallback_app_menu (); | ||
if (fallback_app_menu != null && fallback_app_menu.visible) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!-- Generated with glade 3.20.0 | ||
<!-- Generated with glade 3.20.1 | ||
|
||
Copyright (C) Philipp Wolfer <ph.wolfer@gmail.com> | ||
|
||
|
@@ -31,7 +31,7 @@ Author: Philipp Wolfer <ph.wolfer@gmail.com> | |
<property name="visible">True</property> | ||
<property name="can_focus">False</property> | ||
<property name="tooltip_text" translatable="yes">Start recording</property> | ||
<property name="icon_name">media-playback-start-symbolic</property> | ||
<property name="icon_name">media-record</property> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not so keen on the look of this, can we change it to media-record-symbolic? Better fits the layout I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure it looks good too me. |
||
</object> | ||
<object class="GtkImage" id="icon_stop_button"> | ||
<property name="visible">True</property> | ||
|
@@ -48,8 +48,8 @@ Author: Philipp Wolfer <ph.wolfer@gmail.com> | |
<property name="default_height">250</property> | ||
<property name="icon_name">com.uploadedlobster.peek</property> | ||
<property name="show_menubar">False</property> | ||
<signal name="screen-changed" handler="on_window_screen_changed" swapped="no"/> | ||
<signal name="draw" handler="on_window_draw" swapped="no"/> | ||
<signal name="screen-changed" handler="on_window_screen_changed" swapped="no"/> | ||
<child> | ||
<object class="GtkBox" id="content_area"> | ||
<property name="visible">True</property> | ||
|
@@ -124,13 +124,15 @@ Author: Philipp Wolfer <ph.wolfer@gmail.com> | |
<property name="visible">True</property> | ||
<property name="can_focus">False</property> | ||
<property name="homogeneous">True</property> | ||
<property name="baseline_position">bottom</property> | ||
<property name="layout_style">start</property> | ||
<child> | ||
<object class="GtkButton" id="record_button"> | ||
<property name="label" translatable="yes">_Record</property> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="receives_default">True</property> | ||
<property name="margin_left">45</property> | ||
<property name="image">icon_record_button</property> | ||
<property name="relief">half</property> | ||
<property name="use_underline">True</property> | ||
|
@@ -179,9 +181,106 @@ Author: Philipp Wolfer <ph.wolfer@gmail.com> | |
<property name="non_homogeneous">True</property> | ||
</packing> | ||
</child> | ||
<child> | ||
<object class="GtkMenuButton" id="pop_format_menu"> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="receives_default">True</property> | ||
<property name="margin_right">45</property> | ||
<property name="popover">pop_format</property> | ||
<child> | ||
<placeholder/> | ||
</child> | ||
</object> | ||
<packing> | ||
<property name="expand">True</property> | ||
<property name="fill">True</property> | ||
<property name="position">2</property> | ||
</packing> | ||
</child> | ||
</object> | ||
</child> | ||
</object> | ||
</child> | ||
</template> | ||
<object class="GtkPopover" id="pop_format"> | ||
<property name="can_focus">False</property> | ||
<property name="relative_to">record_button</property> | ||
<child> | ||
<object class="GtkButtonBox"> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="orientation">vertical</property> | ||
<property name="layout_style">start</property> | ||
<child> | ||
<object class="GtkButton" id="gif_button"> | ||
<property name="label" translatable="yes">GIF</property> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="receives_default">True</property> | ||
<signal name="clicked" handler="on_gif_button_clicked" swapped="no"/> | ||
<style> | ||
<class name="format_button"/> | ||
</style> | ||
</object> | ||
<packing> | ||
<property name="expand">True</property> | ||
<property name="fill">True</property> | ||
<property name="position">0</property> | ||
</packing> | ||
</child> | ||
<child> | ||
<object class="GtkButton" id="apng_button"> | ||
<property name="label" translatable="yes">APNG</property> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="receives_default">True</property> | ||
<signal name="clicked" handler="on_apng_button_clicked" swapped="no"/> | ||
<style> | ||
<class name="format_button"/> | ||
</style> | ||
</object> | ||
<packing> | ||
<property name="expand">True</property> | ||
<property name="fill">True</property> | ||
<property name="position">1</property> | ||
</packing> | ||
</child> | ||
<child> | ||
<object class="GtkButton" id="webm_button"> | ||
<property name="label" translatable="yes">WebM</property> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="receives_default">True</property> | ||
<signal name="clicked" handler="on_webm_button_clicked" swapped="no"/> | ||
<style> | ||
<class name="format_button"/> | ||
</style> | ||
</object> | ||
<packing> | ||
<property name="expand">True</property> | ||
<property name="fill">True</property> | ||
<property name="position">2</property> | ||
</packing> | ||
</child> | ||
<child> | ||
<object class="GtkButton" id="mp4_button"> | ||
<property name="label" translatable="yes">MP4</property> | ||
<property name="visible">True</property> | ||
<property name="can_focus">True</property> | ||
<property name="receives_default">True</property> | ||
<signal name="clicked" handler="on_mp4_button_clicked" swapped="no"/> | ||
<style> | ||
<class name="format_button"/> | ||
</style> | ||
</object> | ||
<packing> | ||
<property name="expand">True</property> | ||
<property name="fill">True</property> | ||
<property name="position">3</property> | ||
</packing> | ||
</child> | ||
</object> | ||
</child> | ||
</object> | ||
</interface> |
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 blank lines, should match spacing between other methods.