Skip to content
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

Add format to Record button #174

Closed
orschiro opened this issue Aug 11, 2017 · 31 comments
Closed

Add format to Record button #174

orschiro opened this issue Aug 11, 2017 · 31 comments
Milestone

Comments

@orschiro
Copy link
Contributor

Here is my challenge.

My default format is GIF.

But sometimes my recordings become too long and GIF won't work.

Then I need to switch to webm or mp4.

But often I forget to switch back.

Annoying because I then have to redo the video.

It would help me a lot if the record button would say "Record to GIF". Or "Record to WebM".

What do you think?
image

@phw
Copy link
Owner

phw commented Aug 20, 2017

Yes, could be helpful. Maybe even a quick select. My only concern is that it will make the label longer. But since the label autohides anyway this does not matter much.

I am also no opposed to have some kind of quick switching, e.g. by having an arrow dropdown behind the button :)

@orschiro
Copy link
Contributor Author

I am also no opposed to have some kind of quick switching, e.g. by having an arrow dropdown behind the button :)

Ooh, this sounds excitingly good! :-)

@gort818
Copy link
Contributor

gort818 commented Oct 21, 2017

I have an idea for this, what about have a notify event for the record button and have a drop down menu with formats. I will test this out this weekend.

@gort818
Copy link
Contributor

gort818 commented Oct 21, 2017

@phw @orschiro Here is a quick test, how does it look?
What do you guys think?
screenshot from 2017-10-20 22-04-38

@orschiro
Copy link
Contributor Author

@gort818 this looks perfect! How can I test? :-)

@gort818
Copy link
Contributor

gort818 commented Oct 22, 2017

@orschiro It is in my test here https://github.com/gort818/peek/tree/format-popover.. But so far it is just graphical It won't do anything atm, give me some time and I will get it going and let you know when it is ready to test :)

@orschiro
Copy link
Contributor Author

orschiro commented Oct 22, 2017 via email

@gort818
Copy link
Contributor

gort818 commented Oct 22, 2017

@orschiro ok check it out https://github.com/gort818/peek/tree/format-popover build that and let me know what you think.

@orschiro
Copy link
Contributor Author

@gort818 followed the compile from source instructions [1].

But [2] produces an error:

orschiro@x230:/tmp/peek/build$ cmake -DCMAKE_INSTALL_PREFIX=/usr/local ..
-- Could NOT find Vala (missing: VALA_EXECUTABLE) 
CMake Error at /usr/share/cmake-3.9/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find Vala (missing: VALA_EXECUTABLE) (Required is at least
  version "0.22")
Call Stack (most recent call first):
  /usr/share/cmake-3.9/Modules/FindPackageHandleStandardArgs.cmake:377 (_FPHSA_FAILURE_MESSAGE)
  cmake/FindVala.cmake:65 (find_package_handle_standard_args)
  CMakeLists.txt:27 (find_package)


-- Configuring incomplete, errors occurred!
See also "/tmp/peek/build/CMakeFiles/CMakeOutput.log".

Can you help?

[1] https://github.com/gort818/peek/tree/format-popover#from-source
[2] cmake -DCMAKE_INSTALL_PREFIX=/usr/local ..

@gort818
Copy link
Contributor

gort818 commented Oct 22, 2017

You need Vala installed.

@orschiro
Copy link
Contributor Author

Thanks! It finished building peek but if I run it [1], I don't get to see your dropdown menu.

orschiro@x230:/tmp/peek/build$ ls -la
total 672
drwxr-xr-x  9 orschiro orschiro   4096 Okt 22 09:57 .
drwxr-xr-x 12 orschiro orschiro   4096 Okt 22 09:57 ..
-rw-r--r--  1 orschiro orschiro  39387 Okt 22 09:57 application.h
-rw-r--r--  1 orschiro orschiro  45297 Okt 22 09:57 application_internal.h
-rw-r--r--  1 orschiro orschiro  38235 Okt 22 09:57 CMakeCache.txt
drwxr-xr-x 36 orschiro orschiro   4096 Okt 22 09:57 CMakeFiles
-rw-r--r--  1 orschiro orschiro   2264 Okt 22 09:57 cmake_install.cmake
-rw-r--r--  1 orschiro orschiro   1196 Okt 22 09:57 cmake_uninstall.cmake
-rw-r--r--  1 orschiro orschiro   3592 Okt 22 09:57 CPackConfig.cmake
-rw-r--r--  1 orschiro orschiro   4026 Okt 22 09:57 CPackSourceConfig.cmake
-rw-r--r--  1 orschiro orschiro    284 Okt 22 09:57 CTestTestfile.cmake
-rw-r--r--  1 orschiro orschiro   2603 Okt 22 09:57 DartConfiguration.tcl
drwxr-xr-x  5 orschiro orschiro   4096 Okt 22 09:57 data
-rw-r--r--  1 orschiro orschiro  78498 Okt 22 09:57 Makefile
-rwxr-xr-x  1 orschiro orschiro 410800 Okt 22 09:57 peek
drwxr-xr-x  3 orschiro orschiro   4096 Okt 22 09:57 po
drwxr-xr-x  6 orschiro orschiro   4096 Okt 22 09:57 src
drwxr-xr-x  3 orschiro orschiro   4096 Okt 22 09:57 Testing
drwxr-xr-x  3 orschiro orschiro   4096 Okt 22 09:57 tests
drwxr-xr-x  2 orschiro orschiro   4096 Okt 22 09:57 ui

[1] orschiro@x230:/tmp/peek/build$ ./peek

@orschiro
Copy link
Contributor Author

Hang on. I think I didn't check out the right branch...

Let me try again!

@orschiro
Copy link
Contributor Author

@gort818 here is my feedback [1]. :-)

[1] https://www.useloom.com/share/ea24a461c6bf4eda887e1b2dd598bd91

@phw
Copy link
Owner

phw commented Oct 22, 2017

Hey, looking cool so far. This is going into the direction I had imagined.

Two notes:

  1. I actually would like to have a way to still do a one-click record start. So we should somehow separate the start recording with default format, and format selection. I had thought of using a https://developer.gnome.org/gtk3/stable/GtkMenuToolButton.html for this.

  2. Having the popover to be actual clickable is a bit tricky, due to the content area being set to clickthrough. Have a look at AppMenu.update_input_shape(), it already does this for the appmenu, if it is part of the window (https://github.com/phw/peek/blob/master/src/ui/application-window.vala#L404)

Having something like this in update_input_shape() should work:

if (pop_format.visible) {
  var pop_format_region = create_region_from_widget (pop_format);
  window_region.union (pop_format);
}

@gort818
Copy link
Contributor

gort818 commented Oct 22, 2017

@orschiro thanks for the feedback, if you don't mind can you give it another try?
@phw Thanks for the notes, I have updated my branch to include to GtkMenuButton. Everything works on my end. Here are some screenshots:
screenshot from 2017-10-22 09-17-05
screenshot from 2017-10-22 09-17-38

@gort818
Copy link
Contributor

gort818 commented Oct 22, 2017

@orschiro I was curious so I ran ubuntu 17.10 to test for you, so for so good! Take a look!

screenshot from 2017-10-22 19-21-06
test

@orschiro
Copy link
Contributor Author

@gort818 great! Here [1] are two more feedbacks. :-)

[1] https://www.useloom.com/share/0a980874fbb143439efd18f4d3aa1072

@gort818
Copy link
Contributor

gort818 commented Oct 23, 2017

@orschiro Thanks so much for the feedback extremely helpful! I'm not sure why the popmenu is not inheriting the default style on Gnome, on cinnamon it works fine, I will investigate that.

On your second point what do you think about changing the record label when selecting format? For example if I select gif it would say "record as gif" ? Or have check buttons or radio buttons?

@gort818
Copy link
Contributor

gort818 commented Oct 23, 2017

Ok we are getting somewhere!
screenshot from 2017-10-23 23-55-53

@gort818
Copy link
Contributor

gort818 commented Oct 24, 2017

@orschiro Ok after hours of testing there is only an issue with Ubuntu themes, specifically Ambiance theme! So I checked for users themes and if they are using Ambiance it shows like the photo above. I have a feeling they are still many bugs in the Ambiance and Radiance themes.
Arc and Adwaita look gorgeous,
peek-test-ada
peek-test-arc

@orschiro
Copy link
Contributor Author

@gort818
Copy link
Contributor

gort818 commented Oct 26, 2017

@orschiro Thanks once again for the feedback! Unfortunately I cannot seem to do much about the theme, I had to use a work around just to get the popover to match the headerbar colors This is strictly a Ambiance theme issue with Ubuntu by default buttons are white with black text. So anyway take a look and tell me what you think!
I also added changing of the record button label depending on what format you choose.

@phw If you get a chance I would love if you could check it out, I would like to know your opinion as well. https://github.com/gort818/peek/tree/format-popover

@gort818
Copy link
Contributor

gort818 commented Oct 26, 2017

@orschiro Ok I couldn't help myself I made specific style changes for user with the ambiance theme. I think this is as far as I can go..(Been tearing my hair out!) but we made some progress!

peek 2017-10-25 20-45

@orschiro
Copy link
Contributor Author

@gort818 just tried it and it looks and feels great! No further remarks from my side.

Thanks for your work!

@gort818
Copy link
Contributor

gort818 commented Oct 27, 2017

@orschiro Great to hear!! I will do so more slight changes and issue a pr for @phw to review.

Thanks for all your testing!

@orschiro
Copy link
Contributor Author

@gort818 you're welcome! I am very glad I could help and test. :-)

@gort818
Copy link
Contributor

gort818 commented Oct 28, 2017

#207

@phw phw added this to the 1.2.0 milestone Oct 30, 2017
@gort818
Copy link
Contributor

gort818 commented Nov 14, 2017

@orschiro This has been merged.

@orschiro
Copy link
Contributor Author

@gort818 fantastic! Thank you all for your work. I guess we can close this? :-)

@phw
Copy link
Owner

phw commented Nov 14, 2017

@orschiro The development packages should all soon be up-to-date if you want to try it :)

@orschiro
Copy link
Contributor Author

@phw I will definitely give it a try! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants