U4-9025 - Add sortable to color picker prevalues

[bjarnef]

Issue: http://issues.umbraco.org/issue/U4-9025

I have added sortable to color picker prevalues, like multivalues prevalue editor.
However it has some issues saving model.value after sorting ... ui-sortable require ngModel, but when I add this, then ui.item.index() returns another value, than if it is not added.

I have previously noticed some bugs in then current version - not sure if this is related to this issue:
angular-ui/ui-sortable#218

I have also updated multivalues prevalue editor, so you can't add same prevalue twice like in color picker prevalues - e.g. if you add "Test 1" and then "Test 1" again you will get an error on save anyway.

[madsrasmussen]

Hi Bjarne,

Thanks for the PR. I have tested your changes and I am not able to get the reorder to work. It looks like the server has reset the sort order when the data comes back after save. Is this something you can look into?

Thanks.

[bjarnef]

Hi Mads

Yes, that was the only thing I was missing. Often when using ui-sortable it is enough to add ng-model on the container where ui-sortable directive is added. I also tried using same sort functionality from multivalues prevalue editor, but didn't seem to work correct.

Even the order (indexes) was correct after sorting, on save it seems to change to order. Not sure if there is some other logic server side on this part?

[madsrasmussen]

I have just tested the ui-sortable and it works fine with the ng-model. It updates the model fine and I think we will be able to remove all the code in the update callback. I think the problem is the way the server handles the preValues and resets the order again. I will have to get some assistance from @Shazwazza because I don’t know the reason why we are doing this.

[bjarnef]

@madsrasmussen @Shazwazza maybe it is this that change order of the array?

Umbraco-CMS/src/Umbraco.Web/PropertyEditors/ColorListPreValueEditor.cs

Line 32 in 5397f2c

var arrayOfVals = dictionary.Select(item => item.Value).ToList();

I think it creates a new list, which is ordered by the value? As I remember I think black (#000000) was always re-ordered to this first item.

[bjarnef]

I had a closer look at this yesterday and compared with multivalues prevalus editor for e.g. dropdownlist where sorting works. I noticed this get its model for "items" updated with id, value and sortorder from serverside.

In ValueListPreValueEditor.cs is order by sortorder, which it doesn't in ColorListPreValueEditor.cs

Umbraco-CMS/src/Umbraco.Web/PropertyEditors/ValueListPreValueEditor.cs

Line 67 in 5397f2c

.OrderBy(x => x.SortOrder)

Furthermore ValueListPreValueEditor.cs use this part:

Umbraco-CMS/src/Umbraco.Web/PropertyEditors/ValueListPreValueEditor.cs

Lines 74 to 82 in 5397f2c

	/// <summary>
	/// Formats the prevalue as a dictionary (as we need to return not just the value, but also the sort-order, to the client)
	/// </summary>
	/// <param name="preValue">The prevalue to format</param>
	/// <returns>Dictionary object containing the prevalue formatted with the field names as keys and the value of those fields as the values</returns>
	private IDictionary<string, object> PreValueAsDictionary(PreValue preValue)
	{
	return new Dictionary<string, object>() { { "value", preValue.Value }, {"sortOrder", preValue.SortOrder } };
	}

In Angular controller for multivalues prevalue it furthermore has:

Umbraco-CMS/src/Umbraco.Web.UI.Client/src/views/prevalueeditors/multivalues.controller.js

Lines 11 to 22 in 5397f2c

	//make an array from the dictionary
	var items = [];
	for (var i in $scope.model.value) {
	items.push({
	value: $scope.model.value[i].value,
	sortOrder: $scope.model.value[i].sortOrder,
	id: i
	});
	}
	//ensure the items are sorted by the provided sort order
	items.sort(function (a, b) { return (a.sortOrder > b.sortOrder) ? 1 : ((b.sortOrder > a.sortOrder) ? -1 : 0); });

Maybe prevalue editor for color picker should be moved to prevalues folder in a major version, so e.g. package developers can use it - or other property editors.

[bjarnef]

I reviewed more of the source code and how the multivalues prevalues works, which are used for dropdown, checkboxlist, radiobuttonlist..

In commit bbf3dff I have updated the model for colors, so it also has a id, value and sortOrder properties.

I have also updated the property editor to use the new model.

It could also set id and sortorder when appending new items to array client side - and before saving to server side - however it doesn't seem to be necessary and the multivalues prevalues doesn't do this either.
To demonstrate this:

Before save:

After save (model updated with sortOrder and id):

[bjarnef]

I am not sure if the logic in ui-sortable update function is necessary if the view has ng-model, which I think is required, otherwise it write info about this in console window.

<div ui-sortable="sortableOptions" ng-model="model.value">
...
</div>

[bjarnef]

After this PR has been solved/merged I can update PR #1503 so it is possible to use color prevalues in grid editor config.

[bjarnef]

@Shazwazza can you check if the server side logic to sort the prevalues is correct?

[bjarnef]

I think this is almost finish. @Shazwazza does the server side code look correct? :)

[zpqrtbnk]

Hey,
I know it's been a looong time... but since your proposed this PR, we have merged another PR which adds support for labels to the color picker. It's not in branch dev-v7. I wanted to review your PR next, but of course it cannot merge easily now. Do you think you would have a moment to review your changes?
Thanks!

[bjarnef]

Hi @zpqrtbnk

I can have a look at it maybe tonight or one of the next day and update the branch with latest changes in dev-v7 branch.

I guess the other feature is part of dev-v7 branch now?
#2245
http://issues.umbraco.org/issue/U4-5322

[bjarnef]

@zpqrtbnk I have updated the branch with latest changed in dev-v7 and resolved the conflicts. However some of my changes is missing and need to be merged with the new label feature.

On server side in ConvertEditorToDb it need to update to SortOrder and in javascript array also have a sortOrder property.

I have also fixed a few other issues:

• Localization of choose and cancel text in color picker
• val-highlight didn't highlight on error, it seems it needs to be wrapped in {{ }} https://github.com/umbraco/Umbraco-CMS/search?utf8=%E2%9C%93&q=val-highlight&type=

I also noticed I can't add two different colors with same Label value?

It might also be worth to make the labels editable, e.g. by showing label in a textbox or toggle between label and text input.

It looks like this at the moment, but needs some changes combined with the new Labels to preserve the sort order.

[nul800sebastiaan]

Hi @bjarnef - I've been trying to figure out how to make this one work, but the ColorListPreValueEditor has changed over time and I can't figure out how to add sorting to it.

Currently there's a few problems: sort order is not sent to the server, when I do send the sort order it's not persisted, when I do persist it (ConvertEditorToDb) then it's not applied (ConvertDbToEditor) when I do apply it the result is in the correct order but... in the browser they're suddenly sorted wrong again even though the response to the browser looks good, so might be something in js going on?

Maybe it would be better to start this one from scratch, what do you think?

FYI I updated ConvertEditorToDb like this to persist the sortOrder:

int.TryParse(x["sortOrder"].ToString(), out var sortOrder);

var value = useLabel
            ? JsonConvert.SerializeObject(new { value = color, label = label, sortOrder = sortOrder.ToString() })
            : color;
return new PreValue(id, value, sortOrder);

And in ConvertDbToEditor I'm retrieving it in the correct order like so:

var dictionary = persistedPreVals.FormatAsDictionary();
var items = dictionary
            .OrderBy(x => x.Value.SortOrder)

[nul800sebastiaan]

Ah crap, now I've messed it up! :-(

I pushed my changes, but that also included the merge so now everything is changed. Yikes. Well, have a look at the current state and then if you could see if you can figure out how to include the sortOrder on the model so it's actually sent to the server and then when the sorted results come back if you can display them in the correct order then we can figure out how to untangle this mess here :/ Sorry about that!

[bjarnef]

@nul800sebastiaan indeed it is a bit of a mess now and Github only shows latest 250 commits, so I can't actually see what I committed two years ago 😬😀

[bjarnef]

@nul800sebastiaan I will close this PR since I have cleaned up this mess and I have opened this new PR instead #2775 🦄