Skip to content
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 auto hotcue colors option #2578

Closed

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Mar 20, 2020

This option is now obsolete since the same can be archieved using the palette editor.

I also added a second Mixxx Hotcue Palette that includes the orange default color and always assigns that to all new hotcues.

@Holzhaus Holzhaus added the ui label Mar 20, 2020
@Holzhaus Holzhaus added this to the 2.3.0 milestone Mar 20, 2020
@Holzhaus Holzhaus requested a review from daschuer March 20, 2020 18:05
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep the orange default for all new cues after update.
The cue color by number option should be explicit selected, because it has the potential to mess the color up for cues set on the fly.

kSchemaMigrationReplacementColor,
},
QList<unsigned int>{8});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the use cases for kMixxxHotcueColorPaletteWithDefaultColor and kMixxxHotcueColorPalette?
How about adding orange to the kMixxxHotcueColorPalette?

@Holzhaus
Copy link
Member Author

I think we should keep the orange default for all new cues after update.

We could make the "Mixxx Palette with default color" palette the default palette, but in my opinion we should enable auto-colored hotcue by default so that users that don't open the settings profit from colored cues right away.

The cue color by number option should be explicit selected, because it has the potential to mess the color up for cues set on the fly.

I'm not sure what you mean. Do you want to keep the "auto_hotcue_colors" option? We should definitely remove it. It's redundant and inflexible because you can't configure the default color and you can just use the palette editor to configure a single default color.
Can you elaborate what you mean by "mess up cue colors set on the fly"?

@daschuer
Copy link
Member

We could make the "Mixxx Palette with default color" palette the default palette,

OK.

but in my opinion we should enable auto-colored hotcue by default so that users that don't open the settings profit from colored cues right away.

I do not agree that this is a good feature to color the cues by numbers but this is only a opinion from one user, the same as yours.
In Mixxx we normally do not change the preferences for the user. The important fact is that we migrate the 2.2 cues to a single color, so we should not change this silently.

I would like to add this:
Default cue color: Combo box with colored square icon:

  • current default (orange after migations = Palette color n)
  • Palette color 1
  • Palette color 2
    ..
  • Palette color n
  • By hotcue number

This way the expensive to tweak cue number assignment in the palette only need to b adjusted in exceptional cases.

Can you elaborate what you mean by "mess up cue colors set on the fly"?

That would be part of my personal experience. I can set the cue point and it will have most likely the wrong color and I am forced to adjust the color immediately because later I can't distinguish that the color was not adjusted. Using a single default color allows to adjust the cue point later at home.

@daschuer
Copy link
Member

I can add this combobox to #2589 and the additions of orange to the mixxx palette.

@Holzhaus
Copy link
Member Author

Holzhaus commented Mar 23, 2020

I do not agree that this is a good feature to color the cues by numbers but this is only a opinion from one user, the same as yours.

  • When a user doesn't care about cue colors none of these two options has an advantage over the other.
  • When a user cares about cue colors:
    • Without automatic hotcue colors all cues have the same, default color -> all new cues need to be colored manually
    • With automatic hotcue colors and the user sets a new hotcue:
      • Best case: The auto-selected color matches the color scheme that the user prefers, no further interaction needed. Users that just get started with cue colors might even choose a color scheme based on this auto-selection (e.g. always assign the main drop cue point to hotcue 3, so that it will be colored green) -> no manual coloring needed
      • Worst case: The auto-selected color does not match the user's color scheme -> the new cue needs to be colored manually

So basically with automatic hotcue colors it is possible that the user sometimes needs to set the color manually, when if we disable it we'll always hit the worst case and require manual intervention. Hence, I think enabling it by default is the obvious choice.

In Mixxx we normally do not change the preferences for the user.

Using a single color for all cues wasn't a user preference, it was just cue colors didn't exist in Mixxx 2.2. We cannot infer that users actually preferred it, there was simply no alternative. If we stretch the definition of "user preference" and consider this as such, we couldn't add any new features to Mixxx without disabling them by default (which we currently don't).

The important fact is that we migrate the 2.2 cues to a single color, so we should not change this silently.

We don't change this silently, all legacy cues still have a single color. This only applies to new cues. Users can instantly see that their new cues have different colors. They can still set a different color manually or go to the preferences and switch to a single default color.

I would like to add this:
Default cue color: Combo box with colored square icon [...]

I agree that there needs to be an easier way to set a single default color, but let's discuss this over at #2589.

Can you elaborate what you mean by "mess up cue colors set on the fly"?

That would be part of my personal experience. I can set the cue point and it will have most likely the wrong color and I am forced to adjust the color immediately because later I can't distinguish that the color was not adjusted. Using a single default color allows to adjust the cue point later at home.

I think this is actually a very special use case and I'm not quite sure why you would want to take the extra step of sitting down after a gig and color all cues manually.

I think we agree that the most frictionless way to set cues is when you don't need to set the color manually at all. Let's consider a possible user's color scheme:

Position Color
After intro Red
Before drop Green
After Drop Sky Blue
Before main vocals Blue
Before outro Pink

With automatic hotcue colors, it's actually possible to use that scheme without manual coloring. If you want to set the cue point before the main vocal part, just press hotcue 5 (which will be colored blue automatically) -> done. No manual coloring needed.
That also means that a hotcue of the same type (e.g. "before main vocals") does not only have the same color but is also assigned to the same pad, so the possibility of getting confused during a gig is reduced even more than when just using colors.

If you have another color scheme, you can use the palette editor to use different colors and adjust the mapping between colors and hotcue indices.
Chances are that most Mixxx users don't have a preferred cue color scheme yet (because Mixxx didn't support colors), so I'm certain that the number of users that do use the palette editor would be small.

If you still like to assign a color to each new cue manually, you can just configure a single default color for all new cues in the preferences, but that should not be the default for the reasons I explained above.

Also, setting a color automatically makes the colored cue feature much more discoverable IMHO. It's also what Serato does by default, but it doesn't even provide a way to modify the default color(s) at all. Automatic colors in combination with the ability to configure the default colors really puts Mixxx ahead of our largest competitor, so we should make it the default.

@daschuer
Copy link
Member

Understand how the described approach can be a benefit. However this requires an certain degree of conference how to use the colors before starting it.
Since we do not bother the user with it in 2.2 he should not be bothers with this after upgrade, during his first Gig. It should be an intentional decision to start with cue colors.

With default orange, the user can clearly destingush between legacy cues and cues created after he has introduced a color concept and switched the auto coloring on.
It is not the time when he upgraded to 2.3.

Please remember that I originally have preferred to keep no-color for new cues. It was a kind of compromise to select orange for all uncolored cues. I see no reason to give this up and watering this compromise.

It is just one preferences option and a real small act compared to the mass it will create if Mixxx auto colors the cues without a concept.

So please let's stick with it.

@Holzhaus
Copy link
Member Author

Holzhaus commented Mar 23, 2020

Alright, this discussion got a bit off-topic. I'm still in favor of coloring cues in different colors by default, at least for new users (i.e. users that didn't come from 2.2). But if everyone else thinks a single default color for all new cues is the best choice, I won't insist.

In any case, we should remove the auto_hotcue_colors and always use the palette's hotcue indices. We can set the mixxx palette with orange default color as default palette and after #2589 has been merged I'll open a PR that adds an easier way to pick a default color to the preferences. When the user picks a single default hotcue color, the preferences dialog creates a palette with with the hotcue index set for that color automatically. That way we don't need to consider multiple cases when setting cues, and keep the complexity introduced by that configuration option in the color prefences dialog.

@daschuer
Copy link
Member

Can't we get did of the cue number assignment column in the GUI and just use it one by one?
I think it is confusing to use the first color in the color picker for cue 2 and the second for cue 1.

I am still convinced that the default color selector and the palette editor should be two separate things.

What stands against the proposed combo box?

@Holzhaus
Copy link
Member Author

It's very exhausting when mixing so many different topics that all have not that much to do with this specific PR. This discussion gets really OT now.

Can't we get did of the cue number assignment column in the GUI and just use it one by one?

No, this would greatly limit the flexibility. In fact, the current config UI does not even offer the full potential of the approach, because we could also assign the first color to the cues 1 and 2, the second color to cues 3 and 4, etc...

I want to use a gradient-like Hotcue Palette but use distinct hotcue colors for cues next to each other. This isn't possible when the palette indices is the same as the default hotcue color indices.

We could remove the "Assign to Hotcue Number" column from the palette editor and use a second tableview where you drag an drop colors from the palette if you want to implement that. I posted a GUI mockup in last December on zulip, but opted for the single table approach in #2530 because it was easier to implement:
2019-12-16-225208_977x685_scrot

I think it is confusing to use the first color in the color picker for cue 2 and the second for cue 1.

Then don't configure it like that. Mixxx won't do that unless you open the palette editor and tell it to behave like that.

I am still convinced that the default color selector and the palette editor should be two separate things.

The default color(s) should be part of the palette, so it makes sense to edit and save them together.

What stands against the proposed combo box?

I'm not against a combobox where you can select a single default color. I'm just saying that this combobox should update the palette instead of introducing another way of configuring hotcue colors. We shouldn't have to consider multiple different options we assign a color a to cue. Let's just use palette.colorForHotcueIndex(i) instead of if-else-ladders.

@daschuer
Copy link
Member

I'm not against a combobox where you can select a single default color.

OK, so I will add it to #2589 and you can here make it use the colorForHotcueIndex() function.

OK ?

The GUI Mock up looks good.
I am only concerned that this will delay 2.3 even more.

IMHO the only mandatory topic is to allow mass changing the default color. the rest can go to 2.4.
What do you think?

@Be-ing
Copy link
Contributor

Be-ing commented Mar 24, 2020

The approach taken in this PR to add a new palette with a 9th color that doesn't get assigned to a hotcue or shown in the cue menu to use as the single default color is bizarre. I think this is a hack around the underlying problem that the palette shown in the cue menu and the palette of default hotcue colors are coupled. (This is the hard part where I stopped working on #2399.) It seems the decoupling of hotcue numbers from the palette position (the second column in the palette editor table) is another hack around this.

I doubt we should show the user two different palettes to configure for the default hotcue colors and the cue menu. I propose making the existence of different palettes an implementation detail. In the preferences GUI, add a radio button to choose between all hotcues one color and a different color per hotcue. If all hotcues one color is selected, the preferences dialog could generate a palette with only the single color which would be used by CueControl to assign the default color.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 24, 2020

I doubt we should show the user two different palettes to configure for the default hotcue colors and the cue menu.

On second thought, what's the point of manually setting the color of a hotcue if your only choices are the colors that are automatically assigned to other hotcues? If we decide to allow this, I'd expect to edit the colors in the cue menu directly from the cue menu, not have to go to the preferences window for that.

@Holzhaus
Copy link
Member Author

The approach taken in this PR to add a new palette with a 9th color that doesn't get assigned to a hotcue or shown in the cue menu to use as the single default color is bizarre.

It does get added to that new palette and it will also be shown in the cue menu.

It seems the decoupling of hotcue numbers from the palette position (the second column in the palette editor table) is another hack around this.

No, that solves multiple other problems like the ability to have more options in the cue menu's color picker than just the colors that get assigned automatically.

In the preferences GUI, add a radio button to choose between all hotcues one color and a different color per hotcue. If all hotcues one color is selected, the preferences dialog could generate a palette with only the single color which would be used by CueControl to assign the default color.

That makes no sense. If we make a palette with only the single default color then the cue menu would also contain just that single default color, so you can not even change color of a hotcue.
It also makes it impossible to customize how default colors are assigned.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 24, 2020

It does get added to that new palette and it will also be shown in the cue menu.

Sorry, I made an incorrect assumption. I should have tested that.

I propose we close this and go with the approach in #2589 instead.

@ywwg ywwg added the blocker label Mar 24, 2020
@Be-ing
Copy link
Contributor

Be-ing commented Apr 5, 2020

Okay #2589 is merged and this has merge conflicts now. Can we close this and remove the "beta blocker" tag from it?

@Holzhaus
Copy link
Member Author

Holzhaus commented Apr 5, 2020

I experimented a bit and creating palettes on the fly can be confusing after all. I'll close this for now. Maybe I'll look into it for 2.4, but the current solution is probably good enough.

@Holzhaus Holzhaus closed this Apr 5, 2020
@Holzhaus Holzhaus removed the blocker label Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants