feat: add capture delay option to the tray icon
Add a submenu to the tray icon to allow quick selection of the capture delay for captures initiated from the tray icon.
Introduce a new captureDelay config option to store this value.
When a delayed capture is started, a (not too big) countdown is displayed until the capture begins.
Easy delayed screenshot, using the interface, is a feature that I need very often, and it looks like that it is also a regularly requested feature: https://github.com/flameshot-org/flameshot/issues/2625 https://github.com/flameshot-org/flameshot/issues/582 https://github.com/flameshot-org/flameshot/issues/233
And the countdown provided with this would probably also solve: https://github.com/flameshot-org/flameshot/issues/3227 https://github.com/flameshot-org/flameshot/issues/358
Examples:
(Don't mind the submenu that is on the left despite the arrow that is on the right, this is the standard behavior of my desktop when the tray icons are on the right)
Thanks for the PR. We generally prefer to have a vrief discussion before receiving a PR with new features. This is because we can to some extent discuss the implementation direction and reduce your development time, and to avoid massive code change during the review.
Regarding this PR:
- I believe the term "captureDelay" is not clear enough because the launcher window also provides delay from GUI and the user will be very confused if the naming is not clear
- It adds a second way or capturing screenshot with delay from GUI
- The countdown imho is a very welcome change, although I'm not sure if we would face problems regarding it's positioning on mutli-monitor setups. Maybe if the countdown be on the tray icon it would be more deterministic? 🤷
I'm not against this feature, but I personally don't see so much value in, perhaps because I almost never use the tray icon menu 😅. Regardless, looks like a clean implementation, and if users can benefit from it, I have zero objections.
I'm curious to know other dev's and contributors opinion on this PR and feature @borgmanJeremy @jack9603301 @FelixJochems
I understand, sorry for the feature out of the blue and not need to feel forced to merge the feature if ever. (Even if for me personally it is a must have) I did not really took me a lot of time, just I was a little bit frustrated by the missing feature (having to use the cli or another tool) one time too much and just wondered how hard it would be to hack that. And in the end was easy, and same for the next step to add the countdown once I had the first part already.
To reply to your points:
- captureDelay is just the name in the code, never visible in the GUI, if this is what you mean by "User". Otherwise I'm open to alternative suggestion you would have.
- Indeed, but from my point of view I don't really see the added value of the launcher window. It could maybe be removed from the trayicon menu afterward? It is currently not very convenient for delay related things because it does not remember the value and it is complicated to open it each time for delayed screenshot.
- Fyi, I'm in a multiscreen setup and I tested that it is ok. It is shown only on a single screen but it is very discreet. That is its purpose. And it is kind of transparent so you can somehow see behind and click on what is behind like if it was not there. I tried normal notification but it has a lot of problem as in control of the system (timeout not settable, each notification kept in notification center, ...). The tooltip of the trayicon is not shown if you don't let your cursor on it. Seconds as icon might be interesting but especially as it has to change every second reliably and it might be small, I'm less tempted to use that. But I did not try. Also, the small popup might one day be extended with a button (or catch esc key) to interrupt countdown if you want to cancel the screenshot.
By curiosity, how do you use it if not with the tray icon? Just started from cli for single shot?
I usually use the "launcher" which can be accessed from the tray icon to set a delay.
Which platform did you test on? I get this error on wayland KDE:
qt.qpa.wayland: Failed to create popup. Ensure popup QWidgetWindow(0x5587af7a3e40, name="CountdownWindowClassWindow") has a transientParent set.
@borgmanJeremy On Plaasma Wayland on Arch it complied for me cleanly
@mmahmoudian did the feature work for you? It compiled for me but gave that as a runtime error.
I think I would actually use this a lot in my workflow since the only reason I ever open the launcher is to set a delay.
@borgmanJeremy
@mmahmoudian did the feature work for you? It compiled for me but gave that as a runtime error
I think it did. Let me confirm that when I get behind the computer. There is a slight chance that I am not remembering correctly as I've done a quite a bunch of testing and compiling these past few days. I'll report back soon...
Sorry for the little delay of my reply. On my side, I tested on a recent Linux Mint Cinnamon. I'm not sure whether it is X or Wayland, but my guess is more X as screensharing and related features are working flawlessly for me :-)
For the bug, I see a few things online related to Wayland with this issue: https://qt-project.atlassian.net/browse/QTBUG-135158
If ever you have the possibility to do a little bit of debugging on the computer where it crashes: Look inside: CountdownWindow::CountdownWindow()
-
Testing if removing the following would change anything: setAttribute(Qt::WA_TransparentForMouseEvents);
and Qt::WindowTransparentForInput -
Testing geometry related things: For the following block:
QScreen* screen = QGuiApplication::primaryScreen();
if (screen) {
QRect screenGeometry = screen->availableGeometry();
// Calculate position: 5% from right edge, 5% from top edge
int xOffset = screenGeometry.width() * 0.05;
int yOffset = screenGeometry.height() * 0.05;
int x = screenGeometry.right() - width() - xOffset;
int y = screenGeometry.top() + yOffset;
move(x, y);
}
Put a log to see if you have a value in "screen"? You could try to comment out this part, just to see if it would be working without moving the window.
Otherwise, I see the following snippet in the other window that I hope is working for you:
show();
// Call show() first, otherwise the correct geometry cannot be fetched for
// centering the window on the screen
QRect position = frameGeometry();
QScreen* screen = QGuiAppCurrentScreen().currentScreen();
position.moveCenter(screen->availableGeometry().center());
move(position.topLeft());
Maybe you could try to follow the advice and add the "show()" before the "QScreen .." code in Countdownwindow.cpp.
It might be a bug in a recent version of QT also: https://github.com/telegramdesktop/tdesktop/issues/29648#issuecomment-3180285439
It looks to be the same bug as in there: https://bugreports.qt.io/browse/QTCREATORBUG-33151 https://bugreports.qt.io/browse/QTBUG-136110 https://bugreports.qt.io/browse/QTBUG-137702
They are trying to fix it in Qt 6.9.2 or for the next Qt 6.10 if I understood it correctly.
Can you check that are not using a version that is affected by the issue?