3
\$\begingroup\$

I required a trigger, whenever the window text selection cleared. Finally I found this approach.

Using the same id, I have added multiple event handlers, but I am not sure my work is the best.

Is there a better way to do this?

 $('#tenderContent').on('mouseup', '.tenderdocument', function (e) {
    e.stopPropagation();
    if ($.trim(window.getSelection().toString()).length) {
        $('#text').text(window.getSelection().toString());
    }

});

$('#tenderContent').on('mousemove', '.tenderdocument', function (e) {
    e.stopPropagation();
    if (!$.trim(window.getSelection().toString()).length) {
         $('#text').empty();
    }
});

LiveDemo

\$\endgroup\$
3
  • \$\begingroup\$ Soo... this code works as it should? \$\endgroup\$
    – Vogel612
    Commented Jan 23, 2015 at 10:40
  • \$\begingroup\$ in the jsfiddle i placed the sample code. that's not integrated. but my app, i did not get any issue. still the code may not such good! \$\endgroup\$
    – 3gwebtrain
    Commented Jan 23, 2015 at 11:03
  • 1
    \$\begingroup\$ Well for one it doesn't work when I change the selection without my mouse :( \$\endgroup\$
    – Vogel612
    Commented Jan 23, 2015 at 11:04

1 Answer 1

2
\$\begingroup\$

The jQuery $.on(); event listener can use 2+ events in its event listener parameter and then let you specify which one through e.type. This allows us to shorten a lot of code. With that in mind, we can also shorten your $.trim(window.getSelection().toString()).length to be $.trim('' + window.getSelection()) and assign it to a variable for use in our if's. We can then further simplify it by testing for e.type being 'mouseup' and if it's evaluates to false, it has to be 'mousemove'. And so we don't have nested if's, we can just use logical AND and OR operators to do something if it even has a value, and something else if it doesn't. That brings your code down to this:

$('#tenderContent').on('mouseup mousemove', '.tenderdocument', function(e) {
  e.stopPropagation();
  var selection = $.trim('' + window.getSelection());
  if ('mouseup' == e.type) {
    selection && $('#text').text(selection);
  } else {
    selection || $('#text').empty();
  }
});

That is a major cut off the amount that you had before. It also simplifies a lot of stuff to be fast and efficient.

\$\endgroup\$
2
  • \$\begingroup\$ Nice suggestion. It's good to highlight the changes you've made clearly. You explained that it was shorter, but it can be worth pointing out how you can put 'mouseup' and 'mousemove' together to make it the one function. \$\endgroup\$ Commented Oct 13, 2015 at 16:11
  • 1
    \$\begingroup\$ I just edited it lol. :( We'll just have to see if it gets peer reviewed then. :/ \$\endgroup\$ Commented Oct 13, 2015 at 17:27

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.