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

__isNew__ options always showing even when filtering #4902

Open
ro-savage opened this issue Nov 7, 2021 · 10 comments
Open

__isNew__ options always showing even when filtering #4902

ro-savage opened this issue Nov 7, 2021 · 10 comments
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet

Comments

@ro-savage
Copy link

ro-savage commented Nov 7, 2021

Are you reporting a bug or runtime error?

In V5 using creatable and filterable options, when an option has __isNew__: true it will always show in the options, even when filtering.

This issue didn't exist in v4, and there is no mention of it in the changelog.

This issue is caused by this line:

if ((option.data as { __isNew__?: unknown }).__isNew__) return true;

Was introduce by this PR: #4702
Which doesn't mention this being an indented behaviour.

Video:
https://www.loom.com/share/e343c5d9770544c398a3c3d5537c649d

See this video of the difference between React-Select 4 and 5.
https://www.loom.com/share/9c7aff61336a475398750539200957a7

Codesandbox:
https://codesandbox.io/s/codesandboxer-example-forked-o2wlz?file=/example.tsx

@ro-savage ro-savage added the issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet label Nov 7, 2021
@ro-savage
Copy link
Author

Happy to make a PR for this, if you can confirm the new items should be filtered.

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Nov 9, 2021

Thanks for bringing this up. It is indeed intended behavior. Can you explain your use case for not wanting new options to be displayed when filtering? Is your new option not tied to what the user is typing in the input?

@ebonow
Copy link
Collaborator

ebonow commented Nov 9, 2021

@Methuselah96

My understanding is that the issue is not the "Create option" but rather onCreate when the option is usually added to the list of options. The newly created option has __isNew__ added to it, so it will always display regardless of what the user has typed after it's been added to the list of options.

This likely could be mitigated by removing the isNew identifier from the newly created option before adding to the list of options, but also worth noting that this also creates a mismatch of the data of the selected value versus the option in the menu.

@ro-savage
Copy link
Author

@Methuselah96 -

The basic case here is that as a user I am filtering my options, but there is an option that isn't filtered, and there is no visible reason the filter isn't being applied.

In our case, we have a form to create many projects, and when selecting a client, you can choose to create a new client.
When you add another project to the form, we want the option to use any of the existing clients, or one of the newly created clients, or create another new client. However now I always see the new options, even when I have applied my filter.

See this video of the difference between React-Select 4 and 5.
https://www.loom.com/share/9c7aff61336a475398750539200957a7

@Methuselah96
Copy link
Collaborator

Got it, that second video is very helpful. Sorry I missed the first video/CodeSandbox in the original description.

How are you sharing the options between the different Select components? Is it similar to how you're doing it in the CodeSandbox? Do you need the options to include the __isNew__ property?

@ro-savage
Copy link
Author

@Methuselah96 - Yes - we do the same as in the codesandbox. We have a array of all options and add to that array of options when a new option is created. This way you can use the new options in other dropdowns before the data has been saved.

We use __isNew__ to let us display newly created items differently, and when we save the data to know what items to create and what already exist.

However, we can handle this on our side separately and remove the __isNew__ property before saving to state. The reason we didn't was because it felt to us like a bug and undocumented change in behaviour.

What is the use case for filtering to not work on newly created options? Is it common someone would try to filter, but also not want to filter out new items?

If this is intended behaviour going forward, we will handle our use case in our app code instead..

@Methuselah96
Copy link
Collaborator

I don't think we considered multiple Selects sharing the same array of creatable options. The thinking was: what is the use case for not showing the "creatable" option? For example, if the creatable option label did not include the text that the user was typing (e.g., if it was just a fixed label) then it might be unintentionally filtered. The fact that it would be filtered out in that scenario was considered a bug which is why we didn't categorize it as a breaking change. We'll discuss this at our maintainers meeting and figure out what we want the intended behavior to be going forward.

@ro-savage
Copy link
Author

Great - Thank you.

The previous behaviour didn't the filter applied and the behaviour to described didn't happen. Then is because when you are typing you have the "Create " option. So the create is always shown, and is never hidden.

The only way to filter is after you've chosen to create it, and that is the same time the __isNew__ is applied.

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Nov 10, 2021

Right, just for context, I think this is the scenario we were trying to "fix" (this obviously doesn't make sense as-is, but some scenario where the option doesn't necessarily contain what the user typed): https://codesandbox.io/s/strange-cache-xijtl?file=/src/App.js. When you type in the input it filters out the creatable option because it's not based on the input value.

@ro-savage
Copy link
Author

Thanks for all your input @Methuselah96

We fixed this on our side some time ago but overwriting the options properties.

I still think the current behaviour doesn't make a lot of sense - to me a filter should always be applied when you are filtering. Regardless of the isNew property. But I understand you might see it differently.

Happy for this to be closed, if this is going to stay as intended behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet
Projects
None yet
Development

No branches or pull requests

3 participants