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.

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(SelectMenu/InputMenu): migrate create-item prop to Reka #2720

Draft
wants to merge 2 commits into
base: feat/reka-ui
Choose a base branch
from

Conversation

ineshbose
Copy link
Member

@@ -204,33 +203,54 @@ function displayValue(value: T | T[]): string {
return item && (typeof item === 'object' ? get(item, props.labelKey as string) : item)
}

function filterFunction(
Copy link
Member Author

Choose a reason for hiding this comment

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

Note - we can get rid of this filterFunction; we can make use of useFilter from reka-ui. I just wonder about custom filter option when the guide for it is completed 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, to access the filtered options, we would need to use ComboboxRootContext (https://reka-ui.com/docs/guides/inject-context) so the SFC would need to be split

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we can't make our own filter with escapeRegExp because it won't necessarily match the filter of Reka UI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you advise on if/how we should split the SFC to access the Combobox context? (as it should be child components)

Copy link
Member

Choose a reason for hiding this comment

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

@zernonia Would you have some hint about this? What would be the best way to access the filtered items?

@benjamincanac
Copy link
Member

Why reintroduce filterFunction? 🤔 It will not be called by the Combobox

@ineshbose
Copy link
Member Author

Why reintroduce filterFunction? 🤔 It will not be called by the Combobox

Lets discuss under #2720 (comment)

@@ -281,6 +302,7 @@ function onUpdateOpen(value: boolean) {
:disabled="disabled"
:multiple="multiple"
:by="by"
:ignore-filter="props.filter === false"
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep using ignore-filter? It's inherited from Combobox already and filter prop has been removed already.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I'll remove it - didn't spot that!

@benjamincanac benjamincanac added the v3 #1289 label Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 #1289
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants