-
Notifications
You must be signed in to change notification settings - Fork 87
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
Even more typecheck fixes for gmf #4588
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that the getter/setters
filterCondition
for ngeo/datasource/OGC have been removed. Why?
Probably by error ...
* @return {string} Filter condition | ||
* @export | ||
*/ | ||
get filterCondition() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to have getter and setter for this variable ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This variable needs to be changed. In the filter component, one can set OR instead of AND, which affect this variable, thus it needs a setter
- It needs a getter because we bind UI components to it
- If you look around the other properties of the data source classes, you'll notice that they have getters/setters as well and those we're removed. If this one was removed, then why not the others?
- Back then, when this was first implemented, it was my first time writing ES6 classes, and the use of getters/setters seemed quite nice and it was working with Angular well. This was a good choice.
- The typecheck complains that there is not a filterCondition property set. Is this because inner properties can't be accessed?
Please, comment. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so it was changed ("simple" public variable to getter / setter) for the typecheck ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why it was changed. It was using a getter/setter before and it was working fine.
9df77f7
to
8a4d6fc
Compare
Please, let me know if I can merge this. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks :-)
As title says.
I noticed that the getter/setters
filterCondition
for ngeo/datasource/OGC have been removed. Why?