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

Check if there is a match in adjustPosition #15

Merged
merged 1 commit into from
Feb 15, 2017
Merged

Check if there is a match in adjustPosition #15

merged 1 commit into from
Feb 15, 2017

Conversation

bkniffler
Copy link
Contributor

... since else, as soon as current word doesn't exactly match trigger (@), the portal will disappear.

@aunswjx
Copy link
Contributor

aunswjx commented Feb 15, 2017

@bkniffler Thank you for contribution

I really want to merge this one since it is a small fix, but I'm curiously want to understand the cause of the problem, because I can't replicate it. Can you provide me some kind of step to replicate the issue, so I can feel comfortable to merge this one?

@bkniffler
Copy link
Contributor Author

bkniffler commented Feb 15, 2017

Well, I copied your example. If I type '@', I can select an item. But if I type '@j', the portal will disappear. The cause seems to be adjustPosition, which will only allow the portal to display if matchTrigger returns true. But matchTrigger will only check if input === trigger, so '@' === '@' is fine, but '@' !== '@j'. Thats why I added a check for the return value of matchCapture, since in my case, this one would return 'j'.

@aunswjx
Copy link
Contributor

aunswjx commented Feb 15, 2017

Thanks @bkniffler

It doesn't occur on my end, but I understand you point.
Merge anyway.

@aunswjx aunswjx merged commit e1d29bb into oozou:master Feb 15, 2017
@bkniffler
Copy link
Contributor Author

bkniffler commented Feb 15, 2017

Hey, how does it not occur on your end? Maybe I misunderstood something in the code. But under what circumstance will the portal remain open if you add more than just '@'? I don't want to add unnecessary code to your plugin @aunsuwijak!

@aunswjx
Copy link
Contributor

aunswjx commented Feb 15, 2017

@bkniffler Actually, I use the example in repo and yes the portal remain open.

bldxk8ntis

@bkniffler
Copy link
Contributor Author

bkniffler commented Feb 15, 2017

Hmm, yeah, you cover 2 cases with expected results:

  • @@@@ will leave portal open, since the last character is always @
  • @2 will close portal, since no key starts with 2

What happens if you insert @j?

@aunswjx
Copy link
Contributor

aunswjx commented Feb 15, 2017

Here it is

unb4wqaizl

@bkniffler
Copy link
Contributor Author

Okay, thats odd since it didn't work for me.. sorry about the fuzz. Anyways, I'll try and give this library a small refactor soon (for #14), so I'll give this a more thorough look.

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