Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a cleanup method to Imager prototype #129

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fiznool
Copy link

@fiznool fiznool commented Dec 2, 2015

This provides a cleanup method for anybody using Imager in a SPA environment.

imager.cleanup()

The method removes any attached event listeners and ensures that the scroll debounce interval is cleared.

If this is acceptable I can look at adding documentation, etc if required.

@thom4parisot
Copy link
Contributor

Hey :-) thanks for this contribution!

What is the expected step after a cleanup? Deleting the imager instance of reusing it for other purposes?

Eventually if you could share a snippet of imager in the context of your SPA it would be helpful to determine what are the responsibilities of this method.

Thanks!

@fiznool
Copy link
Author

fiznool commented Dec 10, 2015

@oncletom Here's a snippet using Angular:

// imager.directive.js
angular
  .module('imager-test')
  .directive('imager', function($window, $timeout, $sce) {
    return {
      restrict: 'E',
      template:
      '<div class="delayed-image-load" data-src="{{:: src }}" data-alt="{{:: data.alt}}"></div>',
      replace: true,
      scope: {
        data: '=imagerData'
      },
      link: function(scope, element) {
        var Imager = $window.Imager;
        if(!Imager) {
          throw new Error('Imager.js must be loaded to use the <imager> directive.');
        }

        // Build the `src` variable from the imager data passed in
        scope.src = $sce.trustAsResourceUrl([
          scope.data.baseUrl,
          '/',
          scope.data.name,
          '-{width}{pixel_ratio}.',
          scope.data.extension
        ].join(''));

        $timeout(function() {
          // Give DOM time to repaint.
          var imager = new Imager(element, {
            availableWidths: scope.data.availableWidths,
            availablePixelRatios: scope.data.availablePixelRatios,
            onResize: true,
            lazyload: false
          });

          // Clean up our imager when the directive is removed from the DOM
          scope.$on('$destroy', function() {
            imager.cleanup();
          });
        });
      }
    };
  });
// app.controller.js
angular
  .module('imager-test')
  .controller('AppCtrl', function($scope) {
    $scope.imagerData = {
      baseUrl: 'http://example.com/path/to/images',
      name: 'logo',
      extension: 'png',
      alt: 'Logo',
      availableWidths: [200, 250, 300],
      availablePixelRatios: [1, 1.5, 2]
    };
  });
<div ng-controller="AppCtrl">
  <imager imager-data="imagerData"></imager>
</div>

So, the idea is, if the HTML snippet above is ever removed from the DOM, the scope.$on('$destroy') listener is fired, which cleans up the global events registered via the imager instance.

@thom4parisot
Copy link
Contributor

Oh OK I get it. Which means you have one instance of Imager per image?

@fiznool
Copy link
Author

fiznool commented Dec 10, 2015

Yes, that's right. I found it easier this way, in case different images have different availableWidths, availablePixelRatios, etc.

@fiznool
Copy link
Author

fiznool commented Jan 12, 2016

@oncletom I finally got round to adding some tests for this PR. Hopefully this is enough to be considered for a merge?

The Travis tests seem to be failing due to not having the credentials for BrowserStack, but when running npm test locally everything passes.

@fiznool
Copy link
Author

fiznool commented Jun 20, 2016

Bump... would love to see this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants