-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 #6124 : Dropdown navigation like PrimeVue #6430
Fix #6124 : Dropdown navigation like PrimeVue #6430
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
@melloware Hello! Its done |
@KirilCycle i think this may also fix this issue correct? #6417 |
Exactly! |
Let me take a deeper look at this. I like where you are going though. |
Here also second approach, we can use simple number for key at simple list, but for group list we can use key like |
I think we need to study PrimeVue they actually do this: https://github.com/primefaces/primevue/blob/344ec64cd461aad1493073dd3ff10bc8a7338654/components/lib/dropdown/Dropdown.vue#L879-L888 And the Groups are actually a different type of Option that has |
Should I try to apply it the way it's done in vue example? |
Yes we should see if we can modify things the Vue way. If you try the Vue showcase the Dropdown with Groups is working fine: https://primevue.org/dropdown/#group |
Okey ill take a look |
@melloware Vue Dropdown will flat all this to single options list first, and then all logic will work with this flatted list, But the React Dropdown component use the same format as it was passed and functions are not able to work with this type of data, also DropdownPanel is designed to handle groups the way they are, however still has an issue with indexses.I guess we can achieve the similar behaviour to Vue. Let me keep working |
Yes I saw that. I feel like Vue is doing it a cleaner way and we just need to tweak the React code to do the same thing. I think DropDownPanel will need some changes but I think in the long run this will be better. |
components/lib/dropdown/Dropdown.js
Outdated
if (hasFilter && !isLazy) { | ||
const filterValue = filterState.trim().toLocaleLowerCase(props.filterLocale); | ||
const searchFields = props.filterBy ? props.filterBy.split(',') : [props.optionLabel || 'label']; | ||
|
||
if (props.optionGroupLabel) { | ||
let filteredGroups = []; | ||
|
||
for (let optgroup of props.options) { | ||
for (let optgroup of options) { |
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.
Did not check with filter functionality yet
Wow this is looking so much cleaner! |
Oh run |
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.
LGTM. Amazing job thx!
Fix #6124
Fix #6417
Each Group item now have unique index