While working on adding some features to this repository, I found that the code wasn't very well organized in all places and it was very difficult to read at times.
While this PR appears to be rather large, it's mostly just moving things around and not changing much of the syntax (while some things have changed slightly).
Move all variable declarations to the top of the scoped method, in this case, the link function.
Move all anonymous functions to the top of the scoped method, so that they are defined before being called.
Sort any list of variables and/or functions in alphabetical order.
Use strict comparisons rather than loose truthy or falsey style, e.g. (true === options.hideOnInsideClick).
More comments.
While reorganizing, I also took the opportunity to be more mindful of event binding. For example, this block of code caches the unregister method of scope.$on or $rootScope.$on, and calls it during a $destroy so we can make sure this component is cleaning up after itself.
Also, prior to this update, the mouseover and mouseout events were being bound to both the $popover and elm, regardless of popover state. So, even though there was no popover being shown, there were 4 events bound to the popover target element.
I have updated it so that when you display a popover, we add the respective mouseover and mouseout events, and when that popover is hidden, we remove them.
This ensures that the 4 mouseover and mouseout events are only active and executing during the period of time that the popover is being displayed. I don't think this would make a huge difference in performance, but I think cleaning up after yourself like this is a better practice.
Again, the size of the PR itself may be misleading as most of this code is just being moved around and not actually being changed very much.
While working on adding some features to this repository, I found that the code wasn't very well organized in all places and it was very difficult to read at times.
While this PR appears to be rather large, it's mostly just moving things around and not changing much of the syntax (while some things have changed slightly).
link
function.truthy
orfalsey
style, e.g.(true === options.hideOnInsideClick)
.While reorganizing, I also took the opportunity to be more mindful of event binding. For example, this block of code caches the unregister method of
scope.$on
or$rootScope.$on
, and calls it during a$destroy
so we can make sure this component is cleaning up after itself.Also, prior to this update, the
mouseover
andmouseout
events were being bound to both the$popover
andelm
, regardless of popover state. So, even though there was no popover being shown, there were 4 events bound to the popover target element.I have updated it so that when you display a popover, we add the respective
mouseover
andmouseout
events, and when that popover is hidden, we remove them.This ensures that the 4
mouseover
andmouseout
events are only active and executing during the period of time that the popover is being displayed. I don't think this would make a huge difference in performance, but I think cleaning up after yourself like this is a better practice.Again, the size of the PR itself may be misleading as most of this code is just being moved around and not actually being changed very much.