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

Send notification when recording has finished. #142

Merged
merged 16 commits into from
May 14, 2021

Conversation

igordsm
Copy link
Sponsor Contributor

@igordsm igordsm commented Oct 28, 2020

Closes #113 .

pr-notification-camera

This is very open for comments. I had never used notifications and actions before, so this may not be the best or even correct way of doing this. If this is indeed garbage, I would appreciate some pointers on how to do this right.

Best regards,

@cassidyjames
Copy link
Contributor

Thanks for working on this @igordsm! I left a comment on the original issue; as a note, it can be helpful to make sure an issue is confirmed and has input from @elementary/ux before jumping into implementation. That said, if this could be changed to a toast, I think it could be useful.

See: #113 (comment)

@cassidyjames cassidyjames added the Needs Design Waiting for input from the UX team label Oct 28, 2020
@igordsm
Copy link
Sponsor Contributor Author

igordsm commented Nov 16, 2020

@cassidyjames Thank you the input on #113 . I have updated the code to reflect the new design using Toast. Feel free to comment when you have the time.

@cassidyjames cassidyjames requested a review from a team November 25, 2020 18:43
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works fine if the video is saved OK. My only quibble is that there is no checking of the path returned and it could in theory be "", in which a different message should be displayed.

src/MainWindow.vala Outdated Show resolved Hide resolved
@davidmhewitt
Copy link
Member

I've noticed that this doesn't highlight the relevant file in the folder. It is possible to do this by getting the AppInfo of the file manager and then telling it to open the URI of the file. Or there is the ShowItems method on the org.freedesktop.FileManager1 interface.

However, we could follow up with that later.

igordsm and others added 3 commits March 15, 2021 21:58
Co-authored-by: David Hewitt <davidmhewitt@users.noreply.github.com>
@igordsm
Copy link
Sponsor Contributor Author

igordsm commented Mar 16, 2021

@jeremypw If location is empty it now shows the following toast:

Captura de tela de 2021-03-15 22-15-58

@davidmhewitt I'm now using GLib.File.get_parent () to get the folder path. I suppose that's what you were suggesting earlier. Am I correct? I also tried to get the AppInfo for the file manager (using AppInfo.get_default_for_uri (..)) , but keep getting null as result. Any tips?

@igordsm igordsm requested a review from jeremypw March 16, 2021 11:49
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. I think, overall it is clearer to use a second toast, as you have done, than try to modify one toast according the outcome.

@jeremypw
Copy link
Collaborator

Leaving open as there is an outstanding question for @davidmhewitt

Jeremy Wootten added 2 commits May 14, 2021 18:49
…cording-notification

# Conflicts fixed in:
#	src/Widgets/CameraView.vala
@jeremypw
Copy link
Collaborator

This seems to work OK for me now. Adding the AppLaunchContext seems to fix the problem that if Files is already open on another workspace then nothing seems to happen. Now Gala switches workspace to the one that Files is on.

@jeremypw
Copy link
Collaborator

Not sure how FlatPaking this will affect the functionality though.

@jeremypw jeremypw merged commit 64b24d9 into elementary:master May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Waiting for input from the UX team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There's no feedback when a video is recorded
5 participants