-
Notifications
You must be signed in to change notification settings - Fork 221
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
Added compareWith input for select #310
base: master
Are you sure you want to change the base?
Conversation
This function allows to add a comparison function for the option used as a value. It is called when searching for the option in the options array. If it is not defined, it defaults to a standard === comparison.
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.
Thank you for opening this, I'm sorry it's taken so long to be looked at. 2 points and then we're good to go.
@@ -197,6 +197,11 @@ export class SelectPage { | |||
"Must return a <code>Promise</code>. " + | |||
"This must be defined as an arrow function in your class." | |||
}, | |||
{ | |||
name: "compareWith", |
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.
Could you name this function optionsCompare
please to get it inline with optionsLookup
?
@@ -331,6 +341,9 @@ export abstract class SuiSelectBase<T, U> implements AfterContentInit, OnDestroy | |||
public abstract selectOption(option:T):void; | |||
|
|||
protected findOption(options:T[], value:U):T | undefined { | |||
if (this._compareWith) { |
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 think rather than checking to see if the function has been assigned, it would be cleaner to initialise this property in the constructor with (a, b) => a === b
, then you don't need this check.
This function allows to add a comparison function for the option used as a value. It is called when searching for the option in the options array. If it is not defined, it defaults to a standard === comparison.
I haven't fully tested yet because I'm not quite sure that this is going in the right direction. If it seems to be correct, I'll update and add maybe an example.
Before submitting a pull request, please make sure you have at least performed the following: