Refactor to simplify supporting new CSS props; add support for variable character width & letter-spacing

[tinymachine]

Okay, here's a cleaner PR than my last attempt. Again, this PR is pretty far-reaching, so I totally understand if you can't accept it. Just thought you might be interested in some aspects of how I approached these changes.

Major change: apply defaults, calculate and adjust property values programmatically

The biggest update here is in 887d31de69fbd322af3f74755904c07110b1349e, which refactors how CSS properties are handled by the script. Instead of 'manually' handling all calculations (with separate lines of code for applying defaults for each property, and calculating and applying each property value), all supported properties and their defaults are defined in a config var. Then defaults are applied for all properties included in the config, and all calculations are made and style changes are applied programmatically.

This way when you want to support additional CSS properties, you just update the config, without having to worry about making additional per-property updates throughout the script. For example, adding letter-spacing support to the script is now as simple as adding the following to config.supportedProps:

{
  propName: 'letterSpacing',
  unitsDefault: 'em'
}

Adding per-property defaults for minWidth and maxWidth is supported as well. Here's the entire config for all five props supported in the PR:

supportedProps: [
  {
    propName: 'fontSize',
    minWidthDefault: 1.0,
    maxWidthDefault: 1.8,
    unitsDefault: 'em'
  },
  {
    propName: 'fontStretch',
    unitsDefault: '%'
  },
  {
    propName: 'fontWeight'
  },
  {
    propName: 'letterSpacing',
    unitsDefault: 'em'
  },
  {
    propName: 'lineHeight',
    minWidthDefault: 1.33,
    maxWidthDefault: 1.25
  }
]

Limitations

1. Parameter naming restriction: each supported property must have a propName that matches the JS equivalents for CSS properties. The script assumes the propName is what's used by the API params, so the letter-spacing params would (as currently coded) have to be min/maxWidthLetterSpacing (the script is aware of the initial cap on the property name) and letterSpacingUnits. Params for changing character width would (as currently coded) have to be min/maxWidthFontStretch and fontStretchUnits. This could be changed pretty easily to handle param names that don't necessarily map directly to their respective CSS properties, but it might be a good thing to enforce a standard style on the params and to make explicit exactly what the params control.

2. No support for special CSS properties: properties like font-variation-settings, which can control multiple properties at once, aren't currently supported. This is probably fine, though, since the VF variation axes users will probably generally want to control (and the ones more variable fonts support) would be more standardized variation axes like weight and sometimes width, which honor their own respective CSS properties (e.g. font-weight and font-stretch).

Other updates

• Deprecated min/maxWidthVariableGrade in favor of min/maxWidthFontWeight (closes #29) (also related to limitation 1 above)
• Replaced use of font-variation-settings for weight with font-weight so the cascade is better respected (see 661cf6c3a9afcf84f82437d65e8af03fc03bb33f's commit message for details) (also related to limitation 2 above)
• Added support for per-property units; replaced units with fontSizeUnits (but plain units is still honored) (closes #23)
• Replaced date with version number in textblock.min.js (closes #31)
• Added support for variable font-width and letter-spacing (closes #22 and #28)
• Improved grunt compression (though this update contains a little more code than previous versions, the minified script weighs in at only 1.88 KB, up only slightly from 1.84 KB as of release 0.10.0).

Backward compatibility

This PR is backward-compatible with previous versions with the exception of dropping support for min/maxWidthVariableGrade (and min/maxVariableGrade) in favor of min/maxWidthFontWeight (though supporting it could be added fairly easily).

Questions

• This PR makes the script much easier to update to add support for new CSS properties, but I'm not sure how the script's runtime performance has been affected. If you're aware of how to test performance/efficiency, I'd love to hear about it. (I stumbled across jsPerf, but haven't yet figured out how to set it up for testing Textblock.)

[theorosendorf]

@tinymachine — whoa, this is amazing!

I only skimmed but really looks like the way to approach it. I won't be able to take a proper look for a while. I'll request @traviskochel and @jessevdp in case they’re available to pop in and see.

Thanks Mihira!

[tinymachine]

My pleasure, @theorosendorf!

[traviskochel]

Haven't tested it, but it looks good to me. nice work @tinymachine!

[tinymachine]

Thanks, @traviskochel — you and @theorosendorf both do really beautiful and inspiring work. I'm frankly humbled to be able to contribute to this project.

[theorosendorf]

@tinymachine — This PR also takes care of #34 since formatting becomes customizable, right? I was able to change min/maxWidth to min/maxWidth_ without breaking the demo. Hope you don't mind, I commited that test... :)

[theorosendorf]

I ran Chrome’s performance monitor and was able to ramp up the CPU usage to 100%, although to do that I had to frantically resize the viewport like a crazed meth addict.

[theorosendorf]

Humbled to have you here @tinymachine.

[tinymachine]

@tinymachine — This PR also takes care of #34 since formatting becomes customizable, right? I was able to change min/maxWidth to min/maxWidth_ without breaking the demo. Hope you don't mind, I commited that test... :)

I don't mind at all — please feel free to make changes as you see fit! I hadn't considered this PR to close #34 since it didn't actually change the param formatting, but your commit does the trick.

I ran Chrome’s performance monitor and was able to ramp up the CPU usage to 100%, although to do that I had to frantically resize the viewport like a crazed meth addict.

Ha — yeah, not sure if it's warranted but I wonder if it might make sense to throttle, or maybe throttle and debounce, the script and see if it would bring that CPU usage down. (I set up a test on jsPerf but haven't figured out how to get reliable results. I switched up the ordering of which version of the script to test first, and the first script always performed much better than the others, and with a huge margin of error.)

Humbled to have you here @tinymachine.

🙏

[tinymachine]

@theorosendorf, I just tried some crude benchmarking by changing the run(); line in the onDocReady() block of the script to the following to get it to run 10k times:

    var reps = 10000;
    var startTime = Date.now();
    for (var i = 0; i < reps; i++) {
      run();
    }
    var endTime = Date.now();
    console.log(reps + ' executions took ' + (endTime - startTime) / 1000 + ' seconds');

Somewhat consistently I'm seeing a time of ~3.4s for this PR, and ~0.8s for 0.10.0 when I try multiple runs in Safari and Chrome. So though this PR makes it much easier to make changes to the params, etc., it might result in ~4x slower performance 😬. I'll look into this further but wanted to give a heads-up...

[theorosendorf]

I really love the idea of this PR. Are there any tools we can use to better understand the performance hit?

[theorosendorf]

You know, because this would squash a ton of current and future features...