Changeset View
Changeset View
Standalone View
Standalone View
src/kcolorcombo.cpp
Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Line(s) | |||||
64 | 64 | | |||
65 | KColorComboDelegate::~KColorComboDelegate() | 65 | KColorComboDelegate::~KColorComboDelegate() | ||
66 | { | 66 | { | ||
67 | } | 67 | } | ||
68 | 68 | | |||
69 | void KColorComboDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const | 69 | void KColorComboDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const | ||
70 | { | 70 | { | ||
71 | // background | 71 | // background | ||
72 | QColor innercolor(Qt::white); | | |||
73 | bool isSelected = (option.state & QStyle::State_Selected); | 72 | bool isSelected = (option.state & QStyle::State_Selected); | ||
74 | bool paletteBrush = (k_colorcombodelegate_brush(index, Qt::BackgroundRole).style() == Qt::NoBrush); | 73 | bool paletteBrush = (k_colorcombodelegate_brush(index, Qt::BackgroundRole).style() == Qt::NoBrush); | ||
75 | if (isSelected) { | 74 | const QColor innerColor = option.palette.color( isSelected ? QPalette::Highlight : QPalette::Base); | ||
76 | innercolor = option.palette.color(QPalette::Highlight); | 75 | | ||
patrickelectric: Missing const, also the name should be `innerColor` with a capital C if we are following the… | |||||
araujoluis: Done! | |||||
77 | } else { | | |||
78 | innercolor = option.palette.color(QPalette::Base); | | |||
79 | } | | |||
80 | // highlight selected item | 76 | // highlight selected item | ||
81 | QStyleOptionViewItem opt(option); | 77 | QStyleOptionViewItem opt(option); | ||
82 | opt.showDecorationSelected = true; | 78 | opt.showDecorationSelected = true; | ||
83 | QStyle *style = opt.widget ? opt.widget->style() : QApplication::style(); | 79 | QStyle *style = opt.widget ? opt.widget->style() : QApplication::style(); | ||
84 | style->drawPrimitive(QStyle::PE_PanelItemViewItem, &opt, painter, opt.widget); | 80 | style->drawPrimitive(QStyle::PE_PanelItemViewItem, &opt, painter, opt.widget); | ||
85 | QRect innerrect = option.rect.adjusted(FrameMargin, FrameMargin, -FrameMargin, -FrameMargin); | 81 | QRect innerrect = option.rect.adjusted(FrameMargin, FrameMargin, -FrameMargin, -FrameMargin); | ||
82 | | ||||
83 | auto textColorLambda = [&paletteBrush, &isSelected, &option, innerColor]() -> QColor { | ||||
patrickelectric: missing reference for innercolor ? | |||||
araujoluis: Yes, reference not found for innercolor or innerColor | |||||
84 | QColor textColor; | ||||
85 | if (paletteBrush) { | ||||
86 | textColor = option.palette.color( isSelected ? QPalette::HighlightedText : QPalette::Text); | ||||
87 | } else { | ||||
88 | int unused, v; | ||||
89 | innerColor.getHsv(&unused, &unused, &v); | ||||
Please don't use "value" component to calculate the brightness of a color. #000081 is much darker than #808080. To decide, use qGray(). The threshold also needs to be higher than 128; I use 170, but this mostly depends on correctness of monitor gamma settings. cfeck: Please don't use "value" component to calculate the brightness of a color. #000081 is much… | |||||
araujoluis: Done! | |||||
90 | textColor = v > 128 ? Qt::black : Qt::white; | ||||
I would probably do what Kirigami does for ColorUtils::brightnessForColor here: ((0.299 * color.red() + 0.587 * color.green() + 0.114 * color.blue()) / 255) > 0.5 ? Qt::black : Qt::white cblack: I would probably do what Kirigami does for `ColorUtils::brightnessForColor` here: `((0.299 *… | |||||
araujoluis: Done! | |||||
91 | } | ||||
92 | return textColor; | ||||
93 | }; | ||||
94 | | ||||
86 | // inner color | 95 | // inner color | ||
96 | QVariant tv = index.data(Qt::DisplayRole); | ||||
patrickelectric: Missing const. | |||||
87 | QVariant cv = index.data(ColorRole); | 97 | QVariant cv = index.data(ColorRole); | ||
88 | if (cv.type() == QVariant::Color) { | 98 | if (cv.type() == QVariant::Color) { | ||
patrickelectric: missing const. | |||||
89 | QColor tmpcolor = cv.value<QColor>(); | 99 | const QColor tmpColor = cv.value<QColor>(); | ||
90 | if (tmpcolor.isValid()) { | 100 | if (tmpColor.isValid()) { | ||
91 | innercolor = tmpcolor; | | |||
92 | paletteBrush = false; | 101 | paletteBrush = false; | ||
93 | painter->setPen(Qt::transparent); | 102 | painter->setPen(Qt::transparent); | ||
94 | painter->setBrush(innercolor); | 103 | painter->setBrush(tmpColor); | ||
95 | QPainter::RenderHints tmpHint = painter->renderHints(); | 104 | QPainter::RenderHints tmpHint = painter->renderHints(); | ||
96 | painter->setRenderHint(QPainter::Antialiasing); | 105 | painter->setRenderHint(QPainter::Antialiasing); | ||
97 | painter->drawRoundedRect(innerrect, 2, 2); | 106 | const QRect colorRect = !tv.toString().isEmpty() ? QRect(innerrect.x(), innerrect.y() + innerrect.height() / 2, innerrect.width(), innerrect.height() / 2) : innerrect; | ||
107 | painter->drawRoundedRect(colorRect, 2, 2); | ||||
108 | painter->setPen(textColorLambda()); | ||||
109 | painter->drawText(colorRect, tmpColor.name()); | ||||
Maybe changing the logic to make it more simpler: QRect colorRect = tv.toString().isEmpty() ? innerrect : QRect(innerrect.x(), innerrect.y() + innerrect.height() / 2, innerrect.width(), innerrect.height() / 2); patrickelectric: Maybe changing the logic to make it more simpler:
```
QRect colorRect = tv.toString().isEmpty()… | |||||
araujoluis: Done! | |||||
araujoluis: Done! | |||||
patrickelectric: It's missing const | |||||
araujoluis: Done! | |||||
araujoluis: Done! | |||||
98 | painter->setRenderHints(tmpHint); | 110 | painter->setRenderHints(tmpHint); | ||
99 | painter->setBrush(Qt::NoBrush); | 111 | painter->setBrush(Qt::NoBrush); | ||
100 | } | 112 | } | ||
101 | } | 113 | } | ||
102 | // text | 114 | // text | ||
103 | QVariant tv = index.data(Qt::DisplayRole); | | |||
104 | if (tv.type() == QVariant::String) { | 115 | if (tv.type() == QVariant::String) { | ||
105 | QString text = tv.toString(); | 116 | QString text = tv.toString(); | ||
106 | QColor textColor; | 117 | painter->setPen(textColorLambda()); | ||
107 | if (paletteBrush) { | | |||
108 | if (isSelected) { | | |||
109 | textColor = option.palette.color(QPalette::HighlightedText); | | |||
110 | } else { | | |||
111 | textColor = option.palette.color(QPalette::Text); | | |||
112 | } | | |||
113 | } else { | | |||
114 | int unused, v; | | |||
115 | innercolor.getHsv(&unused, &unused, &v); | | |||
116 | if (v > 128) { | | |||
117 | textColor = Qt::black; | | |||
118 | } else { | | |||
119 | textColor = Qt::white; | | |||
120 | } | | |||
121 | } | | |||
122 | painter->setPen(textColor); | | |||
123 | painter->drawText(innerrect.adjusted(1, 1, -1, -1), text); | 118 | painter->drawText(innerrect.adjusted(1, 1, -1, -1), text); | ||
124 | } | 119 | } | ||
125 | } | 120 | } | ||
126 | 121 | | |||
127 | QSize KColorComboDelegate::sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const | 122 | QSize KColorComboDelegate::sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const | ||
128 | { | 123 | { | ||
129 | Q_UNUSED(index) | 124 | Q_UNUSED(index) | ||
130 | 125 | | |||
131 | // the width does not matter, as the view will always use the maximum width available | 126 | // the width does not matter, as the view will always use the maximum width available | ||
132 | return QSize(100, option.fontMetrics.height() + 2 * FrameMargin); | 127 | const QVariant displayValue = index.model()->data(index); | ||
128 | const double height = option.fontMetrics.height() * (!displayValue.toString().isEmpty() ? 2 : 1) + 2 * FrameMargin; | ||||
129 | return QSize(100, height); | ||||
133 | } | 130 | } | ||
134 | 131 | | |||
135 | static const uchar standardPalette[][4] = { | 132 | static const uchar standardPalette[][4] = { | ||
136 | { 255, 255, 255 }, // white | 133 | { 255, 255, 255 }, // white | ||
137 | { 192, 192, 192 }, // light gray | 134 | { 192, 192, 192 }, // light gray | ||
138 | { 160, 160, 160 }, // gray | 135 | { 160, 160, 160 }, // gray | ||
139 | { 128, 128, 128 }, // dark gray | 136 | { 128, 128, 128 }, // dark gray | ||
140 | { 0, 0, 0 }, // black | 137 | { 0, 0, 0 }, // black | ||
Show All 37 Lines | 172 | public: | |||
178 | void addColors(); | 175 | void addColors(); | ||
179 | void setCustomColor(const QColor &color, bool lookupInPresets = true); | 176 | void setCustomColor(const QColor &color, bool lookupInPresets = true); | ||
180 | 177 | | |||
181 | // slots | 178 | // slots | ||
182 | void _k_slotActivated(int index); | 179 | void _k_slotActivated(int index); | ||
183 | void _k_slotHighlighted(int index); | 180 | void _k_slotHighlighted(int index); | ||
184 | 181 | | |||
185 | KColorCombo *q; | 182 | KColorCombo *q; | ||
186 | QList<QColor> colorList; | 183 | KColorCombo::ColorList colorList; | ||
187 | QColor customColor; | 184 | QColor customColor; | ||
188 | QColor internalcolor; | 185 | QColor internalcolor; | ||
189 | }; | 186 | }; | ||
190 | 187 | | |||
191 | KColorComboPrivate::KColorComboPrivate(KColorCombo *qq) | 188 | KColorComboPrivate::KColorComboPrivate(KColorCombo *qq) | ||
192 | : q(qq), customColor(Qt::white) | 189 | : q(qq), customColor(Qt::white) | ||
193 | { | 190 | { | ||
194 | } | 191 | } | ||
195 | 192 | | |||
196 | void KColorComboPrivate::setCustomColor(const QColor &color, bool lookupInPresets) | 193 | void KColorComboPrivate::setCustomColor(const QColor &color, bool lookupInPresets) | ||
197 | { | 194 | { | ||
198 | if (lookupInPresets) { | 195 | if (lookupInPresets) { | ||
199 | if (colorList.isEmpty()) { | 196 | if (colorList.isEmpty()) { | ||
200 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | 197 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | ||
201 | if (standardColor(i) == color) { | 198 | if (standardColor(i) == color) { | ||
202 | q->setCurrentIndex(i + 1); | 199 | q->setCurrentIndex(i + 1); | ||
203 | internalcolor = color; | 200 | internalcolor = color; | ||
204 | return; | 201 | return; | ||
205 | } | 202 | } | ||
206 | } | 203 | } | ||
207 | } else { | 204 | } else { | ||
208 | int i = colorList.indexOf(color); | 205 | for (int i = 0; i < colorList.count(); ++i) { | ||
209 | if (i >= 0) { | 206 | if (colorList[i].second == color) { | ||
210 | q->setCurrentIndex(i + 1); | 207 | q->setCurrentIndex(i + 1); | ||
211 | internalcolor = color; | 208 | internalcolor = color; | ||
212 | return; | 209 | return; | ||
213 | } | 210 | } | ||
214 | } | 211 | } | ||
215 | } | 212 | } | ||
213 | } | ||||
216 | 214 | | |||
217 | internalcolor = color; | 215 | internalcolor = color; | ||
218 | customColor = color; | 216 | customColor = color; | ||
219 | q->setItemData(0, customColor, KColorComboDelegate::ColorRole); | 217 | q->setItemData(0, customColor, KColorComboDelegate::ColorRole); | ||
220 | } | 218 | } | ||
221 | 219 | | |||
222 | KColorCombo::KColorCombo(QWidget *parent) | 220 | KColorCombo::KColorCombo(QWidget *parent) | ||
223 | : QComboBox(parent), d(new KColorComboPrivate(this)) | 221 | : QComboBox(parent), d(new KColorComboPrivate(this)) | ||
Show All 13 Lines | |||||
237 | 235 | | |||
238 | KColorCombo::~KColorCombo() | 236 | KColorCombo::~KColorCombo() | ||
239 | { | 237 | { | ||
240 | delete d; | 238 | delete d; | ||
241 | } | 239 | } | ||
242 | 240 | | |||
243 | void KColorCombo::setColors(const QList<QColor> &colors) | 241 | void KColorCombo::setColors(const QList<QColor> &colors) | ||
244 | { | 242 | { | ||
243 | ColorList namedColors; | ||||
tcanabrava: namedColors.reserve(colors.size()); | |||||
araujoluis: Done! | |||||
244 | namedColors.reserve(colors.size()); | ||||
tcanabrava: for(auto color : colors) | |||||
araujoluis: Done! | |||||
245 | for (auto color : colors) { | ||||
auto& as we just want a reference, avoids a copy constructor call each time. kossebau: `auto& ` as we just want a reference, avoids a copy constructor call each time. | |||||
246 | namedColors.append({QString(), color}); | ||||
247 | } | ||||
248 | setNamedColors(namedColors); | ||||
249 | } | ||||
250 | | ||||
251 | void KColorCombo::setNamedColors(const ColorList &colors) | ||||
252 | { | ||||
245 | clear(); | 253 | clear(); | ||
246 | d->colorList = colors; | 254 | d->colorList = colors; | ||
247 | d->addColors(); | 255 | d->addColors(); | ||
248 | } | 256 | } | ||
249 | 257 | | |||
258 | void KColorCombo::insertNamedColor(int index, const QPair<QString, QColor> &namedColor) | ||||
259 | { | ||||
260 | ColorList namedColorList = namedColors(); | ||||
261 | namedColorList.insert(index, namedColor); | ||||
262 | setNamedColors(namedColorList); | ||||
263 | } | ||||
264 | | ||||
250 | QList<QColor> KColorCombo::colors() const | 265 | QList<QColor> KColorCombo::colors() const | ||
251 | { | 266 | { | ||
252 | if (d->colorList.isEmpty()) { | | |||
253 | QList<QColor> list; | 267 | QList<QColor> list; | ||
268 | if (d->colorList.isEmpty()) { | ||||
254 | list.reserve(STANDARD_PALETTE_SIZE); | 269 | list.reserve(STANDARD_PALETTE_SIZE); | ||
255 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | 270 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | ||
256 | list += standardColor(i); | 271 | list += standardColor(i); | ||
257 | } | 272 | } | ||
258 | return list; | 273 | return list; | ||
259 | } else { | 274 | } else { | ||
tcanabrava: remove the else, just return directly. | |||||
260 | return d->colorList; | 275 | list.reserve(d->colorList.size()); | ||
276 | for (auto iColor : d->colorList) { | ||||
cfeck: Variables declared in 'for' are local, so just use 'color'. | |||||
araujoluis: Done! | |||||
auto& as we just want a reference, avoids a copy constructor call each time. kossebau: `auto& ` as we just want a reference, avoids a copy constructor call each time. | |||||
277 | list.append({iColor.second}); | ||||
278 | } | ||||
261 | } | 279 | } | ||
280 | return list; | ||||
281 | } | ||||
282 | | ||||
283 | QList<QPair<QString, QColor>> KColorCombo::namedColors() const | ||||
284 | { | ||||
285 | if (d->colorList.isEmpty()) { | ||||
286 | ColorList list; | ||||
287 | for (int index = 0; index < STANDARD_PALETTE_SIZE; ++index) { | ||||
kossebau: list.reserve(STANDARD_PALETTE_SIZE); | |||||
288 | list += {QString(), standardColor(index)}; | ||||
tcanabrava: why is there a for running with all code comented out? | |||||
araujoluis: Done! | |||||
289 | } | ||||
290 | return list; | ||||
291 | } | ||||
292 | return d->colorList; | ||||
262 | } | 293 | } | ||
263 | 294 | | |||
264 | void KColorCombo::setColor(const QColor &col) | 295 | void KColorCombo::setColor(const QColor &col) | ||
265 | { | 296 | { | ||
266 | if (!col.isValid()) { | 297 | if (!col.isValid()) { | ||
267 | return; | 298 | return; | ||
268 | } | 299 | } | ||
269 | 300 | | |||
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Line(s) | 342 | if (index == 0) { | |||
312 | QColor c = QColorDialog::getColor(customColor, q); | 343 | QColor c = QColorDialog::getColor(customColor, q); | ||
313 | if (c.isValid()) { | 344 | if (c.isValid()) { | ||
314 | customColor = c; | 345 | customColor = c; | ||
315 | setCustomColor(customColor, false); | 346 | setCustomColor(customColor, false); | ||
316 | } | 347 | } | ||
317 | } else if (colorList.isEmpty()) { | 348 | } else if (colorList.isEmpty()) { | ||
318 | internalcolor = standardColor(index - 1); | 349 | internalcolor = standardColor(index - 1); | ||
319 | } else { | 350 | } else { | ||
320 | internalcolor = colorList[index - 1]; | 351 | internalcolor = colorList[index - 1].second; | ||
321 | } | 352 | } | ||
322 | 353 | | |||
323 | emit q->activated(internalcolor); | 354 | emit q->activated(internalcolor); | ||
324 | } | 355 | } | ||
325 | 356 | | |||
326 | void KColorComboPrivate::_k_slotHighlighted(int index) | 357 | void KColorComboPrivate::_k_slotHighlighted(int index) | ||
327 | { | 358 | { | ||
328 | if (index == 0) { | 359 | if (index == 0) { | ||
329 | internalcolor = customColor; | 360 | internalcolor = customColor; | ||
330 | } else if (colorList.isEmpty()) { | 361 | } else if (colorList.isEmpty()) { | ||
331 | internalcolor = standardColor(index - 1); | 362 | internalcolor = standardColor(index - 1); | ||
332 | } else { | 363 | } else { | ||
333 | internalcolor = colorList[index - 1]; | 364 | internalcolor = colorList[index - 1].second; | ||
334 | } | 365 | } | ||
335 | 366 | | |||
336 | emit q->highlighted(internalcolor); | 367 | emit q->highlighted(internalcolor); | ||
337 | } | 368 | } | ||
338 | 369 | | |||
339 | void KColorComboPrivate::addColors() | 370 | void KColorComboPrivate::addColors() | ||
340 | { | 371 | { | ||
341 | q->addItem(KColorCombo::tr("Custom...", "Custom color")); | 372 | q->addItem(KColorCombo::tr("Custom...", "Custom color")); | ||
342 | | ||||
343 | if (colorList.isEmpty()) { | 373 | if (colorList.isEmpty()) { | ||
344 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | 374 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | ||
345 | q->addItem(QString()); | 375 | q->addItem(QString()); | ||
346 | q->setItemData(i + 1, standardColor(i), KColorComboDelegate::ColorRole); | 376 | q->setItemData(i + 1, standardColor(i), KColorComboDelegate::ColorRole); | ||
347 | } | 377 | } | ||
348 | } else { | 378 | } else { | ||
349 | for (int i = 0, count = colorList.count(); i < count; ++i) { | 379 | for (int i = 0, count = colorList.count(); i < count; ++i) { | ||
350 | q->addItem(QString()); | 380 | q->addItem(QString()); | ||
351 | q->setItemData(i + 1, colorList[i], KColorComboDelegate::ColorRole); | 381 | q->setItemData(i + 1, colorList[i].second, KColorComboDelegate::ColorRole); | ||
382 | if (!colorList[i].first.isEmpty()) { | ||||
383 | q->setItemData(i + 1, colorList[i].first, Qt::DisplayRole); | ||||
384 | } | ||||
352 | } | 385 | } | ||
353 | } | 386 | } | ||
354 | } | 387 | } | ||
355 | 388 | | |||
356 | #include "moc_kcolorcombo.cpp" | 389 | #include "moc_kcolorcombo.cpp" | ||
357 | #include "kcolorcombo.moc" | 390 | #include "kcolorcombo.moc" |
Missing const, also the name should be innerColor with a capital C if we are following the code style from this file.