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

Add duScrollDelay option for smoothScroll directive #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

homerjam
Copy link
Contributor

This is useful when you have an additional click event bound to the same element as the smoothScroll directive that should trigger an event before triggering the scroll.

@oblador
Copy link
Owner

oblador commented Nov 19, 2015

Thanks for your PR @homerjam!

Wouldn't it make more sense to have the other click handlers to invoke the smooth scroll via scrollToElementAnimated() in that case? Don't want to unnecessarily bloat the API and configs.

@homerjam
Copy link
Contributor Author

Well yes that's an option of course, but I personally thought the bloat was worth it in this case.

No problem if you don't want to include it though. I can make a custom directive derived from yours to use in my projects.

Thanks for the great module anyway!

@oblador
Copy link
Owner

oblador commented Nov 19, 2015

I think it's an edge case that pretty easily can be done in another way that's why :-) Leaving this PR open for a while to see if anybody else has any need for it.

@homerjam
Copy link
Contributor Author

Good plan, consider me a +1 : )

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.

2 participants