Pomax
allow multiple ignore classes
Only allowing a single ignore class has two issues:
- Forces you to pollute your entire app with that ignore class. You should be able to declare at the site of the target component all the classes you want to blacklist.
- In many cases there are plugin architectures where there are elements whose classes you can't control (e.g. popovers) that you still want ignored.
Cool - generalized containment function definitely makes sense. Should I try to make the above changes to get this merged or should we just wait on your refactor? The current PR is probably good for people who want the new functionality without sacrificing back-compat
@hzuo Im still not sure if I want to merge this, requested changes after code review are really rather cosmetic, so you might try to adjust the code accordingly. No promises about merging it though, but I won't say definite "no" about this at the moment.
@hzuo sorry for a delay - I've created a PR i was planning to do for quite some time. I'd love your input there - https://github.com/Pomax/react-onclickoutside/pull/246
Hey just to clarify is this PR blocked because of #246? I'd be interested in this feature :)
@augbog yeah, i need to finish the #246 (wanna help with that? 😉 ), but got occupied with tons of other OSS work and somehow never got back to it :s
I would prefer not to land this PR in favour of a containment function, but I would also advocate adding that containment function independent of any overhaul, so just as a simple update that adds the containment functionality while still allowing the ignore class, with an update to the documentation that the ignore class is now deprecated but still supported for the time being.
As long as the API stays the same across both changes, no one will notice the rug got replaced (rather than pulled out from) under their feet.
