jQuery function to select user specified text within a div

Question

I'm very new to jQuery, and have recently converted one JavaScript function to jQuery. This function is part of a small utility that mimics click and drag for selection of text. Complete code is available on my github repo. Following is the screenshot of the utility.

The purpose of the code is to allow user to select text by entering the desired text in target text input, hitting the select button calls the getSelection function to select the target text within sentence div.

JavaScript

getSelection = function(){
  sentence = document.getElementById("sentence");
  target= document.getElementById("target");
  selection = window.getSelection();
  range = document.createRange();
  index = sentence.innerHTML.indexOf(target.value);
  range.setStart(sentence.firstChild,index);
  range.setEnd(sentence.firstChild,(index + target.value.length));
  selection.removeAllRanges();
  selection.addRange(range);
}

jQuery

$(document).ready(function(){
  $("#getSelection").click(function(){
    sentence = $("#sentence")[0];
    target= $("#target")[0];
    selection = window.getSelection();
    range = document.createRange();
    index = sentence.innerHTML.indexOf(target.value);
    range.setStart(sentence.firstChild,index);
    range.setEnd(sentence.firstChild,(index + target.value.length));
    selection.removeAllRanges();
    selection.addRange(range);
  });
});

Additional Info

Question: Having little knowledge of jQuery I'm concerned whether I'm doing everything right? Please let me know.

Answer 1

Current solution

DOM queries

Whenever you call getSelection you select the same two elements again and again. You can simply store them in variables for re-use outside of the function. Also you never use the element #sentence itself, so you can actually store its textNode:

var textNode = document.getElementById('sentence').firstChild,
    source = document.getElementById('source');

I also renamed target to source as it represents the "source" of the selection. But this might be arguable.

---

You can even store the selection and range in variables:

var selection = document.getSelection(),
    range = document.createRange();

Cases

You don't handle these cases:

• empty value or value not found
• case sensitivity in both the value and the text
• multiple occurrences of the value

If no value is given or no match is found, you should deselect any text. You could also present a message to the user:

if (0 === value.length || -1 === start) {
    selection.removeAllRanges();
    return;
}

Being case sensitive could be by design, but it would be great if I could type this and it would select the text This in the beginning of the sentence.

Your solution always selects the first occurrence of the string. Multiple ranges are only supported by Firefox and apparently by Edge, see MDN Selection.addRange(). So, this is by design. But it would be nice if the button's wording would reflect that:

Select first occurence

Getting text using innerHTML

You're using innerHTML to get the content of the div element. We have stored the textNode already. You can simply use nodeValue instead, as it is way faster than innerHTML in Webkit browsers. It's about the same in Firefox:

textNode.nodeValue.indexOf(value);

Updated function

Taking everything into account, this could be the final function:

getSelectionText = function() {
    var value = source.value.toLowerCase(),
        start = textNode.nodeValue.toLowerCase().indexOf(value);

    selection.removeAllRanges();

    if (0 === value.length || -1 === start) {
        return;
    }

    range.setStart(textNode, start);
    range.setEnd(textNode, start + value.length);

    selection.addRange(range);
}

jQuery

I'm not sure, why you're trying to rewrite this code using jQuery. Let's assume it's an exercise to get to know jQuery better.

The thing is, you're creating a lot of overhead, as you'll see next.

DOM queries

If you follow through and create jQuery objects of all DOM elements, the selectors will look like this:

var textNode = $("#sentence").contents().first(),
    source = $('#source');

jQuery optimizes selectors, so this should end in a call to getElementById(). But calling contents().first() is overhead to a simple .firstChild.

Getting text the jQuery way

Use .val() for form elements and .text() for other nodes:

var value  = source.val().toLowerCase(),
    start = textNode.text().toLowerCase().indexOf(value);

.text() wil definitely be slower than the native options discussed before.

Other changes

Well actually you have only one change left. You have to pass the real node to the range methods instead of the jQuery object:

range.setStart(textNode[0], start);
range.setEnd(textNode[0], start + value.length);

Adding everything into the anonymous function

I would not advise to place everything into the anonymous function of the event handler. What if you need to call the function from another place? Keep it as is and bind the function call, for example like:

$("#getSelection").click(getSelectionText);

Updated function

getSelectionText = function() {
    var value  = source.val().toLowerCase(),
        start = textNode.text().toLowerCase().indexOf(value);

    selection.removeAllRanges();

    if (0 === value.length || -1 === start) {
        return;
    }

    range.setStart(textNode[0], start);
    range.setEnd(textNode[0], start + value.length);

    selection.addRange(range);
}

Final thoughts

I guess you can see, that there's no actual benefit from switching to jQuery for this particular task:

• The code is not getting easier to read or maintain.
• You create a lot of overhead.
• You still have to access the contained native DOM element often.
• You have additional weight of 87 KB for the compressed library.