patternjavascriptMinor
jQuery function to select user specified text within a div
Viewed 0 times
usertextdivfunctionwithinjqueryselectspecified
Problem
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
JavaScript
jQuery
Additional Info
Question: Having little knowledge of jQuery I'm concerned whether I'm doing everything right? Please let me know.
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.
Solution
Current solution
DOM queries
Whenever you call
I also renamed
You can even store the selection and range in variables:
Cases
You don't handle these cases:
If no value is given or no match is found, you should deselect any text. You could also present a message to the user:
Being case sensitive could be by design, but it would be great if I could type
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
You're using
Updated function
Taking everything into account, this could be the final function:
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:
jQuery optimizes selectors, so this should end in a call to
Getting text the jQuery way
Use
Other changes
Well actually you have only one change left. You have to pass the real node to the
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:
Updated function
Final thoughts
I guess you can see, that there's no actual benefit from switching to jQuery for this particular task:
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
innerHTMLYou'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.
Code Snippets
var textNode = document.getElementById('sentence').firstChild,
source = document.getElementById('source');var selection = document.getSelection(),
range = document.createRange();if (0 === value.length || -1 === start) {
selection.removeAllRanges();
return;
}textNode.nodeValue.indexOf(value);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);
}Context
StackExchange Code Review Q#158306, answer score: 3
Revisions (0)
No revisions yet.