2
\$\begingroup\$

I am trying to learn Meteor, so I tried to write a basic location autocomplete.

Currently, given a string, it will request suggestions from https://photon.komoot.de/ (amazing free location autocomplete btw) and display them in an ul-li. What is not done yet is keyboard navigation.

autocomplete.html

<template name="autocomplete">
    <input id="{{name}}" class="autocomplete-input {{className}}"
        placeholder="{{placeholder}}" autocomplete="off"/>
    <ul class="suggestions"></ul>
</template>

autocomplete.js

(function() {
    var inputEl, suggestionsEl, timeout, displayedSuggestions, selectedSuggestion;
    var search = function(inputString) {
        if (timeout) {
            clearTimeout(timeout);
        }
        timeout = setTimeout(function() {
            HTTP.get("https://photon.komoot.de/api/?q=" + inputString + "&lang=" + globalConfig.language, {},
                function(error, response) {
                    if (error) {

                    } else {
                        displayedSuggestions = response.data.features;
                        display(displayedSuggestions);
                    }
                });
        }, 300);
    };

    var suggestionString = function (suggestion) {
        return suggestion.properties.name + ' (' + suggestion.properties.country + ')';
    };

    var selectSuggestion = function(elt) {
        inputEl.value = elt.innerHTML;
        selectedSuggestion = displayedSuggestions[elt['data-index']];
        suggestionsEl.style.display = 'none';
    };

    var display = function(data) {
        suggestionsEl.innerHTML = '';
        suggestionsEl.style.display = 'block';
        for (var i = 0; i < data.length; i++) {
            var suggestion = data[i];
            var li = document.createElement('li');
            li.className = "suggestion";
            li.onclick = function() {selectSuggestion(this);};
            li['data-index'] = i;
            li.innerHTML = suggestionString(suggestion);
            suggestionsEl.appendChild(li);
        }

    };

    Template.autocomplete.onRendered( function(){
        inputEl = this.find('.autocomplete-input');
        suggestionsEl = this.find('.suggestions');
    });

    Template.autocomplete.events({
        "keyup .autocomplete-input": function (event) {
            search(event.target.value);
        }
    });
})();

Two things does not seem very "meteor"-like. First, I have a lot of local functions, and local variables. Are they supposed to be put somewhere else?

Also, I am worried about performance. The keyup event is based on the class of the input. If it is looking through the DOM at each keystroke, it might get slow. I tried using inputEl instead, but this does not seem possible (or is it?).

\$\endgroup\$
1

1 Answer 1

1
\$\begingroup\$

The first thing I look for in an autocomplete widget is how the list is implemented. First, you are using inline styles to display/hide the list. You can simply use CSS :empty to hide the list container when nothing is listed. That way, you don't have to deal with toggling the visibility in JS. All you need to worry is if the list should be empty or not.

.suggestions:empty{
  display: none;
}

My Meteor is rusty, but I remember it had some form of reactive programming by way of session (correct me if I'm wrong) variables and templates. When these variables change, any UI bound to it changes. Suggesting you create an array and bind it to the UI. That way, all you have to care about is populating/unpopulating that array, and not mess around with innerHTML.

function(error, response) {
  if (error) {

  } else {
    displayedSuggestions = response.data.features;
    display(displayedSuggestions);
  }
}

// to

function(error, response){
  if(error) return; // throw new Error('nothing') or console.warn('')
  displayedSuggestions = response.data.features;
  display(displayedSuggestions);
}

So essentially, you're swallowing the error. That's fine, however you can simplify the code by returning early. On the other hand, a few debugging hints won't hurt. Suggesting you throw an error, or log it as a warning.

I see a timer, but it looks weird coupled with the nearby AJAX call. Then I found out it was a form of debouncing. Suggesting you move that function out into a more verbose form to avoid mistaking the timer as a form of asynchronous utility function and making the AJAX look like some synchronous operation. Something like:

function debounce(fn, delay){
  var timer = null;
  return function(){
    if(timer) clearTimeout(timer);
    setTimeout(fn, delay || 300);
  }
}

var search = debounce(function(){
  // Your AJAX code
});

In the above, search (or any function for that matter) will have automatic debounce capabilities once "wrapped" by debounce without the clutter that came with it.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot! I didn't know the debounce concept and :empty. As for using the Session to store the data and use it directly in the template... it made my code much shorter. \$\endgroup\$
    – Stilltorik
    Feb 7, 2016 at 17:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.