2

I'm working on a project which contains a search form which looks like this:

enter image description here

Here is the HTML (actually, a Django template):

<form action="{% url action %}" method="get" class="left search col s6 hide-on-small-and-down" novalidate>
  <div class="input-field">
    <input id="search" placeholder="{{ placeholder }}"
        autocomplete="off" type="search" name="q"
        value="{{ search_form.q.value.strip|default:'' }}"
        data-query="{{ search_form.q.value.strip|default:'' }}">
    <label for="search" class="active"><i class="material-icons search-icon">search</i></label>
    <i data-behavior="search-clear"
        class="material-icons search-icon"
        {% if not search_form.q.value %}style="display: none;"{% endif %}>close</i>
  </div>
</form>

Here is the concomitant jQuery code:

  $(document).on('click', '[data-behavior="search-clear"]', function(e) {
    $('#search').val('').parents('form').submit();
  });

  $(function() {
    setSearchClearVisibility($('#search'));
  });

  $(document).on('keydown', '#search', function(e) {
    var $el = $(this);

    setTimeout(function() {
      setSearchClearVisibility($el);
    }, 10);
  });

  function setSearchClearVisibility(el) {
    if (el.val() === '') {
      $('[data-behavior="search-clear"]').hide();
    } else {
      $('[data-behavior="search-clear"]').show();
    }
  }

  $(document).on('keydown', '#search', function(e) {
    var $el = $(this);

    setTimeout(function() {
      if (e.keyCode !== 27) return;

      $('#search').val('').blur();
      setSearchClearVisibility($el);

      if ($el.data('query') !== '') {
        $('#search').parents('form').submit();
      }
    }, 10);
  });

I have a couple of questions about the jQuery code:

Firstly, it seems like the setSearchClearVisibility() function could be refactored as follows:

  function setSearchClearVisibility(el) {
    if (el.val() === '') {
      $('[data-behavior="search-clear"]').toggle();
    }
  }

Is this indeed equivalent?

Secondly, it would also seem that the line

$(document).on('keydown', '#search', function(e) {

could be replaced by

$('#search').keydown(function(e) {

or not?

Thirdly, I'm not sure why there is a 10-ms setTimeout in a couple of places. Why not perform the action immediately?

Fourthly, I'm puzzled by the line

if (e.keyCode !== 27) return;

The key code 27 corresponds to the Escape key; would this not mean that if the key pressed is not the Escape key, the function would return and the code below it (the toggling of the close icon visibility and submission of the form) would not be executed? I also don't notice any difference in behavior if I comment out this line.

Kurt Peek
  • 52,165
  • 91
  • 301
  • 526
  • 2
    The proposed change for `setSearchClearVisibility` assumes the display state of the element will always match the value being blank or not. The existing logic does not make this assumption and explicitly sets it to what it should be, so the original version is more explicit and safe. – Taplar May 04 '18 at 20:46
  • 2
    `$('#search').keydown(function(e) {` is not equivalent to the existing version. The existing version is a delegate event binding, while the keydown is not. You should read up on delegate event handlers. http://learn.jquery.com/events/event-delegation/ – Taplar May 04 '18 at 20:47

1 Answers1

4

1. No, they're not equivalent.

The code you propose will toggle (change from existing state to the other) if the condition is true and will not perform any change if the condition is not met.
The code you have now is: hide if condition, show if not.

Passing a condition to toggle will show if true and hide if not. So this is equivalent:

function setSearchClearVisibility(el) {
  $('[data-behavior="search-clear"]').toggle(el.val() !== '');
}

2. $(parent).on(event, child, callback) is not the same as $(child).on(event, callback)

And this is really important.
The first only binds once, on the parent for all existing or future children meeting the child selector. The evaluation (of child selector) is done when event happens.
The second binds once per child. So if you have 1k children, you have 1k bindings, instead of one. And the evaluation of selector is done at the time of binding, not of the event. So if you add more children after the binding, they won't have the callback bound on event.

3. setTimeout(callback, delay) is to queue the execution of callback at the latest between:

  • end of current execution queue
  • current time + delay

See this answer for details. setTimeout(callback) is a typical way to queue execution of callback at the end of the execution queue. It ensures current function/scope/thread finishes execution before callback is run.

4. if (e.keyCode !== 27) return; means:

if the pressed key was not escape, exit function. Else, proceed and run what's after me (till end of function). Please note this function only gets executed on keydown event on #search. So, to test it, you'd have to have #search focused/active/selected and press a key. If the key pressed is escape, the code after the condition will get executed. If it's any other key, it won't.

tao
  • 82,996
  • 16
  • 114
  • 150