8000 Add onActivate function by chdanielmueller · Pull Request #6 · sidsbrmnn/scrollspy · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add onActivate function #6

8000 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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 4, 2021
Merged

Add onActivate function #6

merged 4 commits into from
Oct 4, 2021

Conversation

chdanielmueller
Copy link
Contributor

I did add the functionality for an onActivate function.
It will be triggered when a menuItem gets activated.

I also did change the isActive comparison to string, because isActive was always false if I had more than one activeClass (like the sample)

sidsbrmnn
sidsbrmnn approved these changes Oct 4, 2021
View reviewed changes
Copy link
Owner
@sidsbrmnn sidsbrmnn left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for contributing mate

@sidsbrmnn sidsbrmnn merged commit 1c48281 into sidsbrmnn:master Oct 4, 2021
@chdanielmueller
Copy link
Contributor Author

@sidsbrmnn Sure thing.
Any plans on publishing it on npm?

@sidsbrmnn
Copy link
Owner

@sidsbrmnn Sure thing. Any plans on publishing it on npm?

Yeah. This will go on NPM by the end of the week. There are a couple of things I need to add. Thanks for being patient

@chdanielmueller
Copy link
Contributor Author

Hi @sidsbrmnn
Do you still plan to publish these changes?
Thanks for your feedback

@sidsbrmnn
Copy link
Owner

Hi @sidsbrmnn Do you still plan to publish these changes? Thanks for your feedback

I've renamed the callback as onActive for semantic reasons and is now available in the current release

@chdanielmueller
Copy link
Contributor Author

Thanks for the release.
I found an error in the docs regarding onActive which I fixed.
Please see #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sidsbrmnn sidsbrmnn sidsbrmnn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chdanielmueller @sidsbrmnn
0