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

regression : search input is disabled if options are lazily loaded when the dropdown is displayed #297

Open
abenhamdine opened this issue Jan 5, 2024 · 14 comments · May be fixed by #327
Open

Comments

@abenhamdine
Copy link
Contributor

abenhamdine commented Jan 5, 2024

because of #276 :
image

and especially e4cd487#diff-7075f90f187d4b281f22b3136982040883bc7595e92f73267c6e47868729dd64R2335
we have many selects in our application where the search input is now disabled, probably because the options are lazily loaded when the dropdown is shown

The culprit code checks a condition on property hasServerSearch which is set to true when onServerSearch is a function.

We don't use onServerSearch, but we didn't have to until now, it was working perfectly.

This change is problematic, is it possible to revert it ?

@abenhamdine abenhamdine changed the title regression : search input is disabled if options are loaded later regression : search input is disabled if options are not provided at init Jan 5, 2024
@abenhamdine abenhamdine changed the title regression : search input is disabled if options are not provided at init regression : search input is disabled if options are lazily loaded when the dropdown is displayed Jan 5, 2024
@gnbm
Copy link
Collaborator

gnbm commented Jan 11, 2024

@abenhamdine feel free to do a PR to fix that (keep in mind that the use case for accessibility needs to be fixed as well)

@gnbm
Copy link
Collaborator

gnbm commented Jan 12, 2024

@abenhamdine could you please share a codepen with that use case?

@abenhamdine
Copy link
Contributor Author

@abenhamdine could you please share a codepen with that use case?

I will try to find time to do it.
But I think any case where you init the select without options, then provide the options later, should exhibit the bug.

@gnbm
Copy link
Collaborator

gnbm commented Jan 31, 2024

@abenhamdine could you please share a codepen with that use case?

I will try to find time to do it. But I think any case where you init the select without options, then provide the options later, should exhibit the bug.

Could you replicate it here? Maybe I'm missing something and you can go from that example and fork it

@gnbm
Copy link
Collaborator

gnbm commented Feb 22, 2024

@abenhamdine any update on this?

@funnygod
Copy link

funnygod commented Mar 5, 2024

When I try to use allowNewOption with an empty option, it also causes the search input to be disabled. Could you please enable the search input when allowNewOption is true? it worked perfectly before version 1.0.41

@gnbm
Copy link
Collaborator

gnbm commented Mar 5, 2024

When I try to use allowNewOption with an empty option, it also causes the search input to be disabled. Could you please enable the search input when allowNewOption is true? it worked perfectly before version 1.0.41

@funnygod I would say it would be better to create a separate issue and include a working example for your scenario that is not the same as reported here.
I might also be missing something but I'm not being able to replicate what you mentioned.

@funnygod
Copy link

funnygod commented Mar 6, 2024

When I try to use allowNewOption with an empty option, it also causes the search input to be disabled. Could you please enable the search input when allowNewOption is true? it worked perfectly before version 1.0.41

@funnygod I would say it would be better to create a separate issue and include a working example for your scenario that is not the same as reported here. I might also be missing something but I'm not being able to replicate what you mentioned.

Thank you, I will open a new issue.

@gnbm
Copy link
Collaborator

gnbm commented Mar 18, 2024

@abenhamdine any update on this? Did you get the chance to create a sample and/or test this with the latest version?

@gnbm
Copy link
Collaborator

gnbm commented Apr 23, 2024

Hi @abenhamdine
I had a similar issue to #319 so I decided to look at the code and try to fix this.
Created a draft PR with a possible solution, could you please help validate this before opening the PR itself (I'm just having some issues running the tests, where 3 are failing, even though I can't manually get any error but would like double check to mitigate the risk of introducing any regression issue)? Does it help on this particular issue, since I was not able to reproduce it?
You'll need to get my branch, build it and use the Dev Tools overrides.

@abenhamdine
Copy link
Contributor Author

Hi @abenhamdine I had a similar issue to #319 so I decided to look at the code and try to fix this. Created a draft PR with a possible solution, could you please help validate this before opening the PR itself (I'm just having some issues running the tests, where 3 are failing, even though I can't manually get any error but would like double check to mitigate the risk of introducing any regression issue)? Does it help on this particular issue, since I was not able to reproduce it? You'll need to get my branch, build it and use the Dev Tools overrides.

hi, thx to try fix this
I assume this particular change aims to fix this issue : https://github.com/sa-si-dev/virtual-select/pull/325/files#diff-c05424d9560afeceb2af0a702e65eceb831b7a202b14940261f43dec5a4b285eR2784 ?

@gnbm
Copy link
Collaborator

gnbm commented Apr 23, 2024

Hi @abenhamdine I had a similar issue to #319 so I decided to look at the code and try to fix this. Created a draft PR with a possible solution, could you please help validate this before opening the PR itself (I'm just having some issues running the tests, where 3 are failing, even though I can't manually get any error but would like double check to mitigate the risk of introducing any regression issue)? Does it help on this particular issue, since I was not able to reproduce it? You'll need to get my branch, build it and use the Dev Tools overrides.

hi, thx to try fix this I assume this particular change aims to fix this issue : https://github.com/sa-si-dev/virtual-select/pull/325/files#diff-c05424d9560afeceb2af0a702e65eceb831b7a202b14940261f43dec5a4b285eR2784 ?

Hi. Since I was not able to reproduce it I just wanted to understand if you could test it for your scenario since you never ended up sharing a sample with this issue.

@abenhamdine
Copy link
Contributor Author

I just have tested version 1.0.43 and this issue is kind of mitigated, but still occurs : search input is still disabled when the user open the dropdown at first time, but when the dropdown is closed then reopened, the search input is now enabled.

@gnbm
Copy link
Collaborator

gnbm commented Apr 24, 2024

I just have tested version 1.0.43 and this issue is kind of mitigated, but still occurs : search input is still disabled when the user open the dropdown at first time, but when the dropdown is closed then reopened, the search input is now enabled.

As mentioned, I was not able to replicate it so to fix it you can give it a try since I won't have time to dedicate to this atm.
If I can help with something I'll try to validate it. But it would be important to have a sample where that can be replicated, like other members did

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants