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

Add SearchableSelect component with example #50

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

Conversation

dricair
Copy link
Contributor

@dricair dricair commented Feb 8, 2025

Added:

  • SearchableSelect isolated inside a searchable_select feature as it adds several dependencies
  • example/searchable_select
  • Documentation

Feedback welcome.

Cedric Airaud added 7 commits February 8, 2025 20:27
Only included as a separate feature due to additional
dependencies.
Open/close the dropdown when clicking on the input

When opening and an element is selected, show it as
active
@isosphere
Copy link
Owner

First, thank you for your contribution. This looks like a good quality PR overall.

The behaviour of the example is jarring. When I click one of the selects, the browser scrolls down to the selected item in the list, even if its the last item. If we're going to move the scroll location, IMO it should be to a bit above the SearchableSelect instance. Perhaps no auto-scroll at all, though?

@dricair
Copy link
Contributor Author

dricair commented Feb 10, 2025 via email

@dricair
Copy link
Contributor Author

dricair commented Feb 10, 2025 via email

isosphere and others added 3 commits February 10, 2025 21:10
Change scrolling behaviour so that the containing
list element scrolls to make the active element
visible, but it does not cause the browser window
to scroll.

It requires to load web-sys with some more
features.
Change title from None to "": this ensure that on
a transition Some(x) -> None the value displayed
in the input is changed correctly.
@dricair
Copy link
Contributor Author

dricair commented Feb 10, 2025

I was actually able to fix both issues.

@isosphere
Copy link
Owner

isosphere commented Feb 13, 2025

This video capture is as of d0d9cca; it feels pretty yanky.

Firefox 128.6.0esr

pr-50-yank-demo.small.mp4

@dricair
Copy link
Contributor Author

dricair commented Feb 13, 2025 via email

@dricair
Copy link
Contributor Author

dricair commented Feb 13, 2025 via email

Cedric Airaud added 3 commits February 21, 2025 21:06
Add this option if scrolling to the selected item is not
wanted.
@dricair
Copy link
Contributor Author

dricair commented Feb 21, 2025

I added the noautoscroll option

@isosphere
Copy link
Owner

I added the noautoscroll option

It seems like it is set to default to false (i.e.: autoscroll enabled), and yet all of the examples no longer display the autoscroll behaviour. I've manually set it to false for Single, and yet it still does not autoscroll.

I would like this to default to disabled, but I think it is doing so right now against your intentions; does the feature still work? I can't seem to get it to.

@dricair
Copy link
Contributor Author

dricair commented Mar 2, 2025

I checked and for me it's still working correctly. If I set noautoscroll = {true} then it stops scrolling automatically. I find this behaviour really not intuitive because if an entry is selected at the end of the list, we need to manually scroll to the end to find the selected entry, so in my opinion it should scroll automatically, even if it can have small side effects depending on the browser.

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.

2 participants