/**
 * Strip extraneous information ("px") from a given value in preparation
 * for getPositionProperties().
 */
Drupal.edit.util.stripPX = function(value) {
  if (value) {
    var index = value.indexOf('px');
    if (index === -1) {
      return NaN;
    }
    else {
      return Number(value.substring(0, index));
    }
  }
  else {
    return NaN;
  }
};

Feels a lot like you're trying to do what the native parseFloat does.

Also, it's generally advised to not use primitive constructors like Number() as they cause unnecessary overhead and can sometimes lead to getting a different type than you expected.

Attached patch removes the stripPX method and replaces all occurrences with parseFloat.

CommentFileSizeAuthor
edit-parsefloat.patch2.25 KBseutje
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

seutje’s picture

Argumentation: http://jsperf.com/constructor-vs-parse
Note how parseFloat is slightly slower in chrome (8%), but the stripPX function is far slower in FF (64%) and IE (54%)

General argumentation for constructors vs literal notation: http://jsperf.com/new-array

Wim Leers’s picture

Assigned: Unassigned » Wim Leers
Status: Active » Reviewed & tested by the community

I didn't write this particular piece of code (@prestonso) did.

That being said, this looks like an excellent improvement — thanks! :) Will commit ASAP.

Wim Leers’s picture

Title: Did you mean parseFloat? » Remove Drupal.edit.util.stripPX() in favor of parseFloat().
Wim Leers’s picture

Status: Reviewed & tested by the community » Fixed

Committed just in time for the DrupalCon demo! :)

Thanks, Steve :)

http://drupalcode.org/project/edit.git/commit/ffb3ce3

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.