webscopeio / react-textarea-autocomplete

📝 React component implements configurable GitHub's like textarea autocomplete.
MIT License
451 stars 80 forks source link

Prevent opening the autocomplete on a pair of triggers #142

Closed andypearson closed 5 years ago

andypearson commented 5 years ago

Uh oh, it's me again!

This bug has been reported by one of my users, after they have put in a paint (resulting in a complete token that looks like :PaintRange/PaintName:) when they backspace, and then type a character the autocomplete opens again:

large gif 766x446

I tried to rectify this by using the afterWhitespace: true option, which I understand to mean that a trigger will only open the autocomplete if it is preceded by a character of whitespace, but for some reason that means that I can no longer open the autocomplete at all using the ":" trigger.

I also looked to see if I could do something by passing in a different regex, but it still looks like it is quite hard to override the actual trigger matching functionality.

I guess I kind of want a look back, to see if the current triggering ":" is actually part of a complete token, but I'm not sure that's going to be easy.

Any ideas?

jukben commented 5 years ago

Hey! 👋

Haha, nice 🐛. Actually, this is quite difficult due to the current architecture. However afterWhitespace: true should prevent it.

Check it out this and "use" ; trigger. https://926-94480675-gh.circle-artifacts.com/0/example/index.html

However, which is really strange, it's not working on https://codepen.io/jukben/pen/bYZqvR 😱

There actually an additional bug, that afterWhitespace: true should be ignored when the textarea is empty. But this should be easy to fix, first things first.

Which build are you using? (UMD, ES)

andypearson commented 5 years ago

Errrrr I'm using whatever build I get with yarn add so ES I guess?

I can't seem to get either example to work (in Safari 12.0.2 or Chrome latest).

Although I might be misunderstanding what you mean!

jukben commented 5 years ago

Not sure if we are on the same page, but I think this could solve the issue for you if you would use this as in this example. e.g use autocomplete only when the token is acting as "new word". Does it cover all your use cases?

afterwhitespace

andypearson commented 5 years ago

@jukben yes, that looks like it would do the trick!

Is that afterWhitespace or a new option?

jukben commented 5 years ago

It should be afterWhitespace – but there might be issue, that the autocomplete won't open if the first character you type gonna be the ; e.g. the logic should be tweaked – I can do it (I'd say the current behavior is a bug).

However, I'm curious if it works for you because I'm codepen it's somehow broken :O. Could you please check it out afterWhitespace option in your project and let me know?

andypearson commented 5 years ago

Yeah, I tried it in my project and for some reason when afterWhitespace: true is set then the autocomplete doesn't open at all, even if I pad the string a bit:

large gif 492x312

But here it is working fine with that option commented out:

large gif 492x312

jukben commented 5 years ago

@andypearson 😞 That so weird. How it's this even possible. This works for you as intended right https://926-94480675-gh.circle-artifacts.com/0/example/index.html, with ; trigger? There has to be some bug but I'm not able to reproduce it.

andypearson commented 5 years ago

Can confirm that https://926-94480675-gh.circle-artifacts.com/0/example/index.html works as expected with the ; trigger.

Is it possible that a trigger of : could be breaking the generated regex?

I will try changing my trigger to ; or [ to see if that helps... will report back in a bit!

andypearson commented 5 years ago

@jukben Okay, I've just tested and all of :, ; and [ work perfectly until afterWhitespace: true is added, at which point the autocomplete stops showing up completely.

Here is the triggers object I am using, in case that has anything to do with it...

{
  ":": {
    component: PaintOption,
    output: (item, trigger) => `${trigger}${item.fullIdentifier}${trigger}`,
    afterWhitespace: true,
    dataProvider: token => (
      fuzzy
        .filter(last(token.split("/")), paints, fuzzyOptions)
        .slice(0, RESULTS_LIMIT)
        .map(result => ({
          ...result.original,
          name: result.string
        }))
    )
  }
}
jukben commented 5 years ago

This is very strange indeed. I'll try to create "repro" from scratch or https://codesandbox.io during the weekend. 🙋‍♂️

jukben commented 5 years ago

Found the issue, working on fix. There has to be taken minChar in mind. Unfortunately the test were unable to catch it. :(

jukben commented 5 years ago

Hey @andypearson I've just released 4.3.3 🚀 – now the afterWhitespace should work as expected. Please, give it a try.

andypearson commented 5 years ago

@jukben should have time to try it out in the next couple of days, will let you know how it goes :)

andypearson commented 5 years ago

@jukben just tried your fix and it's working a treat! My autocomplete is working perfectly now, time to ship! Thanks again! 🎉