In D3579#156382, @Campbell Barton (campbellbarton) wrote:

In D3579#156377, @Rudolf Ortner (rudolf.ortner) wrote:

In D3579#156372, @Campbell Barton (campbellbarton) wrote:

Regarding use case of setting colors, it seems other applications use a palette of categories, which seems more useful then manually setting RGB for for every strip

Like what we have for bones.

Linking from UI task.

That's why I wanted to add presets in the first place. So that the user can quickly assign predefined colors..
I am currently not familiar with Bones and what you actually mean?

See the preferences -> Theme -> Bone Color Sets,

This wouldn't have to have colors for each state (selected, active), it's just as example of a predefined color palette.

In D3579#156372, @Campbell Barton (campbellbarton) wrote:

Regarding use case of setting colors, it seems other applications use a palette of categories, which seems more useful then manually setting RGB for for every strip

Like what we have for bones.

Linking from UI task.

Removed preset header
Panel renaming

In D3579#156290, @Sybren A. Stüvel (sybren) wrote:

In D3579#156272, @Rudolf Ortner (rudolf.ortner) wrote:

I don't care anymore... It was on right-click-select and I thought it would be a good start to get into programming for Blender...

It still is!

But simply doing it so as it is somewhere else in Blender also does not fullwill the requirements.. (node color also is float)

Every developer will get feedback on their patches. There are always things you don't think about that others do see, that's what the review process is all about. The original developer of the patch and the reviewers work as a team, and keep going back & forth with improvements and tweaks until the patch is of such quality that it can go into Blender. Nobody is scolding you for doing the wrong thing; nobody is expecting you to do the right thing from the get-go. Any feedback remarks you get are about that particular bit of code, and do not reflect on you personally.

I think what the most important question here is: what is the reason VSE strips need a custom colour? What is the intended workflow? You said you were on RightClickSelect before; I can imagine that this feature was discussed there. It would be useful to have a link to that discussion.

Your patch has been sitting still for quite a while, sorry for that. Recently I made a list of Ingredients of a Patch that might be useful for you to read through. It'll help make future patch review sessions a bit smoother/faster/nicer.

In D3579#156268, @Campbell Barton (campbellbarton) wrote:

Is this common functionality in video editors? We can of course add custom colors all over (many other places too), but would avoid unless it's really needed.

If video previews are added this may be less useful too.

I don't want to seem mad.. but I think that there are a lot of discrepancies between what developers say and what is actually in the Blender code... (makes it very hard for new developers to do the smallest things..)
To be correct, yes, the default values when the Strip is created should be set somewhere else, i did not knew exactly where...
But in the presets of tracking marker colors, there is also a default preset and that is why I added it here too...

In D3579#155746, @Peter Fog (tintwotin) wrote:

Most functions in the Strip tab in the Sidebar will change values in the full selection if +Alt is pressed or by using 'Copy to Selected' in the right click menu:

Maybe this would be a useful way to batch change colors on multiple strips?

In D3579#155706, @Peter Fog (tintwotin) wrote:

Works as advertised. Very nice with the presets, too.

I don't know if the color presets should have a commit of it's own, and then add it to all the color elements in the VSE, like the text color, the color strips, and several of the modifiers too. But maybe @Richard Antalik (ISS) can let you know of it is okay to include it in this patch?

On the UI, maybe the panel could be called "Strip Color" to avoid confusion with other color elements in the Sidebar? Or maybe the color and preset button could be added next to the strip name/string entry box on top, so all the visual strip elements are collected in the same line? What do @William Reynish (billreynish) say?

If you, at some point, want to include some preset colors, maybe it would be best to avoid the colors already used for the various strip types to avoid confusion?

If the argument comes up, that changing the strip color will make it difficult to know what strip type it is, then an option could be to add a checkbox for limiting the vertical drawing to a line in the top or bottom of the strip, so both the original strip color and the new color is visible?

For reference, this is the current UI:

Updated the diff to Blender 2.8
includes advantage of using presets

In D3579#155097, @Eitan Traurig (EitanSomething) wrote:

This looks very useful.I will review the patch and see if the code needs to be updated.

In D3709#144440, @Jeroen Bakker (jbakker) wrote:

This is a very long outstanding code review, but still think it is useful to have.
If you don't want to work on this patch I can take over.