During the timeout period Spectacle now displays an animation in the taskmanager.
If the windows is unminized and the button clicked again (possibly multiple
times) the window will hide and show and hide again (depending on how often you
clicked) while all the button presses are processed.
To avoid this situation change the button to cancel the currently running
process. It then can also be triggered by the appropiate shortcut.
Details
- Reviewers
broulik ngraham - Group Reviewers
Spectacle - Commits
- R166:da30d66cc7fd: Change the screenshot button to cancel during the timeout
- Unminimize and click the button
- Let the screenshot finish, the button is in it's original state
- cancel a region selection
Diff Detail
- Repository
- R166 Spectacle
- Branch
- button (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 13553 Build 13571: arc lint + arc unit
Cool
src/Gui/KSWidget.cpp | ||
---|---|---|
126 | Keep the label as it was: "Take a New Screenshot" | |
132 | You forget to set the window title back. It stays at e.g. "2 seconds" if I cancel | |
140 | I would force it to Qt::ToolButtonTextBesideIcon however I tried with diferent styles and it always looked like a QPushButton, so maybe that behavior is only relevant for when inside a QToolbar | |
171 | Unrelated change | |
223 | Clever trick I must say :) | |
src/Gui/KSWidget.h | ||
32 | Coding style. But this can probably just be forward declared here class QAction; | |
83 | While the coding style is ... interesting, I would keep it and the new variables consistent with the rest for now |
src/Gui/KSWidget.h | ||
---|---|---|
83 | I thought given enough time maybe every variable will be touched and the style changed |
src/Gui/KSWidget.cpp | ||
---|---|---|
171 | It looked so bad I thought I had accidentally changed it |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
320 | Can't you just call restoreWindowTitle() in any case? | |
326–328 | What is this supposed to do? Do you mean pixmap.isNull() or pixmap.size().isEmpty()? Looks unrelated to the patch, though. | |
496 | Can you perhaps move the setWindowModified() handling into this method, too? | |
src/Gui/KSWidget.cpp | ||
269 | Thinking about it, would using an enum make this nicer? | |
src/Gui/KSWidget.h | ||
24–25 | Unused |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
320 | No because if you saved a screenshot and then take a new one we want to the title to be "Unsaved" and not the filename of the previous saved file. | |
326–328 | In this patch I also changed that we go through this function if the region selection is canceled (to reset title and and button). |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
496 | Do you think of making this a general method to handle windowModified and title setting? Because otherwise I see no need because windowModified doesn't change when we restore the title. |
I quite like it. One UI suggestion: when the button is displaying "Cancel" rather than "Take new screenshot", maybe we should give it keyboard focus so you can quickly activate it with the spacebar or return key in addition to via a click.
String freeze is in two days if I'm reading https://community.kde.org/Schedules/Applications/19.08_Release_Schedule correctly.