Fix track color library inline editing #2517
Conversation
| @@ -723,6 +723,10 @@ QVariant BaseSqlTableModel::data(const QModelIndex& index, int role) const { | |||
| // role | |||
| switch (role) { | |||
| case Qt::ToolTipRole: | |||
| if (column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COLOR)) { | |||
There was a problem hiding this comment.
Wouldn't QVariant() work be more appropriate here?
There was a problem hiding this comment.
This works for me:
const auto color = mixxx::RgbColor::optional(value);
if (color) {
value = mixxx::toQColor(color).name();
} else {
value = QString();
}
There was a problem hiding this comment.
Maybe add a reusable, free function to rgbcolor.h:
QString nullableName(mixxx::RgbColor::optional_t color);
Please note that QColor().name() seems to return "#000000", i.e. black although undefined/invalid/null. So we should test this new function to prevent regressions if someone thinks the check is redundant!
Not sure about the naming, but at least it is concise and easy to remember.
There was a problem hiding this comment.
Do you want me to do this as part of this PR?
There was a problem hiding this comment.
Hold on. Let's first merge this PR and I will publish a separate PR then.
Holzhaus
force-pushed
the
trackcolor-fix
branch
from
6059399
to
6a20ac2
Compare
Holzhaus
force-pushed
the
trackcolor-fix
branch
from
6a20ac2
to
78a7855
Compare
|
@Holzhaus GitHub doesn't offer me the option to open a PR for you repo: |
| value = QString(); | ||
| } | ||
| } | ||
| [[fallthrough]]; |
There was a problem hiding this comment.
This causes unfortunately a warning.
Please use M_FALLTHROUGH_INTENDED
Fixes the issue reported here: #2470 (comment)