Differential D16353 Diff 44029 src/statusbar/statusbarspaceinfo.cpp

Changeset View

Standalone View

View Options

src/statusbar/statusbarspaceinfo.cpp

Show All 19 Lines
20 20​
21#include "statusbarspaceinfo.h" 21​#include "statusbarspaceinfo.h"
22 22​
23#include "spaceinfoobserver.h" 23​#include "spaceinfoobserver.h"
24 24​
25#include <KLocalizedString> 25​#include <KLocalizedString>
26#include <KNS3/KMoreToolsMenuFactory> 26​#include <KNS3/KMoreToolsMenuFactory>
27#include <knewstuff_version.h> 27​#include <knewstuff_version.h>
28 28​
cfeckUnsubmitted
Done

`#include <KColorScheme>' (and move above the KLocalizedString, we (sometimes) try to keep includes alphabetical)

cfeck: `#include <KColorScheme>' (and move above the KLocalizedString, we (sometimes) try to keep…
29#include <QMouseEvent> 29​#include <QMouseEvent>
30​#include <QPalette>
30 31​
31StatusBarSpaceInfo::StatusBarSpaceInfo(QWidget* parent) : 32​StatusBarSpaceInfo::StatusBarSpaceInfo(QWidget* parent) :
32 KCapacityBar(KCapacityBar::DrawTextInline, parent), 33​ KCapacityBar(KCapacityBar::DrawTextInline, parent),
33 m_observer(nullptr) 34​ m_observer(nullptr)
34{ 35​{
35 setCursor(Qt::PointingHandCursor); 36​ setCursor(Qt::PointingHandCursor);
36} 37​}
37 38​
▲ Show 20 Lines • Show All 62 Lines • ▼ Show 20 Line(s)100 if (size == 0) {
100 setText(i18nc("@info:status", "Unknown size")); 101​ setText(i18nc("@info:status", "Unknown size"));
101 setValue(0); 102​ setValue(0);
102 update(); 103​ update();
103 } else { 104​ } else {
104 const quint64 available = m_observer->available(); 105​ const quint64 available = m_observer->available();
105 const quint64 used = size - available; 106​ const quint64 used = size - available;
106 const int percentUsed = qRound(100.0 * qreal(used) / qreal(size)); 107​ const int percentUsed = qRound(100.0 * qreal(used) / qreal(size));
107 108​
109​ if (percentUsed >= 90) {
elvisangelaccioUnsubmitted
Done

Please use a descriptive variable name (e.g. palette)

elvisangelaccio: Please use a descriptive variable name (e.g. `palette`)
110​ QPalette p = this->palette();
elvisangelaccioUnsubmitted
Done

Why KColorScheme::Selection ? That should be used for items that can be selected.

We should probably use KColorScheme::Window instead

elvisangelaccio: Why `KColorScheme::Selection` ? That should be used for items that can be selected. We should…
111​ p.setColor(QPalette::Highlight, QColor(Qt::red));
112​ this->setPalette(p);
113​ }
elvisangelaccioUnsubmitted
Done

Why QPalette::Highlight ? That's supposed to be used for the selected text's color

elvisangelaccio: Why `QPalette::Highlight` ? That's supposed to be used for the selected text's color
sourabhbossUnsubmitted
Done

Then what should i use
For text i have used

QPalette::HighlightedText
sourabhboss: Then what should i use For text i have used ``` QPalette::HighlightedText ```
114​
elvisangelaccioUnsubmitted
Done

What's the second setBrush() needed for?

elvisangelaccio: What's the second `setBrush()` needed for?
108 setText(i18nc("@info:status Free disk space", "%1 free", KIO::convertSize(available))); 115​ setText(i18nc("@info:status Free disk space", "%1 free", KIO::convertSize(available)));
cfeckUnsubmitted
Done

Shouldn't this be QPalette::HightlightedText?

cfeck: Shouldn't this be `QPalette::HightlightedText`?
cfeckUnsubmitted
Done

... and it should also be in the previous if branch ? I was confused until I read the actual mail.

cfeck: ... and it should also be in the previous `if` branch ? I was confused until I read the actual…
109 setUpdatesEnabled(false); 116​ setUpdatesEnabled(false);
cfeckUnsubmitted
Done

} else {

cfeck: `} else {`
110 setValue(percentUsed); 117​ setValue(percentUsed);
111 setUpdatesEnabled(true); 118​ setUpdatesEnabled(true);
112 update(); 119​ update();
113 } 120​ }
114} 121​}
mevenAuthorUnsubmitted
Not Done

Is this the correct way to do this ?

meven: Is this the correct way to do this ?
115 122​