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.

Sign up for GitHub

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

Allow to pass in boolean to noDirtyCheck directive #8638

Merged
merged 16 commits into from
Aug 30, 2021
Merged

Allow to pass in boolean to noDirtyCheck directive #8638

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
dd5f65b
Adjust noDirtyCheck directive to accept a parameter to enable/disable…
bjarnef Aug 17, 2020
22d87df
Update dirty property
bjarnef Aug 17, 2020
db2f095
Use Utilities.noop and add description
bjarnef Aug 17, 2020
2514d2b
Set default value from controller
bjarnef Aug 17, 2020
0ab8651
Check for truthly value
bjarnef Sep 1, 2020
0bb2d18
Use Object.toBoolean
bjarnef Sep 2, 2020
eb48dbd
Revert setting $pristine and $dirty for now
bjarnef Sep 2, 2020
9a4867d
Merge remote-tracking branch 'upstream/v8/contrib' into pr/8638
Sep 16, 2020
0a5b442
Handle default values in dirty check attributes
Oct 1, 2020
c08f11c
Merge branch 'v8/contrib' into pr/8638
mikecp Mar 29, 2021
115a475
Add check on presence of disableDirtyCheck atribute
mikecp Mar 29, 2021
b63e218
Check for truthy value otherwise it should be default return false an…
bjarnef Mar 29, 2021
94e8e39
update test conditions for vamue of no-dirty-check and disable-dirty-…
mikecp Mar 30, 2021
6dbc4b4
Remove whitespace
bjarnef Jun 30, 2021
f5c04ee
Merge branch 'v8/contrib' into pr/8638
mikecp Aug 28, 2021
8807bf5
update check
mikecp Aug 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbcheckbox.directive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

function onInit() {
vm.inputId = vm.inputId || "umb-check_" + String.CreateGuid();

vm.disableDirtyCheck = Object.toBoolean(vm.disableDirtyCheck) === true;
vm.icon = vm.icon || vm.iconClass || null;

// If a labelKey is passed let's update the returned text if it's does not contain an opening square bracket [
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbradiobutton.directive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@

function onInit() {
vm.inputId = vm.inputId || "umb-radio_" + String.CreateGuid();

vm.disableDirtyCheck = Object.toBoolean(vm.disableDirtyCheck) === true;
vm.icon = vm.icon || vm.iconClass || null;

// If a labelKey is passed let's update the returned text if it's does not contain an opening square bracket [
Expand Down
11 changes: 5 additions & 6 deletions src/Umbraco.Web.UI.Client/src/common/directives/validation/nodirtycheck.directive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ function noDirtyCheck() {
require: 'ngModel',
link: function (scope, elm, attrs, ctrl) {

var alwaysFalse = {
get: function () { return false; },
set: function () { }
};
Object.defineProperty(ctrl, '$pristine', alwaysFalse);
Object.defineProperty(ctrl, '$dirty', alwaysFalse);
// If no attribute value is "false", then skip and use default behaviour.
var dirtyCheck = scope.$eval(attrs.noDirtyCheck) === false;
Copy link
Contributor

Choose a reason for hiding this comment

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

There must be a native JS function to evaluate a boolean instead of depending on AngularJS to do it for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, probably. However I have noticed the $eval function is used several other places to parse attribute values.

Copy link
Contributor

Choose a reason for hiding this comment

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

With the above changes in mind, shouldn't this be var dirtyCheck = Object.toBoolean(attrs.noDirtyCheck) === false; ?

if (dirtyCheck)
return;

ctrl.$setDirty = Utilities.noop;
Copy link
Contributor

Choose a reason for hiding this comment

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

While I'm sure this works, I'm a tad worried it's too good to be true. The quite intricate solution with the alwaysFalse binding to $pristine and $dirty must be there for a reason... this is just a hunch, but I'm afraid your solution over simplifies things. Wouldn't it be prudent to keep the old solution here, just after the conditional dirty check handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if there is a specific reason it change $pristine. In other alternatives of no-dirty-check directives I have seen in only change $dirty. Regarding the "Unsaved changes" dialog it only rely on $dirty property as far as I know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep I agree with you. As I tried to explain, it should work fine, but the current solution seems quite explicitly done as-is. There can be two reasons for this: Either the current solution is over complicated and implemented as such due to lack of knowledge/understanding, or else the current solution covers edge cases we can't right now. My point is it can't hurt to retain the current solution from a better-safe-than-sorry standpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, not sure if $pristine has any effect on the "Unsaved changes" dialog though. The reason we have the no-dirty-check directive is to prevent the "Unsaved changes" dialog to show in certain circumstances.

It seems it only rely on $dirty before opening the overlay here:

Umbraco-CMS/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js

Lines 175 to 177 in 0140d90

if (!formCtrl.$dirty && infiniteEditors.length === 0 || isSavingNewItem && infiniteEditors.length === 0) {
return;
}

}
};
}
Expand Down
34 changes: 11 additions & 23 deletions src/Umbraco.Web.UI.Client/src/views/components/forms/umb-checkbox.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,17 @@
<label class="checkbox umb-form-check umb-form-check--checkbox {{vm.cssClass}}" ng-class="{ 'umb-form-check--disabled': vm.disabled }">
<span class="umb-form-check__symbol">
<input ng-if="vm.disableDirtyCheck"
type="checkbox"
id="{{vm.inputId}}"
name="{{vm.name}}"
value="{{vm.value}}"
class="umb-form-check__input"
val-server-field="{{vm.serverValidationField}}"
ng-model="vm.model"
ng-disabled="vm.disabled"
ng-required="vm.required"
ng-change="vm.change()"
no-dirty-check />

<input ng-if="!vm.disableDirtyCheck"
type="checkbox"
id="{{vm.inputId}}"
name="{{vm.name}}"
value="{{vm.value}}"
class="umb-form-check__input"
val-server-field="{{vm.serverValidationField}}"
ng-model="vm.model"
ng-disabled="vm.disabled"
ng-required="vm.required"
ng-change="vm.change()"/>
<input type="checkbox"
id="{{vm.inputId}}"
name="{{vm.name}}"
value="{{vm.value}}"
class="umb-form-check__input"
val-server-field="{{vm.serverValidationField}}"
ng-model="vm.model"
ng-disabled="vm.disabled"
ng-required="vm.required"
ng-change="vm.change()"
no-dirty-check="{{vm.disableDirtyCheck}}" />

<span class="umb-form-check__state" aria-hidden="true">
<span class="umb-form-check__check">
Expand Down
34 changes: 11 additions & 23 deletions src/Umbraco.Web.UI.Client/src/views/components/forms/umb-radiobutton.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,17 @@
<label class="radio umb-form-check umb-form-check--radiobutton {{vm.cssClass}}" ng-class="{ 'umb-form-check--disabled': vm.disabled }">
<span class="umb-form-check__symbol">
<input ng-if="vm.disableDirtyCheck"
type="radio"
id="{{vm.inputId}}"
name="{{vm.name}}"
value="{{vm.value}}"
class="umb-form-check__input"
val-server-field="{{vm.serverValidationField}}"
ng-model="vm.model"
ng-disabled="vm.disabled"
ng-required="vm.required"
ng-change="vm.change()"
no-dirty-check />

<input ng-if="!vm.disableDirtyCheck"
type="radio"
id="{{vm.inputId}}"
name="{{vm.name}}"
value="{{vm.value}}"
class="umb-form-check__input"
val-server-field="{{vm.serverValidationField}}"
ng-model="vm.model"
ng-disabled="vm.disabled"
ng-required="vm.required"
ng-change="vm.change()" />
<input type="radio"
id="{{vm.inputId}}"
name="{{vm.name}}"
value="{{vm.value}}"
class="umb-form-check__input"
val-server-field="{{vm.serverValidationField}}"
ng-model="vm.model"
ng-disabled="vm.disabled"
ng-required="vm.required"
ng-change="vm.change()"
no-dirty-check="{{vm.disableDirtyCheck}}" />

<span class="umb-form-check__state" aria-hidden="true">
<span class="umb-form-check__check"></span>
Expand Down