use QColor for cue colors (rebased) #2399
Conversation
COs and the database now hold the color hex int representation instead of a color id.
... and use QColor instead.
and change cue color CO name
Shade still does not work
| : m_colorList(colorList) { | ||
| } | ||
|
|
||
| QColor at(int i) const { |
There was a problem hiding this comment.
Should we replace this class with a typedef for QList<QColor>?
|
Thanks for rebasing |
|
Between #2345 and #2103 (and many before those), the bikeshedding in Mixxx has burnt me out, and likewise for @ferranpujolcamins. We could have had a 2.3 release months ago if those two PRs had not been repeatedly delayed by prolonged discussions of overcomplicated proposals. I tried several times over the past few weeks to continue working on this PR, but I couldn't summon the motivation to work through minor technical challenges, even when I had plenty of free time to do so (which I don't right now). Developers being pushed away is a serious problem for the community. I feel it's long past time we deal with it somehow. I don't know how, maybe by instituting a decision making process. I think we can take that discussion to Zulip; let's keep this PR focused on finishing this feature so we can get 2.3 released already. @ferranpujolcamins told me he would finish this if it would be merged without getting bogged down in endless discussion -- that means letting go of the "no color"/"default color" state idea. All we need to finish this is to implement a way for users to set default colors for hotcues. The Decks preference page seems like the best place to do this, although it still feels somewhat awkward. I propose having a radio button group with 3 choices: I don't think reusing WCueMenuPopup for choosing the colors would be appropriate because there is no cue label to pick here. Maybe WColorPicker could be reused. Alternatively, QColorDialog could be used. I think we need to decide the relationship between these default hotcue colors and the colors available for picking in the main screen through WCueMenuPopup. Currently they are coupled, but I think they should be independent. I am not sure why a user would want to manually set a hotcue to a color which is also used for default cue colors. The colors that are available in WCueMenuPopup are the ones that should be exposed to the controller scripting environment. |
|
I've added a commit (8bce62f) that does not build of the code I had in progress. Feel free to use it or start from the prior commit. |
|
@Be-ing: It is rather unfair to blame our discussion as bikeshedding. If it just where that kind of problem it wouldn't have made you to issue this concurrent PR after I have issued mine. In the AutoDJ case it was also everything else than bikeshedding. It turns out that even after the long review with long discussions and extensive tests serious issues have been slipped in, I had to issue during the last weeks. So it was finally worth all this discussion for the good results we have now in master. Complaining mode off. In this case I think we have found a conclusion how to continue since #2345 (comment) 2019-12-12 So I took my time and have turned #2398 2020-01-05 into a solution this is IMHO not "halve baked" anymore. It fixes hopefully the findings from @uklotzde and does not maintain the "default color" state after the user has selected a default from the current palette. I would be happy to receive some comments to put #2398 in a merge-able state. |
|
I think @ferranpujolcamins is working on PR #2345. I'll close this for now. |
|
Yes, I'll cherry-pick from here if needed. |
This is #2345 rebased on current master to work with the new WCueMenuPopup and WColorPicker widgets from #2362. I squashed 4367fe3 and 85a3762 together to prevent merging commits that do not build.
TODO: