-
Notifications
You must be signed in to change notification settings - Fork 125
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 Search Field functionality to Combobox component #4354
Conversation
✔️ Deploy preview for fundamental-ngx ready! 🔨 Explore the source changes: 4b11721 🔍 Inspect the deploy logs: https://app.netlify.com/sites/fundamental-ngx/deploys/6005a311e1e34e00084182fa 😎 Browse the preview: https://deploy-preview-4354--fundamental-ngx.netlify.app |
@@ -3,6 +3,7 @@ | |||
<ng-template #desktopTemplate> | |||
<fd-popover | |||
additionalBodyClass="fd-popover-custom-list" | |||
[class.fd-combobox-full-width]="isSearch" |
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.
Shouldn't this be up to the developer? If we are making this decision for them, not sure why it'd only apply to search comboboxes and not all of them
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.
That's a good question. I also thought about it during the implementation. Why I did it this way? Combobox has restrictions when it comes to max and min width, which do not apply on Search Field. Multi Input, for example, also takes the 100% of the container. Making Search Field "full-width" we give the option to the user to apply whatever width they want on the container inside which they gonna place the Search field and this way also remove the css rules that apply on the Compobox when in comes to the width. Does this make sense?
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.
Yes! PR looks good
Please provide a link to the associated issue.
Closes #3375
Please provide a brief summary of this pull request.
This PR introduces Search Field enhancement to Combobox component. When the
isSearch
property is set to true, the glyph of the Input Group changes to 'search' icon and also an additional clear button is shown when the user starts typing in the input field. The Clear button can be hidden by setting theshowClearButton
to false. By default it's set to true.Please check whether the PR fulfills the following requirements
https://github.com/SAP/fundamental-ngx/blob/main/CONTRIBUTING.md
https://github.com/SAP/fundamental-ngx/wiki/PR-Review-Checklist
Documentation checklist:
README.md