Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This project has evolved a lot since I started learning Angular. I'm pretty happy with it now, and I'm wondering if there's anything I can improve upon, or have forgotten at this point...

HTML

<h1 data-fittext>FitText</h1>

<h1 data-fittext=".315" data-fittext-min="12" data-fittext-max="50">ng-FitText</h1>

Inclusion & Customization

var myApp = angular.module( 'myApp', [ 'ngFitText' ] );

myApp.config( function( fitTextConfigProvider ) {
    fitTextConfigProvider.config = {
        debounce: true, //default is false
        delay: 1000 //default is 250
    };

    // OR

    fitTextConfigProvider.config.debounce = true;
    fitTextConfigProvider.config.delay = 1000;
});

The Module

'use strict';

angular.module( 'ngFitText', [] )
    .value( 'config', {
        'debounce': false,
        'delay': 250
    })

    .directive( 'fittext', [ 'config', 'fitTextConfig', function( config, fitTextConfig ) {
        return {
            restrict: 'A',
            scope: true,
            transclude: true,
            replace: true,
            template: function( element, attrs ) {
                var tag = element[0].nodeName;
                return "<"+tag+" data-ng-transclude data-ng-style='{fontSize:fontSize}'></"+tag+">";
            },
            link: function( scope, element, attrs ) {
                angular.extend(config, fitTextConfig.config);

                scope.compressor = attrs.fittext || 1;
                scope.minFontSize = attrs.fittextMin || Number.NEGATIVE_INFINITY;
                scope.maxFontSize = attrs.fittextMax || Number.POSITIVE_INFINITY;
                scope.elementWidth = element[0].offsetWidth;

                ( scope.resizer = function() {
                    scope.elementWidth = element[0].offsetWidth;
                    scope.fontSize = Math.max(
                        Math.min(
                            scope.elementWidth / ( scope.compressor * 10 ),
                            parseFloat( scope.maxFontSize )
                        ),
                        parseFloat( scope.minFontSize )
                    ) + 'px';

                    if( !scope.$$phase ) scope.$digest();
                })();

                config.debounce == true
                    ? angular.element( window ).bind( 'resize', debounce( scope.resizer, config.delay ))
                    : angular.element( window ).bind( 'resize', scope.resizer);

                function debounce(a,b,c){var d;return function(){var e=this,f=arguments;clearTimeout(d),d=setTimeout(function(){d=null,c||a.apply(e,f)},b),c&&!d&&a.apply(e,f)}}
            }
        }
    }])

    .provider( 'fitTextConfig', function() {
        var self = this;
        this.config = {};
        this.$get = function() {
            var extend = {};
            extend.config = self.config;
            return extend;
        };
        return this;
    });
share|improve this question
add comment

1 Answer

  • As personal opinion, I wouldn't use config as name for .value, it sounds too generic and can be confused with .config method of the module API.

  • Putting comments like //default is false looks confusing - why are these not default? Also it is easy to change the defaults and forget to adjust comments. Uncle Bob recommends to minimize comments and use expressive code with descriptive names instead.

  • function debounce(a,b,c) is great for minimizers but cryptic for code readers

  • In my view, using .provider is possibly an overkill as there is not much configuration logic here.

  • The last two comments here also apply. Instead the config JSON can be kept inside .value and decoupled from the provider.

share|improve this answer
    
Thanks for your points. The debounce function is taken from Underscore and stated as such in the module comments. Overkill doesn't concern me, but is there another way to extend configuration? As far as I'm concerned it's not a configurable module if config settings are blown away upon updating the module... –  Patrick Apr 22 at 17:00
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.