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

Check highlight option in mouseover() instead of highlight(). #191

Closed
wants to merge 2 commits into from

Conversation

macfreek
Copy link

@macfreek macfreek commented Mar 2, 2014

See issue #190.

This allow external methods to control the highlight status, just as it is possible to set isSelectable to false, and control the selection status from external methods.
Effectively, mouseover() and mouseout() no long call the highlight() and clearEffects() methods if the highlight option is false. Also, the highlight() method now highlights the area, regardless of the highlight option.

Note: I did not add the build output (dist/jquery.imagemapster.js and dist/jquery.imagemapster.min.js). Part of the src in the original repository has MS-DOS line endings, some UNIX line endings, and rake can't handle that, leading to spurious lines in the diff.

This allow external methods to control the highlight status, just as it is possible to set isSelectable to false, and control the selection status from external methods.
Effectively, mouseover() and mouseout() no long call the highlight() and clearEffects() methods if the highlight option is false. Also, the highlight() method now highlights the area, regardless of the highlight option.
See jamietre#190.
Enforcing MS-DOS line-breaks.
@techfg
Copy link
Collaborator

techfg commented Jan 26, 2021

Hello @macfreek -

Thank you for your contribution, apologize for the long delay in responding!

I've taken a look at #190 where you describe your objective with this PR. The first thing that comes to mind is that clearEffects also is responsible for closing tooltips, etc. so highlight isn't the only consideration here.

With that said, happy to review this more closely. If you still have an interest in having this considered, please let me know and also, if you could, update your fiddles. I was able to make some adjustments based on what I thought you were trying to do but would much rather have you update them to be exactly what you were after so I don't misunderstand anything.

Let me know your thoughts, thanks again!!

@macfreek
Copy link
Author

Hi @techfg
Thanks for getting back to me! Unfortunately, it is a been a while since I last looked at ImageMaspster, so I would have to dive into this issue from scratch again.

I suggest to close the PR.

To anyone who comes across the same issue, feel free to re-use any part of the code that may still be relevant. I hereby release any part of my contribution in this PR as well as in the above Fiddles as public domain and/or creative commons zero (at your choice).

@techfg
Copy link
Collaborator

techfg commented Jan 27, 2021

Hi @macfreek -

Thanks for getting back to me on this so quickly and again, apologies for the long delay in getting back to you on this PR.

Per your comments, I'll go-ahead and close. Appreciate you releasing your contribution to the community for anyone that it might be helpful to. If you do decide to revisit the PR, please just let us know!

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