patrickmarabeas / ng-ScrollSpy.js

An AngularJS module for navigation highlighting
http://patrickmarabeas.github.io/ng-ScrollSpy.js
MIT License
26 stars 9 forks source link

Spies defined "async" are not registered until windows resize #7

Closed OClement closed 10 years ago

OClement commented 10 years ago

When the Spy directive is used inside a template that's loaded async', the setup function is fired the first time with no spies being registered.

This happens where navigation is loaded through an ng-include or directive's templateUrl for instance.

The Spies get pushed in the service after the setup function was fired;

Couldn't the Setup logic be handled by each instances of the spy directive instead?

patrickmarabeas commented 10 years ago

Yes, it does need a rewrite - first on my list when I have some time.

You should be able to resolve this issue by adding an additional MutationObserver to watch the navigation container for changes.

OClement commented 10 years ago

I don't know the MutationObserver Api's unfortunately, and don't have a lot of time on my hands at the moment.

I think I'll just "hack my way through it" for the moment and wait for a propper fix, or try to fix this propperly when I have time, whichever comes first.

I'll let you know if I do of course

Thanks for your quick reply

patrickmarabeas commented 10 years ago

https://github.com/patrickmarabeas/ng-ScrollSpy.js/blob/master/ng-ScrollSpy.js#L114

Stick the following outside the for loop. May need to change the var names.

var target = document.getElementById( /* the static container that async nav items are inserted into */ );

var config = {
    attributes: true,
    childList: true,
    characterData: true,
    subtree: true
};
var observer = new MutationObserver(function(mutations) {
    mutations.forEach(function(mutation) {
        setup();
        determiner();
    });
}).observe(target, config);

I haven't tested this, but it should do the trick. Any changes to the DOM element will instigate the setup and determiner functions. You can reduce what changes to look for by altering the config object.

patrickmarabeas commented 10 years ago

You got me to free up some time : P

If you wouldn't mind testing something for me, comment out the ngScrollSpy module and replace it with the following code. Use data-scrollspy-listen where data-spy was (use the same value data-spy was set to), and place data-scrollspy-broadcast (no value) on the content elements on the page you want to watch.

Does this work as expected (clicking the element to symbolize scrolling to it) in your use case?


angular.module( 'ngScrollSpy', [] )

    .directive( 'scrollspyBroadcast', [ '$rootScope', function( $rootScope ) {
        return {
            restrict: 'A',
            scope: {},
            link: function( scope, element, attrs ) {

                angular.element( element ).bind( 'click', function() {

                    console.log('clicked me');

                    $rootScope.$broadcast( 'spied', {
                        'activeSpy': attrs.id
                    });
                });

            }
        }
    }])

    .directive( 'scrollspyListen', [ '$rootScope', function( $rootScope ) {
        return {
            restrict: 'A',
            scope: {
                scrollspyListen: '@',
                enabled: '@'
            },
            replace: true,
            transclude: true,
            template: function( element, attrs ) {
                var tag = element[0].nodeName;
                return '<'+tag+' data-ng-transclude data-ng-class="{active: enabled}"></'+tag+'>';
            },
            link: function( scope, element, attrs ) {

                $rootScope.$on('spied', function(event, args){

                    scope.enabled = false;

                    if( scope.scrollspyListen === args.activeSpy ) {
//                      console.log( scope.scrollspyListen + ' -> ' + args.activeSpy );
                        scope.enabled = true;
                    }

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

                });

            }
        }
    }]);
patrickmarabeas commented 10 years ago

Module has been completely updated. Give it a run and let me know...

OClement commented 10 years ago

Seems to work fine at first try Got a small glitch but is most likely due to my own code/layout being a bit particular; will look into that

Cheers

OClement commented 10 years ago

Ok turns out the "glitch" was caused by the triggerOffset="150' hardcoded in the broadcast directive It was preventing the first nav item not to be triggered

Again, my layout is a bit unortodox, might be an edge case

Setting it to 0 fixes it for me and all seems to work as expected; I guess scope.tiggerOffset should be parametrable

Thanks for your work!

patrickmarabeas commented 10 years ago

Awesome! There are some improvements that can be made, just wanted to get a proper usable product out there for now.