-
Notifications
You must be signed in to change notification settings - Fork 17
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
Fix TNO-1190 #661
Fix TNO-1190 #661
Conversation
@@ -42,7 +42,7 @@ export const Topic: React.FC<ITopicProps> = () => { | |||
: `${item.name} (${item.topicType})`} | |||
</div> | |||
), | |||
item.id, | |||
item.id + '_' + item.name, |
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.
We do not want to do this as the value
is generally used as the primary key of the item. Also this does not actually resolve the issue with the component.
It will be a cleaner solution if we add an optional search property to the IOptionItem
which can be used to filter instead of the default value
property.
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.
It seems that we still need to use the value for filtering purpose in this case even if we add an optional search property, and the current approach is working as expected based on testing, or maybe I miss something else?
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.
Updated, and it seems that a simpler approach than previously thought is available, thanks
@@ -70,6 +70,10 @@ export const Topic: React.FC<ITopicProps> = () => { | |||
)} | |||
required | |||
isDisabled={!values.sourceId} | |||
filterOption={(option, input) => { | |||
const label = (option.label as any)?.props?.children?.toLowerCase(); |
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.
Isn't this converting the html into a string and searching it? If you searched for <
would it return all values?
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.
If we search for "<" it won't return anything, as we just extract the value of the option label in this case, thanks
TNO-1190: cannot search in topic drop down