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: new search component - implement Combobox approach #506

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

MengLinMaker
Copy link

@MengLinMaker MengLinMaker commented Oct 27, 2024

Draft Search component implementation using Combobox underneath. #504

This solution should be more maintainable than #501

  • Component

    • Export some Combobox components as Search
    • SearchRoot to implement ComboBoxBase disabling defaultFilter as this is handled externally - eg: database
    • Add suggestion debouncer
    • Indicator for loading search query
    • No results found?
    • Remove extraneous components?
  • Test

    • No results - cannot open dropdown with solid testing library
    • Debouncer
  • Doc

    • Import, Features, Anatomy
    • Example: Emoji search
    • Usage: Debounce
    • API Reference
    • Rendered elements, Accessibility
  • Bugs:

    • The dropdown bug: empty options -> typing -> non-empty options -> results in no dropdown unless dropdown is already open (Nani? ¯_(ツ)_/¯)
    • Selecting item triggers onInputChange, which should not trigger
    • Resets input on blur, which it shouldn't - add noResetInputOnBlur option to Combobox

Note: I'm using minisearch npm package to perform text search

Copy link

netlify bot commented Oct 27, 2024

Deploy Preview for kobalte ready!

Name Link
🔨 Latest commit aaafbfd
🔍 Latest deploy log https://app.netlify.com/sites/kobalte/deploys/6723732bfd90a2000879e433
😎 Deploy Preview https://deploy-preview-506--kobalte.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 83 (🔴 down 2 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 82 (🟢 up 7 from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@MengLinMaker MengLinMaker marked this pull request as draft October 27, 2024 15:14
@MengLinMaker MengLinMaker changed the title feat(search): start impl - implement Combobox approach feat: new search component - implement Combobox approach Oct 27, 2024
@MengLinMaker
Copy link
Author

MengLinMaker commented Oct 28, 2024

Solution

Cancel timeouts when onChange is triggered. (May introduce flickering momentarily to loading icon?)

Problem

Example search:

Screen.Recording.2024-10-29.at.8.38.00.am.mp4

Note: debouncer is triggered as emoji is selected (indicated by spinner), this should not happen.

@MengLinMaker
Copy link
Author

MengLinMaker commented Oct 28, 2024

Typing fast then stopping somehow does not display suggestion options:

Screen.Recording.2024-10-29.at.9.07.16.am.mp4

Issue seems to occur when:

  • debounceOptionsMillisecond is over 100ms
  • options was empty []

@MengLinMaker
Copy link
Author

The dropdown bug - solution

Found the cause of the bug:

  • The bug originates from Combobox.Input onInput function, which controls when to open the dropdown.
  • When options is empty, so collection().getSize() = 0, context.open is not triggered.

The possible fix:

  • allowsEmptyCollection should allow Combobox.Input onInput function to trigger context.open
  • Internally context.open will open the dropdown when allowsEmptyCollection is set to true

@MengLinMaker MengLinMaker marked this pull request as ready for review October 30, 2024 22:09
@MengLinMaker
Copy link
Author

@jer3m01 I think this component is ready for review

@MengLinMaker
Copy link
Author

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 this pull request may close these issues.

1 participant