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

pass Transaction to validFor #23

Closed
wants to merge 2 commits into from

Conversation

AlexErrant
Copy link
Contributor

I'm using codemirror as a searchbox. I have two "types" of autocompletions:

  1. Historical, i.e. previous things you've searched for.
  2. Syntactical, i.e. typing kind: will give you different "kinds" of cards that you can search for.

I'm using activateOnCompletion to get completion after accepting another completion. I'm also using validFor to make historical completions work since they may have spaces.

However, as seen in this demo video, the historical completion is "overriding" the syntactical completion because the validFor is still returning true even after the initial completion was accepted. You can play with the searchbox here.

This PR passes the Transaction to validFor. Using that transaction, I can call tr.isUserEvent('input.complete') and break out of validFor. I used pnpm patch to test this new behavior in a branch of my project and it works the way I want.

@marijnh
Copy link
Member

marijnh commented Jun 20, 2024

Would it maybe make more sense to always reset existing completions when picking a completion?

@AlexErrant
Copy link
Contributor Author

Yeah, I can't think of a scenario where someone would want to keep the existing completions after accepting one of them. I tried to avoid potentially breaking behavior by making a small additive change. But if you also think it would make sense for validFor to always return false after accepting a completion, that'd work for me!

@marijnh marijnh closed this in 82893f8 Jun 21, 2024
@marijnh
Copy link
Member

marijnh commented Jun 21, 2024

I've pushed an alternative patch, which cleans up some of the state transition handling, and does not add this argument to validFor.

@AlexErrant AlexErrant deleted the validForTransaction branch June 21, 2024 14:32
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