New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove doubling "None" in effect selection #2176
Conversation
src/preferences/dialog/dlgprefeq.cpp
Outdated
@@ -231,10 +231,10 @@ void DlgPrefEQ::slotPopulateDeckEffectSelectors() { | |||
} | |||
//: Displayed when no effect is selected | |||
box->addItem(tr("None"), QVariant(QString(""))); | |||
if (selectedEffectId.isNull()) { | |||
if (selectedEffectId.isNull() || QString::compare(selectedEffectName, tr("None")) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please prefer to use operator ==
if you don't need the ordering capabilities of compare()
src/preferences/dialog/dlgprefeq.cpp
Outdated
currentIndex = availableQuickEffects.size(); // selects "None" | ||
} | ||
if (currentIndex < 0 && !selectedEffectName.isEmpty()) { | ||
else if (currentIndex < 0 && !selectedEffectName.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow our coding guidelines by placing the else on the same line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just didn't see them beforehand. I will keep it in mind and re-commit.
I don't really like the string compare, wouldn't the following diff work as well?
|
I will give it a shot, but the code would probably not work as expected. As far as I can tell the selectedEffectId will not be empty unless no entry of the was ever selected. Probably only after fresh install or something. |
I think |
@rrrapha thanks for the hint, the isEmpty() does the job. I didn't recognize that the effectid is set to an empty string. |
Cool, it looks good and works for me :) |
LGTM. Thank you. |
Remove the doubling "None" in the preferences -> equalizer -> effect list when "None" is already selected.
Fixes https://bugs.launchpad.net/mixxx/+bug/1824624.