Changeset View
Changeset View
Standalone View
Standalone View
src/kcolorcombo.cpp
Show First 20 Lines • Show All 70 Lines • ▼ Show 20 Line(s) | 70 | { | |||
---|---|---|---|---|---|
71 | // background | 71 | // background | ||
72 | QColor innercolor(Qt::white); | 72 | QColor innercolor(Qt::white); | ||
73 | bool isSelected = (option.state & QStyle::State_Selected); | 73 | bool isSelected = (option.state & QStyle::State_Selected); | ||
74 | bool paletteBrush = (k_colorcombodelegate_brush(index, Qt::BackgroundRole).style() == Qt::NoBrush); | 74 | bool paletteBrush = (k_colorcombodelegate_brush(index, Qt::BackgroundRole).style() == Qt::NoBrush); | ||
75 | if (isSelected) { | 75 | if (isSelected) { | ||
76 | innercolor = option.palette.color(QPalette::Highlight); | 76 | innercolor = option.palette.color(QPalette::Highlight); | ||
77 | } else { | 77 | } else { | ||
78 | innercolor = option.palette.color(QPalette::Base); | 78 | innercolor = option.palette.color(QPalette::Base); | ||
79 | } | 79 | } | ||
patrickelectric: Missing const, also the name should be `innerColor` with a capital C if we are following the… | |||||
araujoluis: Done! | |||||
80 | // highlight selected item | 80 | // highlight selected item | ||
81 | QStyleOptionViewItem opt(option); | 81 | QStyleOptionViewItem opt(option); | ||
82 | opt.showDecorationSelected = true; | 82 | opt.showDecorationSelected = true; | ||
83 | QStyle *style = opt.widget ? opt.widget->style() : QApplication::style(); | 83 | QStyle *style = opt.widget ? opt.widget->style() : QApplication::style(); | ||
84 | style->drawPrimitive(QStyle::PE_PanelItemViewItem, &opt, painter, opt.widget); | 84 | style->drawPrimitive(QStyle::PE_PanelItemViewItem, &opt, painter, opt.widget); | ||
85 | QRect innerrect = option.rect.adjusted(FrameMargin, FrameMargin, -FrameMargin, -FrameMargin); | 85 | QRect innerrect = option.rect.adjusted(FrameMargin, FrameMargin, -FrameMargin, -FrameMargin); | ||
86 | // inner color | 86 | // inner color | ||
87 | QVariant cv = index.data(ColorRole); | 87 | QVariant cv = index.data(ColorRole); | ||
patrickelectric: missing reference for innercolor ? | |||||
araujoluis: Yes, reference not found for innercolor or innerColor | |||||
patrickelectric: Missing const. | |||||
88 | if (cv.type() == QVariant::Color) { | 88 | if (cv.type() == QVariant::Color) { | ||
patrickelectric: missing const. | |||||
89 | QColor tmpcolor = cv.value<QColor>(); | 89 | QColor tmpcolor = cv.value<QColor>(); | ||
90 | if (tmpcolor.isValid()) { | 90 | if (tmpcolor.isValid()) { | ||
91 | innercolor = tmpcolor; | 91 | innercolor = tmpcolor; | ||
92 | paletteBrush = false; | 92 | paletteBrush = false; | ||
93 | painter->setPen(Qt::transparent); | 93 | painter->setPen(Qt::transparent); | ||
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! | |||||
94 | painter->setBrush(innercolor); | 94 | painter->setBrush(innercolor); | ||
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! | |||||
95 | QPainter::RenderHints tmpHint = painter->renderHints(); | 95 | QPainter::RenderHints tmpHint = painter->renderHints(); | ||
96 | painter->setRenderHint(QPainter::Antialiasing); | 96 | painter->setRenderHint(QPainter::Antialiasing); | ||
97 | painter->drawRoundedRect(innerrect, 2, 2); | 97 | painter->drawRoundedRect(innerrect, 2, 2); | ||
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); | 98 | painter->setRenderHints(tmpHint); | ||
99 | painter->setBrush(Qt::NoBrush); | 99 | painter->setBrush(Qt::NoBrush); | ||
100 | } | 100 | } | ||
101 | } | 101 | } | ||
102 | // text | 102 | // text | ||
103 | QVariant tv = index.data(Qt::DisplayRole); | 103 | QVariant tv = index.data(Qt::DisplayRole); | ||
104 | if (tv.type() == QVariant::String) { | 104 | if (tv.type() == QVariant::String) { | ||
105 | QString text = tv.toString(); | 105 | QString text = tv.toString(); | ||
▲ Show 20 Lines • Show All 72 Lines • ▼ Show 20 Line(s) | 175 | public: | |||
178 | void addColors(); | 178 | void addColors(); | ||
179 | void setCustomColor(const QColor &color, bool lookupInPresets = true); | 179 | void setCustomColor(const QColor &color, bool lookupInPresets = true); | ||
180 | 180 | | |||
181 | // slots | 181 | // slots | ||
182 | void _k_slotActivated(int index); | 182 | void _k_slotActivated(int index); | ||
183 | void _k_slotHighlighted(int index); | 183 | void _k_slotHighlighted(int index); | ||
184 | 184 | | |||
185 | KColorCombo *q; | 185 | KColorCombo *q; | ||
186 | QList<QColor> colorList; | 186 | QList<KColorCombo::KNamedColor> colorList; | ||
187 | QColor customColor; | 187 | QColor customColor; | ||
188 | QColor internalcolor; | 188 | QColor internalcolor; | ||
189 | }; | 189 | }; | ||
190 | 190 | | |||
191 | KColorComboPrivate::KColorComboPrivate(KColorCombo *qq) | 191 | KColorComboPrivate::KColorComboPrivate(KColorCombo *qq) | ||
192 | : q(qq), customColor(Qt::white) | 192 | : q(qq), customColor(Qt::white) | ||
193 | { | 193 | { | ||
194 | } | 194 | } | ||
195 | 195 | | |||
196 | void KColorComboPrivate::setCustomColor(const QColor &color, bool lookupInPresets) | 196 | void KColorComboPrivate::setCustomColor(const QColor &color, bool lookupInPresets) | ||
197 | { | 197 | { | ||
198 | if (lookupInPresets) { | 198 | if (lookupInPresets) { | ||
199 | if (colorList.isEmpty()) { | 199 | if (colorList.isEmpty()) { | ||
200 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | 200 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | ||
201 | if (standardColor(i) == color) { | 201 | if (standardColor(i) == color) { | ||
202 | q->setCurrentIndex(i + 1); | 202 | q->setCurrentIndex(i + 1); | ||
203 | internalcolor = color; | 203 | internalcolor = color; | ||
204 | return; | 204 | return; | ||
205 | } | 205 | } | ||
206 | } | 206 | } | ||
207 | } else { | 207 | } else { | ||
208 | int i = colorList.indexOf(color); | 208 | for (int i = 0; i < colorList.count(); ++i) { | ||
209 | if (i >= 0) { | 209 | if (colorList[i].color == color) { | ||
210 | q->setCurrentIndex(i + 1); | 210 | q->setCurrentIndex(i + 1); | ||
211 | internalcolor = color; | 211 | internalcolor = color; | ||
212 | return; | 212 | return; | ||
213 | } | 213 | } | ||
214 | } | 214 | } | ||
215 | } | 215 | } | ||
216 | } | ||||
216 | 217 | | |||
217 | internalcolor = color; | 218 | internalcolor = color; | ||
218 | customColor = color; | 219 | customColor = color; | ||
219 | q->setItemData(0, customColor, KColorComboDelegate::ColorRole); | 220 | q->setItemData(0, customColor, KColorComboDelegate::ColorRole); | ||
220 | } | 221 | } | ||
221 | 222 | | |||
222 | KColorCombo::KColorCombo(QWidget *parent) | 223 | KColorCombo::KColorCombo(QWidget *parent) | ||
223 | : QComboBox(parent), d(new KColorComboPrivate(this)) | 224 | : QComboBox(parent), d(new KColorComboPrivate(this)) | ||
Show All 13 Lines | |||||
237 | 238 | | |||
238 | KColorCombo::~KColorCombo() | 239 | KColorCombo::~KColorCombo() | ||
239 | { | 240 | { | ||
240 | delete d; | 241 | delete d; | ||
241 | } | 242 | } | ||
242 | 243 | | |||
243 | void KColorCombo::setColors(const QList<QColor> &colors) | 244 | void KColorCombo::setColors(const QList<QColor> &colors) | ||
244 | { | 245 | { | ||
246 | QList<KNamedColor> namedColors; | ||||
tcanabrava: namedColors.reserve(colors.size()); | |||||
araujoluis: Done! | |||||
247 | for (int colorIndex = 0; colorIndex < colors.count(); colorIndex++) { | ||||
tcanabrava: for(auto color : colors) | |||||
araujoluis: Done! | |||||
248 | namedColors.insert(colorIndex, { QString(), colors[colorIndex] }); | ||||
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. | |||||
249 | } | ||||
250 | setNamedColors(namedColors); | ||||
251 | } | ||||
252 | | ||||
253 | void KColorCombo::setNamedColors(const QList<KNamedColor> &colors) | ||||
254 | { | ||||
245 | clear(); | 255 | clear(); | ||
246 | d->colorList = colors; | 256 | d->colorList = colors; | ||
247 | d->addColors(); | 257 | d->addColors(); | ||
248 | } | 258 | } | ||
249 | 259 | | |||
260 | void KColorCombo::insertNamedColor(int index, const KNamedColor &namedColor) | ||||
261 | { | ||||
262 | QList<KNamedColor> namedColorList = namedColors(); | ||||
263 | namedColorList.insert(index, namedColor); | ||||
264 | setNamedColors(namedColorList); | ||||
265 | } | ||||
266 | | ||||
250 | QList<QColor> KColorCombo::colors() const | 267 | QList<QColor> KColorCombo::colors() const | ||
251 | { | 268 | { | ||
252 | if (d->colorList.isEmpty()) { | | |||
253 | QList<QColor> list; | 269 | QList<QColor> list; | ||
254 | list.reserve(STANDARD_PALETTE_SIZE); | 270 | for (int index = 0; index < STANDARD_PALETTE_SIZE; ++index) { | ||
255 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | 271 | list += namedColors()[index].color; | ||
256 | list += standardColor(i); | 272 | } | ||
273 | return list; | ||||
274 | } | ||||
275 | | ||||
276 | QList<KColorCombo::KNamedColor> KColorCombo::namedColors() const | ||||
277 | { | ||||
278 | if (d->colorList.isEmpty()) { | ||||
279 | QList<KNamedColor> list; | ||||
280 | for (int index = 0; index < STANDARD_PALETTE_SIZE; ++index) { | ||||
281 | list += {QString(), standardColor(index)}; | ||||
257 | } | 282 | } | ||
258 | return list; | 283 | return list; | ||
259 | } else { | 284 | } else { | ||
tcanabrava: remove the else, just return directly. | |||||
260 | return d->colorList; | 285 | return d->colorList; | ||
261 | } | 286 | } | ||
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. | |||||
262 | } | 287 | } | ||
263 | 288 | | |||
264 | void KColorCombo::setColor(const QColor &col) | 289 | void KColorCombo::setColor(const QColor &col) | ||
265 | { | 290 | { | ||
266 | if (!col.isValid()) { | 291 | if (!col.isValid()) { | ||
267 | return; | 292 | return; | ||
268 | } | 293 | } | ||
269 | 294 | | |||
270 | if (count() == 0) { | 295 | if (count() == 0) { | ||
271 | d->addColors(); | 296 | d->addColors(); | ||
272 | } | 297 | } | ||
kossebau: list.reserve(STANDARD_PALETTE_SIZE); | |||||
273 | 298 | | |||
tcanabrava: why is there a for running with all code comented out? | |||||
araujoluis: Done! | |||||
274 | d->setCustomColor(col, true); | 299 | d->setCustomColor(col, true); | ||
275 | } | 300 | } | ||
276 | 301 | | |||
277 | QColor KColorCombo::color() const | 302 | QColor KColorCombo::color() const | ||
278 | { | 303 | { | ||
279 | return d->internalcolor; | 304 | return d->internalcolor; | ||
280 | } | 305 | } | ||
281 | 306 | | |||
Show All 30 Lines | 336 | if (index == 0) { | |||
312 | QColor c = QColorDialog::getColor(customColor, q); | 337 | QColor c = QColorDialog::getColor(customColor, q); | ||
313 | if (c.isValid()) { | 338 | if (c.isValid()) { | ||
314 | customColor = c; | 339 | customColor = c; | ||
315 | setCustomColor(customColor, false); | 340 | setCustomColor(customColor, false); | ||
316 | } | 341 | } | ||
317 | } else if (colorList.isEmpty()) { | 342 | } else if (colorList.isEmpty()) { | ||
318 | internalcolor = standardColor(index - 1); | 343 | internalcolor = standardColor(index - 1); | ||
319 | } else { | 344 | } else { | ||
320 | internalcolor = colorList[index - 1]; | 345 | internalcolor = colorList[index - 1].color; | ||
321 | } | 346 | } | ||
322 | 347 | | |||
323 | emit q->activated(internalcolor); | 348 | emit q->activated(internalcolor); | ||
324 | } | 349 | } | ||
325 | 350 | | |||
326 | void KColorComboPrivate::_k_slotHighlighted(int index) | 351 | void KColorComboPrivate::_k_slotHighlighted(int index) | ||
327 | { | 352 | { | ||
328 | if (index == 0) { | 353 | if (index == 0) { | ||
329 | internalcolor = customColor; | 354 | internalcolor = customColor; | ||
330 | } else if (colorList.isEmpty()) { | 355 | } else if (colorList.isEmpty()) { | ||
331 | internalcolor = standardColor(index - 1); | 356 | internalcolor = standardColor(index - 1); | ||
332 | } else { | 357 | } else { | ||
333 | internalcolor = colorList[index - 1]; | 358 | internalcolor = colorList[index - 1].color; | ||
334 | } | 359 | } | ||
335 | 360 | | |||
336 | emit q->highlighted(internalcolor); | 361 | emit q->highlighted(internalcolor); | ||
337 | } | 362 | } | ||
338 | 363 | | |||
339 | void KColorComboPrivate::addColors() | 364 | void KColorComboPrivate::addColors() | ||
340 | { | 365 | { | ||
341 | q->addItem(KColorCombo::tr("Custom...", "Custom color")); | 366 | q->addItem(KColorCombo::tr("Custom...", "Custom color")); | ||
342 | | ||||
343 | if (colorList.isEmpty()) { | 367 | if (colorList.isEmpty()) { | ||
344 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | 368 | for (int i = 0; i < STANDARD_PALETTE_SIZE; ++i) { | ||
345 | q->addItem(QString()); | 369 | q->addItem(QString()); | ||
346 | q->setItemData(i + 1, standardColor(i), KColorComboDelegate::ColorRole); | 370 | q->setItemData(i + 1, standardColor(i), KColorComboDelegate::ColorRole); | ||
347 | } | 371 | } | ||
348 | } else { | 372 | } else { | ||
349 | for (int i = 0, count = colorList.count(); i < count; ++i) { | 373 | for (int i = 0, count = colorList.count(); i < count; ++i) { | ||
350 | q->addItem(QString()); | 374 | q->addItem(QString()); | ||
351 | q->setItemData(i + 1, colorList[i], KColorComboDelegate::ColorRole); | 375 | q->setItemData(i + 1, colorList[i].color, KColorComboDelegate::ColorRole); | ||
376 | q->setItemText(i + 1, colorList[i].colorName); | ||||
352 | } | 377 | } | ||
353 | } | 378 | } | ||
354 | } | 379 | } | ||
355 | 380 | | |||
356 | #include "moc_kcolorcombo.cpp" | 381 | #include "moc_kcolorcombo.cpp" | ||
357 | #include "kcolorcombo.moc" | 382 | #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.