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

fix(SelectMenu): use by prop to compare objects for selected values #2228

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
+25 −4
Open

fix(SelectMenu): use by prop to compare objects for selected values #2228

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1142db1
fix(SelectMenu): use `by` prop to compare objects for selected values
CernyMatej Sep 20, 2024
119c3df
fix(SelectMenu): add modelValue type
CernyMatej Sep 20, 2024
647dbeb
feat(SelectMenu): handle nested `valueAttribute` comparison using the…
CernyMatej Sep 20, 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
11 changes: 10 additions & 1 deletion src/runtime/components/forms/SelectMenu.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,15 @@
if (props.valueAttribute) {
return options.value.filter(option => (props.modelValue as any[]).includes(option[props.valueAttribute]))
}

if (props.by) {
return options.value.filter(
option => typeof option === 'object' && option !== null && props.modelValue.some(

Check failure on line 378 in src/runtime/components/forms/SelectMenu.vue

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, 20) 

Property 'some' does not exist on type 'string | number | boolean | Record<string, any> | unknown[]'.
(value: any) => typeof value === 'object' && value !== null && value[props.by] === option[props.by]
)
)
}

return options.value.filter(option => (props.modelValue as any[]).includes(option))
}

Expand Down Expand Up @@ -481,7 +490,7 @@
}

return props.options || []
}, [], {
}, props.options || [], {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benjamincanac Since the selected computed proeprty uses the computedAsync options to get the possible values, I initialized it to props.options so that it isn't empty during the initial render of the component to prevent a flash.
However, now that I'm thinking about it, shouldn't we just use props.options directly in the selected computed, instead of options - since we don't want to compare the model values with only the search results, but rather the whole options?

Copy link
Contributor Author

@CernyMatej CernyMatej Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I used options because that's what was used previously, however, unless I'm missing something, I think that props.options should have been used there as well:

image

Though this only affects things if someone uses asynchronous search.

lazy: props.searchableLazy
})

Expand Down
Loading