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

When adding a hyperlink, typing something in the URL field that isn’t a URL (i.e. doesn’t start with http) will trigger a search of topics.

image

When selecting a topic from the results, the topic url and title will be added to the respective fields.

Related: https://meta.discourse.org/t/search-existing-topics-when-hyperlinking/112922.

GitHub

You’ve signed the CLA, pmusaraj. Thank you! This pull request is ready for review.

should be grouped:

this.setProperties({
  linkUrl: "",
  linkText: "",
  searchResults: [],
  selectedRow: -1
});

Can you explain why you use next and not afterRender here ?

this listed should be removed, in the onClose hook maybe?

this listener should also be removed

Also note, once I release my select-kit update with better async support, this will be done in few lines of code. I do love the idea though!!!

we really try to not use observers, they are mostly an implementation detail of Ember, do you really need this ?

startsWith is not available under IE11 (https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Objets_globaux/String/startsWith) and we decided to not polyfill it because you can do it very simply with indexOf.

did you test this on mobile ?

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.