From 2c40e46d56518cad69db6369e49591fc89316312 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Wed, 23 Jan 2019 20:49:08 +0100 Subject: [PATCH 1/8] Set form dirty on color picker value change + update selected item label + clean up unused code --- .../colorpicker/colorpicker.controller.js | 65 +++++++++---------- .../colorpicker/colorpicker.html | 3 +- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js index 9b3be511a4..15d8906ec0 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js @@ -1,4 +1,4 @@ -function ColorPickerController($scope) { +function ColorPickerController($scope, angularHelper) { //setup the default config var config = { @@ -77,29 +77,7 @@ function ColorPickerController($scope) { //now make the editor model the array $scope.model.config.items = items; } - - $scope.toggleItem = function (color) { - - var currentColor = ($scope.model.value && $scope.model.value.hasOwnProperty("value")) - ? $scope.model.value.value - : $scope.model.value; - - var newColor; - if (currentColor === color.value) { - // deselect - $scope.model.value = $scope.model.useLabel ? { value: "", label: "" } : ""; - newColor = ""; - } - else { - // select - $scope.model.value = $scope.model.useLabel ? { value: color.value, label: color.label } : color.value; - newColor = color.value; - } - - // this is required to re-validate - $scope.propertyForm.modelValue.$setViewValue(newColor); - }; - + // Method required by the valPropertyValidator directive (returns true if the property editor has at least one color selected) $scope.validateMandatory = function () { var isValid = !$scope.model.validation.mandatory || ( @@ -115,20 +93,35 @@ function ColorPickerController($scope) { } $scope.isConfigured = $scope.model.config && $scope.model.config.items && _.keys($scope.model.config.items).length > 0; - // A color is active if it matches the value and label of the model. - // If the model doesn't store the label, ignore the label during the comparison. - $scope.isActiveColor = function (color) { - - // no value - if (!$scope.model.value) - return false; - + $scope.onSelect = function (color) { // Complex color (value and label)? - if (!$scope.model.value.hasOwnProperty("value")) - return $scope.model.value === color.value; + if (!$scope.model.value.hasOwnProperty("value")) { + // did the value change? + if ($scope.model.value !== color) { + // yes, make sure to set dirty + angularHelper.getCurrentForm($scope).$setDirty(); + } + return; + } - return $scope.model.value.value === color.value && $scope.model.value.label === color.label; - }; + // did the value change? + if ($scope.model.value.value === color) { + // no, skip the rest + return; + } + + // yes, make sure to set dirty + angularHelper.getCurrentForm($scope).$setDirty(); + + // update the label according to the new color + var selectedItem = _.find($scope.model.config.items, function (item) { + return item.value === color; + }); + if (!selectedItem) { + return; + } + $scope.model.value.label = selectedItem.label; + } // Finds the color best matching the model's color, // and sets the model color to that one. This is useful when diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html index b09d73cee2..0b6e8e7c59 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html @@ -8,7 +8,8 @@ + use-label="model.useLabel" + on-select="onSelect(color)"> From 99f0e6c05a80017e1fb3fdb4b3035ed628309f22 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Wed, 23 Jan 2019 20:54:34 +0100 Subject: [PATCH 2/8] Fix callback --- .../common/directives/components/umbcolorswatches.directive.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js index dc67cb464b..494fae9919 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js @@ -38,7 +38,7 @@ Use this directive to generate color swatches to pick from. scope.selectedColor = color; if (scope.onSelect) { - scope.onSelect(color); + scope.onSelect({color: color }); } }; } From 5637c44f69db6c324fa5cb4806189fb5f39e026f Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 29 Jan 2019 16:49:10 +0100 Subject: [PATCH 3/8] Explicitly set the form dirty when toggling a boolean --- .../src/views/propertyeditors/boolean/boolean.controller.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/boolean/boolean.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/boolean/boolean.controller.js index c574e1424f..07be03da29 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/boolean/boolean.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/boolean/boolean.controller.js @@ -1,4 +1,4 @@ -function booleanEditorController($scope) { +function booleanEditorController($scope, angularHelper) { function setupViewModel() { $scope.renderModel = { @@ -28,7 +28,8 @@ function booleanEditorController($scope) { }; // Update the value when the toggle is clicked - $scope.toggle = function(){ + $scope.toggle = function () { + angularHelper.getCurrentForm($scope).$setDirty(); if($scope.renderModel.value){ $scope.model.value = "0"; setupViewModel(); From 48069201798e95103cb0712beaadf19bc52d7fd5 Mon Sep 17 00:00:00 2001 From: Damiaan Peeters Date: Mon, 28 Jan 2019 15:01:09 +0100 Subject: [PATCH 4/8] Do not allow a empty name with for the MemberGroup --- src/Umbraco.Core/Services/MemberGroupService.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/MemberGroupService.cs b/src/Umbraco.Core/Services/MemberGroupService.cs index 78d8581577..e6b0c5e998 100644 --- a/src/Umbraco.Core/Services/MemberGroupService.cs +++ b/src/Umbraco.Core/Services/MemberGroupService.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using Umbraco.Core.Events; using Umbraco.Core.Logging; @@ -69,6 +70,11 @@ namespace Umbraco.Core.Services public void Save(IMemberGroup memberGroup, bool raiseEvents = true) { + if (string.IsNullOrWhiteSpace(memberGroup.Name)) + { + throw new InvalidOperationException("The name of a MemberGroup can not be empty"); + } + using (var uow = UowProvider.GetUnitOfWork()) { var saveEventArgs = new SaveEventArgs(memberGroup); @@ -133,4 +139,4 @@ namespace Umbraco.Core.Services /// public static event TypedEventHandler> Saved; } -} \ No newline at end of file +} From 7656de693cce4a801c103f845a1e1be56ce1202e Mon Sep 17 00:00:00 2001 From: Damiaan Peeters Date: Mon, 28 Jan 2019 16:14:55 +0100 Subject: [PATCH 5/8] Add unittest --- .../Services/MemberGroupServiceTests.cs | 39 +++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 2 files changed, 40 insertions(+) create mode 100644 src/Umbraco.Tests/Services/MemberGroupServiceTests.cs diff --git a/src/Umbraco.Tests/Services/MemberGroupServiceTests.cs b/src/Umbraco.Tests/Services/MemberGroupServiceTests.cs new file mode 100644 index 0000000000..20205d74bc --- /dev/null +++ b/src/Umbraco.Tests/Services/MemberGroupServiceTests.cs @@ -0,0 +1,39 @@ +using System; +using System.Linq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Rdbms; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.TestHelpers.Entities; + +namespace Umbraco.Tests.Services +{ + [DatabaseTestBehavior(DatabaseBehavior.NewDbFileAndSchemaPerTest)] + [TestFixture, RequiresSTA] + public class MemberGroupServiceTests : BaseServiceTest + { + [SetUp] + public override void Initialize() + { + base.Initialize(); + } + + [TearDown] + public override void TearDown() + { + base.TearDown(); + } + + [Test] + [ExpectedException("System.InvalidOperationException")] + public void New_MemberGroup_Is_Not_Allowed_With_Empty_Name() + { + var service = ServiceContext.MemberGroupService; + + service.Save(new MemberGroup {Name = ""}); + + Assert.Fail("An exception should have been thrown"); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 86c36a8d76..08c44aafdc 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -193,6 +193,7 @@ + From 69548aed20bb676a43cb87424942808d878aefe0 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Sun, 27 Jan 2019 12:33:32 +0100 Subject: [PATCH 6/8] Fix the offset in the crop editor --- .../imaging/umbimagecrop.directive.js | 47 +++++-------------- .../common/services/cropperhelper.service.js | 7 --- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagecrop.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagecrop.directive.js index a287ec0476..62bf888852 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagecrop.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagecrop.directive.js @@ -103,34 +103,6 @@ angular.module("umbraco.directives") scope.dimensions.cropper.height = _viewPortH; // scope.dimensions.viewport.height - 2 * scope.dimensions.margin; }; - - //when loading an image without any crop info, we center and fit it - var resizeImageToEditor = function(){ - //returns size fitting the cropper - var size = cropperHelper.calculateAspectRatioFit( - scope.dimensions.image.width, - scope.dimensions.image.height, - scope.dimensions.cropper.width, - scope.dimensions.cropper.height, - true); - - //sets the image size and updates the scope - scope.dimensions.image.width = size.width; - scope.dimensions.image.height = size.height; - - //calculate the best suited ratios - scope.dimensions.scale.min = size.ratio; - scope.dimensions.scale.max = 2; - scope.dimensions.scale.current = size.ratio; - - //center the image - var position = cropperHelper.centerInsideViewPort(scope.dimensions.image, scope.dimensions.cropper); - scope.dimensions.top = position.top; - scope.dimensions.left = position.left; - - setConstraints(); - }; - //resize to a given ratio var resizeImageToScale = function(ratio){ //do stuff @@ -227,12 +199,19 @@ angular.module("umbraco.directives") //set dimensions on image, viewport, cropper etc setDimensions(image); - //if we have a crop already position the image - if(scope.crop){ - resizeImageToCrop(); - }else{ - resizeImageToEditor(); - } + //create a default crop if we haven't got one already + var createDefaultCrop = !scope.crop; + if (createDefaultCrop) { + calculateCropBox(); + } + + resizeImageToCrop(); + + //if we're creating a new crop, make sure to zoom out fully + if (createDefaultCrop) { + scope.dimensions.scale.current = scope.dimensions.scale.min; + resizeImageToScale(scope.dimensions.scale.min); + } //sets constaints for the cropper setConstraints(); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/cropperhelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/cropperhelper.service.js index e613ccad0d..256a1461db 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/cropperhelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/cropperhelper.service.js @@ -123,13 +123,6 @@ function cropperHelper(umbRequestHelper, $http) { return crop; }, - centerInsideViewPort : function(img, viewport){ - var left = viewport.width/ 2 - img.width / 2, - top = viewport.height / 2 - img.height / 2; - - return {left: left, top: top}; - }, - alignToCoordinates : function(image, center, viewport){ var min_left = (image.width) - (viewport.width); From cca1393f2b7fe48499337281d974b9ce171fb08e Mon Sep 17 00:00:00 2001 From: Vigan Shemsiu Date: Tue, 29 Jan 2019 12:08:49 +0100 Subject: [PATCH 7/8] Validate both width and height Make sure to validate both width and height before running "setDimensions();". The focus point will not work if we have "img { width: 100%; }" in the stylesheet. This pull request will fix the bug. --- .../directives/components/imaging/umbimagegravity.directive.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js index 7d1b1c5a18..28b9ebdb78 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js @@ -35,7 +35,7 @@ angular.module("umbraco.directives") var $overlay = element.find(".overlay"); scope.style = function () { - if (scope.dimensions.width <= 0) { + if (scope.dimensions.width <= 0 || scope.dimensions.height <= 0) { setDimensions(); } From 4dd51f645a715b76c669f0d93070cdbdbd404d8e Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Wed, 30 Jan 2019 16:12:50 +0100 Subject: [PATCH 8/8] Make sure the color picker label is stored on first select (#4325) --- .../colorpicker/colorpicker.controller.js | 70 ++++++------------- .../colorpicker/colorpicker.html | 2 +- 2 files changed, 24 insertions(+), 48 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js index 15d8906ec0..9b38be4791 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.controller.js @@ -12,32 +12,13 @@ function ColorPickerController($scope, angularHelper) { //map back to the model $scope.model.config = config; - function convertArrayToDictionaryArray(model) { - //now we need to format the items in the dictionary because we always want to have an array - var newItems = []; - for (var i = 0; i < model.length; i++) { - newItems.push({ id: model[i], sortOrder: 0, value: model[i] }); - } - - return newItems; - } - - - function convertObjectToDictionaryArray(model) { - //now we need to format the items in the dictionary because we always want to have an array - var newItems = []; - var vals = _.values($scope.model.config.items); - var keys = _.keys($scope.model.config.items); - - for (var i = 0; i < vals.length; i++) { - var label = vals[i].value ? vals[i].value : vals[i]; - newItems.push({ id: keys[i], sortOrder: vals[i].sortOrder, value: label }); - } - - return newItems; - } $scope.isConfigured = $scope.model.config && $scope.model.config.items && _.keys($scope.model.config.items).length > 0; + $scope.model.activeColor = { + value: "", + label: "" + }; + if ($scope.isConfigured) { for (var key in $scope.model.config.items) { @@ -94,33 +75,32 @@ function ColorPickerController($scope, angularHelper) { $scope.isConfigured = $scope.model.config && $scope.model.config.items && _.keys($scope.model.config.items).length > 0; $scope.onSelect = function (color) { - // Complex color (value and label)? - if (!$scope.model.value.hasOwnProperty("value")) { - // did the value change? - if ($scope.model.value !== color) { - // yes, make sure to set dirty - angularHelper.getCurrentForm($scope).$setDirty(); - } - return; - } - // did the value change? if ($scope.model.value.value === color) { - // no, skip the rest + // User clicked the currently selected color + // to remove the selection, they don't want + // to select any color after all. + // Unselect the color and mark as dirty + $scope.model.activeColor = null; + $scope.model.value = null; + angularHelper.getCurrentForm($scope).$setDirty(); + return; } - // yes, make sure to set dirty - angularHelper.getCurrentForm($scope).$setDirty(); - - // update the label according to the new color + // yes, update the model (label + value) according to the new color var selectedItem = _.find($scope.model.config.items, function (item) { return item.value === color; }); if (!selectedItem) { return; } - $scope.model.value.label = selectedItem.label; + $scope.model.value = { + label: selectedItem.label, + value: selectedItem.value + }; + // make sure to set dirty + angularHelper.getCurrentForm($scope).$setDirty(); } // Finds the color best matching the model's color, @@ -128,7 +108,7 @@ function ColorPickerController($scope, angularHelper) { // either the value or label was changed on the data type. function initActiveColor() { - // no value + // no value - initialize default value if (!$scope.model.value) return; @@ -137,10 +117,6 @@ function ColorPickerController($scope, angularHelper) { $scope.model.value = { value: $scope.model.value, label: $scope.model.value }; } - // Complex color (value and label)? - if (!$scope.model.value.hasOwnProperty("value")) - return; - var modelColor = $scope.model.value.value; var modelLabel = $scope.model.value.label; @@ -180,8 +156,8 @@ function ColorPickerController($scope, angularHelper) { // If a match was found, set it as the active color. if (foundItem) { - $scope.model.value.value = foundItem.value; - $scope.model.value.label = foundItem.label; + $scope.model.activeColor.value = foundItem.value; + $scope.model.activeColor.label = foundItem.label; } } diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html index 0b6e8e7c59..9022de8444 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/colorpicker.html @@ -6,7 +6,7 @@