Currently notifications are triggered in two ways. Either by triggering it through ExportManager by supplying an argument in doSave
and doCopyToClipboard which emitted forceNotify or directly by SpectacleCore. I propose moving this responsibility solely to
SpectacleCore since it is much clearer where notifications are triggered. For example notification where triggered in KSMainWindow
which also quitted the whole program. This allows us to remove this behavior from KSMainWindow and simplify the Code a bit. To do
this I had to add a new signal to ExportManager which is emitted after an Image is copied to the Clipboard.
Details
Details
- Reviewers
nicolasfella ngraham
Should work as before.
Diff Detail
Diff Detail
- Repository
- R166 Spectacle
- Branch
- arcpatch-D20180
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 15339 Build 15357: arc lint + arc unit
Comment Actions
Thanks, a nice refactoring. However in testing this broke the "quit after copy" feature; now the image is no longer added to klipper when this setting is used.
Comment Actions
You're right. There seems to be some magic involved. If I wait 250 ms and directly quit after hiding like KSMainWindow instead of showing the notification it works. Testing master I also found that spectacle -b -c does not work and if you copy to the clipboard and then exit manually the image doesn't appear Klipper.
Now to figure out the reason...
Comment Actions
For me it is the same behavior as master:
- If you copy and quit on copy is checked it shows up in the history
- If you copy and then exit manually it doesn't show up in the history but you can still paste it (for example into KolourPaint)
Notice that you have to check "Avoid empty clipboard" in Klipper settings. I think we came to the conclusion that this is the same behavior as for text.
Comment Actions
Yes you're right, I think I retestet the wrong version. However pasting still works.
It seems related to shortly quitting after copying. If I add
QTimer::singleShot(250, &QApplication::quit); to imageExported it appears in the history, but then we can't show the notification.
Comment Actions
Ping? Maybe @davidedmundson has insight why it is only added to the history if the app quits
Comment Actions
I would still like to land this even if we lose the image inside Klipper. It doesn't appear there everytime anyway and pasting still works as mentioned earlier.
Comment Actions
I'll defer to you on the code here, with the caveat that it's on the you-break-it-you-fix-it basis. :)
Comment Actions
Landing this would make D23162 harder and the architecture while here simpler worse for that case. I will do an alternative patch after it is landed