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(HideExpression) add consistent context to function expression #281

Merged
merged 2 commits into from
Nov 24, 2016

Conversation

franzeal
Copy link
Contributor

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

What is the current behavior? (You can also link to an open issue here)
When hideExpression is a function, this and args of the function are inconsistent with the context when the expression is a string and does not seem to provide useful access to the field.

What is the new behavior (if this is a feature change)?
When hideExpression is a function it is called with the same this and args as it would be if it were a string.

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:
This may not be necessary when a hideExpression function is declared in the templateOptions for a field; however, this is intended to address the behavior observed when the hideExpression is declared with a function within the defaultOptions of the formly config.

@codecov-io
Copy link

codecov-io commented Nov 23, 2016

Current coverage is 86.13% (diff: 0.00%)

Merging #281 into master will not change coverage

@@             master       #281   diff @@
==========================================
  Files            18         18          
  Lines           642        642          
  Methods         139        139          
  Messages          0          0          
  Branches        152        152          
==========================================
  Hits            553        553          
  Misses           89         89          
  Partials          0          0          

Powered by Codecov. Last update 6cafbd0...2fc2c09

Copy link
Member

@aitboudad aitboudad left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -7,7 +7,7 @@ export class FormlyFieldVisibilityDelegate {
eval(expression: string | Function | boolean): boolean {
// TODO support this.formlyCommon.field.hideExpression as a observable
if (expression instanceof Function) {
return expression();
return expression.apply(this.formlyCommon, [this.formlyCommon.model, this.formlyCommon.options.formState]);
Copy link
Member

Choose a reason for hiding this comment

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

please update FormlyFieldConfig

hideExpression?: boolean | string | ((model, formState) => boolean);

@mohammedzamakhan
Copy link
Contributor

How is this feature related to validation?

@franzeal franzeal changed the title feat(Validation) add consistent context to function expression feat(HideExpression) add consistent context to function expression Nov 24, 2016
@franzeal franzeal force-pushed the hideExpression-function-context branch from b786b54 to 27b0383 Compare November 24, 2016 18:30
@franzeal
Copy link
Contributor Author

franzeal commented Nov 24, 2016

@mohammedzamakhan Sorry about the incorrect naming. Not sure what I was referencing at the time that lead to my poor choice there.

@aitboudad aitboudad merged commit 691bc64 into ngx-formly:master Nov 24, 2016
@aitboudad
Copy link
Member

Thank you @franzeal

@franzeal franzeal deleted the hideExpression-function-context branch November 24, 2016 18:56
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.

5 participants