Differential D21651 Diff 60834 src/Gui/KSMainWindow.cpp

Changeset View

Standalone View

View Options

src/Gui/KSMainWindow.cpp

Context not available.
34#include <QPrintDialog> 34​#include <QPrintDialog>
35#include <QPushButton> 35​#include <QPushButton>
36#include <QTimer> 36​#include <QTimer>
37​#include <QtMath>
37#include <QVariantAnimation> 38​#include <QVariantAnimation>
38#include <QVBoxLayout> 39​#include <QVBoxLayout>
39 40​
Context not available.
291 delayAnimation->setDuration(timer->interval()); 292​ delayAnimation->setDuration(timer->interval());
292 connect(delayAnimation, &QVariantAnimation::valueChanged, this, [=] { 293​ connect(delayAnimation, &QVariantAnimation::valueChanged, this, [=] {
293 unityUpdate({ {QStringLiteral("progress"), delayAnimation->currentValue().toDouble()} }); 294​ unityUpdate({ {QStringLiteral("progress"), delayAnimation->currentValue().toDouble()} });
295​ setWindowTitle(i18ncp("@title:window", "%1 second", "%1 seconds",
ngrahamUnsubmitted
Done

This needs to be localized with i18n(); "s" doean't mean "seconds" in all languages.

ngraham: This needs to be localized with `i18n()`; "s" doean't mean "seconds" in all languages.
ngrahamUnsubmitted
Done

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);

ngraham: Now that I look at this again, I think it needs to use `i18np()` ("p" for plural) because not…
davidreUnsubmitted
Done

Shouln't then this be rather i18ncp?

davidre: Shouln't then this be rather `i18ncp`?
ngrahamUnsubmitted
Done

Right, my bad.

ngraham: Right, my bad.
ngrahamUnsubmitted
Done

Prefer descriptive variable names, e.g. timeoutInS -> timeoutInSeconds

ngraham: Prefer descriptive variable names, e.g. `timeoutInS` -> `timeoutInSeconds`
296​ qCeil((1 - delayAnimation->currentValue().toFloat()) * theTimeout / 1000)));
davidreUnsubmitted
Not Done

Is there a difference between qCeil and ceil/std::ceil? I genuinely don't know or what we prefer. Maybe @broulik does?

davidre: Is there a difference between `qCeil` and `ceil`/`std::ceil`? I genuinely don't know or what…
felixernstAuthorUnsubmitted
Not Done

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));
}
felixernst: qCeil uses std::ceil internally. It is defined like this in qmath.h: ``` inline int qCeil…
294 }); 297​ });
broulikUnsubmitted
Not Done

You forgot to set it back when the animation stops. When screenshot finishes I now have a "0 seconds" window, also affects D22234

broulik: You forgot to set it back when the animation stops. When screenshot finishes I now have a "0…
davidreUnsubmitted
Not Done

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

davidre: Hmm I don't see that behavior and also don't understand it. If you screenshot finishes the…
broulikUnsubmitted
Not Done

What I did:

  • In Spectacle main window choose "Rectangular region"
  • Set timeout to 2 seconds
  • Hit "take new screenshot"
  • When region selector comes up, hit Escape to cancel
  • Spectacle unminimizes and now has "0 seconds" as its title
broulik: What I did: * In Spectacle main window choose "Rectangular region" * Set timeout to 2 seconds *…
davidreUnsubmitted
Not Done

Ah yes when you cancel. I will fix it in D22234

davidre: Ah yes when you cancel. I will fix it in D22234
295 connect(timer, &QTimer::timeout, this, [=] { 298​ connect(timer, &QTimer::timeout, this, [=] {
296 this->hide(); 299​ this->hide();
Context not available.