Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Adds a doDidChangeQuery option when updating query. #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lexicalunit
Copy link

If and only if this option is set to false, the didChangeQuery()
callback will NOT trigger after updating the query. This is useful for
things like autocompletion or when you'd want to update the query text
while bypassing code that recomputes and filters the list items.

This is the only change I needed to make to SelectListView to fully
implement autocompletion (see #12) on top of atom-select-list.

If and only if this option is set to false, the didChangeQuery()
callback will NOT trigger after updating the query. This is useful for
things like autocompletion or when you'd want to update the query text
while bypassing code that recomputes and filters the list items.

This is the only change I needed to make to `SelectListView` to fully
implement autocompletion (see atom#12) on top of `atom-select-list`.
@lee-dohm
Copy link

lee-dohm commented May 8, 2018

I'm not sure I understand what the driver is here. I've looked at the issue and I'm not sure if this is the only way to solve this problem. Can you give some more details as to whether this is a problem you're seeing in the way this is being used in Atom or if this is only being used in this way in one of your own packages?

Thanks for the submission and I'm looking forward to hearing more 😀

@lexicalunit
Copy link
Author

Hey @lee-dohm. Thanks for the questions!

So yah, I'd really like to use atom-select-list as-is for my Atom plugins, but one thing I want is the ability to have as-you-type style auto-completion, which is why I opened #12.

So I forked atom-select-list and did what I needed to get auto-completion working. It turned out that the changes in this PR are the minimal amount of changes necessary to allow developers who are using atom-select-list to fully implement auto-completion on top of it. I assumed this would be preferable to a much larger PR that would add auto-completion as a fully fleshed out feature of atom-select-list — Tho if desired I could totally create a PR for that at this point.

A question I have: Do you intend atom-select-list to be a generic selection list component that any Atom plugin developer can pick up and use in their work. Or is it specifically a component for the command palette and not really intended to support a myriad of different use cases. If the former, then a PR for auto-completion I think makes sense. If the later, then it really doesn't because the command palette wouldn't be using that feature.

@lee-dohm
Copy link

My personal feeling is that it should be a reusable component. But I'll have to see how the rest of the maintainer team feels about it. I've tagged this for review in our next weekly meeting and I'll get back to you. Thanks for the detailed explanation, it's a big help 🙇

@lee-dohm
Copy link

@daviwil Can you take a look at the change here and specifically the naming convention established?

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

Successfully merging this pull request may close these issues.

2 participants