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

[Combobox] Input values are not filtering properly #4348

Closed
tresh11178 opened this issue Mar 31, 2022 · 11 comments
Closed

[Combobox] Input values are not filtering properly #4348

tresh11178 opened this issue Mar 31, 2022 · 11 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug.

Comments

@tresh11178
Copy link

tresh11178 commented Mar 31, 2022

Actual Behavior

When inputting certain values, the Combobox dropdown does not properly filter the options as expected.

Expected Behavior

The expectation is that all values should be filtered properly based on the input from the user.

Reproduction Sample

https://codepen.io/tre11178/pen/KKZXMYW?editors=100

Reproduction Steps

  1. Click into the Combobox
  2. Type "F" into the input.
  3. Notice that the dropdown is not filtering out all options other than "Fire Safety Team".
  4. Type "Fi" into the input.
  5. Notice the dropdown is still not filtering.
  6. Type "Fir" into the input.
  7. Notice that the dropdown properly filters.

Reproduction Version

beta.80

Relevant Info

No response

Regression?

No response

@tresh11178 tresh11178 added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Mar 31, 2022
@tresh11178 tresh11178 changed the title Combobox is not filtering certain values [Combobox] Input values are not filtering properly Mar 31, 2022
@benelan benelan removed the needs triage Planning workflow - pending design/dev review. label Apr 8, 2022
@benelan benelan added this to the Sprint 04/11 - 04/22 milestone Apr 8, 2022
@Elijbet Elijbet self-assigned this Apr 11, 2022
@Elijbet Elijbet added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Apr 11, 2022
@benelan
Copy link
Member

benelan commented Apr 11, 2022

[Implementer Notes]: This is probably because the forIn function "Iterates over own and inherited enumerable properties of an object invoking iteratee for each property."

All of the combobox-items are matching the word "define"

@Elijbet Elijbet added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Apr 12, 2022
@Elijbet Elijbet added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Apr 20, 2022
@github-actions github-actions bot assigned benelan and geospatialem and unassigned Elijbet Apr 20, 2022
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@geospatialem
Copy link
Member

@Elijbet I'm seeing some filtering issues with a few letters on next. Put together an updated codepen for reference. Can you take a peek 👀?

Note: Added an additional item to showcase filtering further, but tested across the alphabet.

Some of the filtering experienced:

  • d, For results must type "di"
  • e, For results must type "edi"
  • f, For results must type "fir"
  • n, For results must type "na"
  • u, For results must type "ur"

filter results from 'd'

filter results from 'di'

@Elijbet
Copy link
Contributor

Elijbet commented Apr 22, 2022

Sure, I'll check now. It might be that with the newer solution we ended up leaving other properties in there still.

@benelan
Copy link
Member

benelan commented Apr 22, 2022

Yeah that's what I thought. Its not just the guid, its the inherited properties too. The first solution worked IIRC.

@benelan
Copy link
Member

benelan commented Apr 22, 2022

Or if I'm wrong and the const inputSubset = (({ label, value }) => ({ label, value }))(input as any); didn't work, I'd look into Object.hasOwn.

@Elijbet
Copy link
Contributor

Elijbet commented Apr 22, 2022

Yeah that's what I thought. Its not just the guid, its the inherited properties too. The first solution worked IIRC.

It's the right thing to do, to remove the guid there. But I should still only filter the subset of the input object. And I'll try your suggestion as well @benelan

@benelan
Copy link
Member

benelan commented Apr 22, 2022

Yeah agreed, we don't need the guid there. I was just referring to when I requested changes. That's my fault, I should have been clearer in my note above about the inherited enumerable properties being the issue.

@geospatialem
Copy link
Member

Going to reallocate this on back to in development. Thanks for the background/insights @benelan and for investigating, @Elijbet!

@geospatialem geospatialem added 2 - in development Issues that are actively being worked on. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Apr 22, 2022
@Elijbet Elijbet assigned benelan and unassigned Elijbet Apr 25, 2022
@benelan benelan added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels May 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2022

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels May 6, 2022
@geospatialem
Copy link
Member

Verified on next ✔️

verification confirmation with 'f' for fire filtering

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug.
Projects
None yet
Development

No branches or pull requests

4 participants