Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#6233 - Listbox: Preserve option groups while filtering. #6275

Merged
merged 7 commits into from
Nov 13, 2024
Merged

#6233 - Listbox: Preserve option groups while filtering. #6275

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0656308
fix: Listbox: Preserve option groups while filtering.
avramz Aug 22, 2024
ecbdfc1
Merge branch 'primefaces:master' into fix/listbox-optiongroup-filter
avramz Sep 15, 2024
e5a49eb
- optimize the options display
avramz Sep 15, 2024
d4e44f3
- optimize optionsListGroup
avramz Sep 15, 2024
ea76bbb
Merge branch 'primefaces:master' into fix/listbox-optiongroup-filter
avramz Sep 16, 2024
41291b9
Merge branch 'primefaces:master' into fix/listbox-optiongroup-filter
avramz Sep 18, 2024
7cd8578
Merge branch 'primefaces:master' into fix/listbox-optiongroup-filter
avramz Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions packages/primevue/src/listbox/Listbox.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,70 @@ describe('Listbox.vue', () => {

expect(icon.classes()).toContain('pi-discord');
});

it('should correctly filter', async () => {
await wrapper.setProps({
filter: true
});

const filterInput = wrapper.find('input.p-listbox-filter');

expect(filterInput.exists()).toBe(true);

await filterInput.setValue('is');

const options = wrapper.findAll('.p-listbox-option');

expect(options.length).toBe(2);
expect(options[0].text()).toBe('Istanbul');
});

it('should correctly filter groups', async () => {
await wrapper.setProps({
filter: true,
optionGroupLabel: 'label',
optionLabel: 'label',
optionGroupChildren: 'items',
options: [
{
label: 'Germany',
code: 'DE',
items: [
{ label: 'Berlin', value: 'Berlin' },
{ label: 'Frankfurt', value: 'Frankfurt' },
{ label: 'Hamburg', value: 'Hamburg' },
{ label: 'Munich', value: 'Munich' }
]
},
{
label: 'USA',
code: 'US',
items: [
{ label: 'Chicago', value: 'Chicago' },
{ label: 'Los Angeles', value: 'Los Angeles' },
{ label: 'New York', value: 'New York' },
{ label: 'San Francisco', value: 'San Francisco' }
]
}
]
});

const filterInput = wrapper.find('input.p-listbox-filter');

expect(filterInput.exists()).toBe(true);

await filterInput.setValue('ch');

const optionGroups = wrapper.findAll('.p-listbox-option-group');
const options = wrapper.findAll('.p-listbox-option');

expect(optionGroups.length).toBe(2);
expect(optionGroups[0].text()).toBe('Germany');
expect(optionGroups[1].text()).toBe('USA');

expect(options.length).toBe(2);
expect(options[0].text()).toBe('Munich');
expect(options[1].text()).toBe('Chicago');
});
});
});
22 changes: 21 additions & 1 deletion packages/primevue/src/listbox/Listbox.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,27 @@ export default {
visibleOptions() {
const options = this.optionGroupLabel ? this.flatOptions(this.options) : this.options || [];

return this.filterValue ? FilterService.filter(options, this.searchFields, this.filterValue, this.filterMatchMode, this.filterLocale) : options;
if (this.filterValue) {
const filteredOptions = FilterService.filter(options, this.searchFields, this.filterValue, this.filterMatchMode, this.filterLocale);

if (this.optionGroupLabel) {
const optionGroups = this.options || [];
const filtered = [];

optionGroups.forEach((group) => {
const groupChildren = this.getOptionGroupChildren(group);
const filteredItems = groupChildren.filter((item) => filteredOptions.includes(item));

if (filteredItems.length > 0) filtered.push({ ...group, [typeof this.optionGroupChildren === 'string' ? this.optionGroupChildren : 'items']: [...filteredItems] });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about typeof this.optionGroupChildren is not 'string'? It seems it returns 'items'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

items is a fallback - this method is the same as in MultiSelect -> https://github.com/primefaces/primevue/blob/master/packages/primevue/src/multiselect/MultiSelect.vue#L1012
and Select -> https://github.com/primefaces/primevue/blob/master/packages/primevue/src/select/Select.vue#L926

They both deal with the same data structure and I didn't want to reimplement the logic for consistency.
If the filtering logic for Listbox still needs to be changed I suggest that Select/MultiSelect is updated too, as they deal with the same data structures. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said before, it is not correct to return a string here. We are open to new solutions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the PR with optimized visibleOptions rendering logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tugcekucukoglu any feedback?

});

return this.flatOptions(filtered);
}

return filteredOptions;
}

return options;
},
hasSelectedOption() {
return isNotEmpty(this.modelValue);
Expand Down
Loading