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

feat(core): add field arg to hideExpression and expressionProperties #1544

Merged
merged 1 commit into from
Apr 19, 2019

Conversation

intellix
Copy link
Contributor

@intellix intellix commented Apr 18, 2019

Add field argument to hideExpression and expressionProperties

fix #704

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Please check if the PR fulfills these requirements

Please provide a screenshot of this feature before and after your code changes, if applicable.

Other information:

@intellix intellix force-pushed the hide-expression-field-param branch from acfe0b2 to f6458c3 Compare April 18, 2019 14:03
@aitboudad
Copy link
Member

could you please apply the same for expressionProperties

@intellix intellix force-pushed the hide-expression-field-param branch from f6458c3 to 1ca48cb Compare April 18, 2019 14:47
@intellix
Copy link
Contributor Author

I think I managed it @aitboudad - LMK

@intellix intellix changed the title feat(core): add field arg to hideExpression feat(core): add field arg to hideExpression and expressionProperties Apr 18, 2019
@juristr
Copy link
Collaborator

juristr commented Apr 18, 2019

awesome 💪 should we add a test for this as well?

Add field argument to hideExpression and expressionProperties

fix ngx-formly#704
@intellix intellix force-pushed the hide-expression-field-param branch from 1ca48cb to e43f530 Compare April 19, 2019 01:18
@intellix
Copy link
Contributor Author

Added a test for hideExpression but couldn't really see similar tests for expressionProperties. Also added a reference in expression-properties.md after finding it searching through the codebase

@aitboudad aitboudad merged commit 24998ad into ngx-formly:master Apr 19, 2019
@aitboudad
Copy link
Member

Thank you @intellix, expect 5.1 release next week.

@intellix intellix deleted the hide-expression-field-param branch April 19, 2019 14:28
@intellix
Copy link
Contributor Author

intellix commented Apr 19, 2019

Sure, thanks :) For the impatient:

https://github.com/ds300/patch-package
patches/@ngx-formly+core+5.0.4.patch

diff --git a/node_modules/@ngx-formly/core/fesm5/ngx-formly-core.js b/node_modules/@ngx-formly/core/fesm5/ngx-formly-core.js
index 7711dfb..09596cc 100644
--- a/node_modules/@ngx-formly/core/fesm5/ngx-formly-core.js
+++ b/node_modules/@ngx-formly/core/fesm5/ngx-formly-core.js
@@ -2204,7 +2204,7 @@ FieldExpressionExtension = /** @class */ (function () {
          */
         function () { return false; }));
         if (typeof expression === 'string') {
-            expression = evalStringExpression(expression, ['model', 'formState']);
+            expression = evalStringExpression(expression, ['model', 'formState', 'field']);
         }
         return parentExpression
             ? (/**
@@ -2212,7 +2212,7 @@ FieldExpressionExtension = /** @class */ (function () {
              * @param {?} formState
              * @return {?}
              */
-            function (model, formState) { return parentExpression() || expression(model, formState); })
+            function (model, formState, field) { return parentExpression() || expression(model, formState, field); })
             : expression;
     };
     /**
@@ -2275,7 +2275,7 @@ FieldExpressionExtension = /** @class */ (function () {
         function (v) { return "templateOptions." + v; }));
         for (var key in expressionProperties) {
             /** @type {?} */
-            var expressionValue = evalExpression(expressionProperties[key].expression, { field: field }, [field.model, field.options.formState]);
+            var expressionValue = evalExpression(expressionProperties[key].expression, { field: field }, [field.model, field.options.formState, field]);
             if (key === 'templateOptions.disabled') {
                 expressionValue = !!expressionValue;
             }
@@ -2319,7 +2319,7 @@ FieldExpressionExtension = /** @class */ (function () {
             return false;
         }
         /** @type {?} */
-        var hideExpressionResult = !!evalExpression(field.hideExpression, { field: field }, [field.model, field.options.formState]);
+        var hideExpressionResult = !!evalExpression(field.hideExpression, { field: field }, [field.model, field.options.formState, field]);
         /** @type {?} */
         var markForCheck = false;
         if (hideExpressionResult !== field.hide || ignoreCache) {
diff --git a/node_modules/@ngx-formly/core/lib/components/formly.field.config.d.ts b/node_modules/@ngx-formly/core/lib/components/formly.field.config.d.ts
index f69113d..8e88bc5 100644
--- a/node_modules/@ngx-formly/core/lib/components/formly.field.config.d.ts
+++ b/node_modules/@ngx-formly/core/lib/components/formly.field.config.d.ts
@@ -81,12 +81,12 @@ export interface FormlyFieldConfig {
     /**
      * Conditionally hiding Field based on values from other Fields
      */
-    hideExpression?: boolean | string | ((model: any, formState: any) => boolean);
+    hideExpression?: boolean | string | ((model: any, formState: any, field: FormlyFieldConfig) => boolean);
     /**
      * An object where the key is a property to be set on the main field config and the value is an expression used to assign that property.
      */
     expressionProperties?: {
-        [property: string]: string | ((model: any, formState: any) => any) | Observable<any>;
+        [property: string]: string | ((model: any, formState: any, field: FormlyFieldConfig) => any) | Observable<any>;
     };
     /**
      * This is the [FormControl](https://angular.io/api/forms/FormControl) for the field.

@juristr
Copy link
Collaborator

juristr commented Apr 24, 2019

Just a consideration as I was applying the patch meanwhile until v5.1 gets released. The newly added param is actually a required one and I was wondering if we want to leave that or make it "optional" similar to the FormlyHookFn where it is optional as well:

export interface FormlyHookFn {
(field?: FormlyFieldConfig): void;
}

I'm mentioning this here as in my setup after upgrading to v5.1 it doesn't compile any more, because I manually did some manipulation of a formly config before giving it to the formly component where I directly invoked the hideExpression function.

...
// hide the checkbox when the according control is hidden dynamically
checkboxConfig.hideExpression = (model, formState) => {
  if (field.hideExpression instanceof Function) {
    return field.hideExpression(model, formState); // <<<<< BREAKS
  } else { 
    return false;
  }
};

Obviously TypeScript doesn't compile as now 3 params are expected. I don't see this as a big problem, as it's quite an edge case. Just wanted to mention that to you guys @aitboudad @intellix

@aitboudad
Copy link
Member

@juristr I'll take care of it, Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Problem with passing model in hideExpression when have fieldGroup
3 participants