This way the user knows by the progressbar and by the changing
number when the screenshot is going to be taken.
Details
- Reviewers
ngraham davidre - Group Reviewers
Spectacle - Commits
- R166:c51dc6fdfb78: Count down the seconds until a screenshot is taken in the window title
Diff Detail
- Repository
- R166 Spectacle
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Use ceil instead.
But for now I'll wait until D21638: Display delay in the taskmanager has landed and I'll rebase to master then.
What do you say it is down to one line now! Your latest changes really helped there.
I would have created this diff sooner but it's way too hot in my room and I had to rebuild all dependencies because I switched to developing in a virtual machine. So I was delaying this but seeing that it won't be cooler anytime soon I took this burden on me. :P
I used <QtMath> but I could just as well have used <math.h>. I didn't research what of both might be better. Maybe I'll look into this later.
src/Gui/KSMainWindow.cpp | ||
---|---|---|
295 | This needs to be localized with i18n(); "s" doean't mean "seconds" in all languages. |
Make seconds localized
I actually tried to research if "s" is an international
symbol beforehand and it seemed to me like it was
(https://en.wikipedia.org/wiki/International_System_of_Units)
but I trust your call on this.
src/Gui/KSMainWindow.cpp | ||
---|---|---|
296 | qCeil uses std::ceil internally. It is defined like this in qmath.h: inline int qCeil(qreal v) { using std::ceil; return int(ceil(v)); } |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
295 | Now that I look at this again, I think it needs to use i18np() ("p" for plural) because not every language has the same rules plurals. So this would be setWindowTitle(i18nc("@title:window seconds", "%1 s", "%1 s", blabla); And while we're at it, maybe we could use the full word too: setWindowTitle(i18nc("@title:window seconds", "%1 second", "%1 seconds", blabla); |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
295 | Shouln't then this be rather i18ncp? |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
295 | Right, my bad. |
Looks good to me. I have just one more idea.
When you were to configure 4.5 seconds it would show 5 for half a second then 4.
Maybe we could something like min(int(timeout), ceil(...)).
@felixernst What do you think?
I would agree with that but the UI only allows for integer values as a timeout right now.
Do not use ceil until the first integer was reached
This way when a non-integer value like 4,5 seconds is entered
it won't show 5 seconds in the title for the first .5 seconds of
the timer.
I also made the math more easily understandable by extracting
calculations into separate lines.
I thought non-integer values could not be entered because entering a dot there is not possible for me. Comma works though.
src/Gui/KSMainWindow.cpp | ||
---|---|---|
295 | Prefer descriptive variable names, e.g. timeoutInS -> timeoutInSeconds |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
297 | Hmm I don't see that behavior and also don't understand it. If you screenshot finishes the title should be set to "unsaved" in setScreenshotAndShow |
src/Gui/KSMainWindow.cpp | ||
---|---|---|
297 | What I did:
|