FEATURE: search topics when adding a link in composer (PR #8178)

Seems low probability but on a slow network I think we could have results showing of previous search showing when using backspace and going under 4 chars for linkUrl

I know we don’t do it often, but a debounce like this one is a potential leak, you should cancel it on close

No reason, that call to next was already there. But replacing it with afterRender makes sense.

I’m also concerned by the lack of loading indicator, you really don’t know if something is happening here.

Also this debounce is 100% non functional, try to add a console.log("foo") in the searchTopics and type in the input, it will take 400ms to appear, but you should have multiple foo showing, as it’s just creating a debounce for each char, debounce can be tricky, and it should def not be initialised inside an observer.

I can’t figure out how to do this cleanly without an observer. Do you have a specific suggestion?

Yes, I did, the overflow doesn’t apply on mobile. Here’s a screenshot:

IMG_70DF6904735D-1

I don’t think this can happen because the results get cleared if the input has less than 4 chars (or if it’s a URL).

I couldn’t reproduce the debounce being non-functional. It works correctly when I add my console.log, it displays it for every search request. Keep in mind that the debounce is only triggered if character count is at least 4 and string starting with something other than http.

Thanks, done.

Nevermind, you’re right about this, it does happen.

Try a higher value than 400 ms you will repro easy

1 Like

You should make sure this is cleaned after tests, otherwise we carry this forever doing tests.

this is the issue here for the observer, you are basically relying on mutation of linkUrl, instead of having an event when the textfield is changed, you could have an event on input and make linkUrl readonly, this ways it would make the whole code better.

I realise this is not your code at the beginning, but you are now impacted by this “bad” patter :stuck_out_tongue:

I’m unsure here, but wouldn’t just href alone be enough/better ?

you can use exists helper for this case instead of checking on length

@jjaffeux I think I have now addressed the observer/debounce issues.

1 Like

Yep thx!! Great work, LGTM :heart: