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

fix(dropdownToggle): Dropdown not repositioned when window resized #261

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

Conversation

lerio
Copy link

@lerio lerio commented Sep 18, 2015

Related to #258.

When window is resized, the open dropdown is not correctly repositioned below its toggle. Now the behaviour is correct either when the dropdown is left or right aligned.

Before:
screen shot 2015-09-18 at 14 37 26

After:
screen shot 2015-09-18 at 14 38 00

@@ -90,6 +90,16 @@ angular.module('mm.foundation.dropdownToggle', [ 'mm.foundation.position', 'mm.f
}
};
$document.on('click', closeMenu);

angular.element($window).bind('resize', function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you use bind() here instead of on() like the rest of the event listeners?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason for bind(), changing.
Also, added cleanup on $destroy as suggested.

@jbrowning
Copy link
Member

Thanks for sending this in, @lerio. In addition to my previous comments can you please try to add some tests for this behavior?

@lerio
Copy link
Author

lerio commented Oct 14, 2015

Did the required changes, but I didn't add any test yet. Can you suggest me a way to test $window resize on jasmine?

@danieljin
Copy link

Bump

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

Successfully merging this pull request may close these issues.

3 participants