Add classname to the returned element about react-click-away-listener HOT 8 CLOSED

I'm not sure I see a problem with that. Besides, if I decide to add one now, it could break some people's codebase as the class name specified could already be targeted.

I think if you need one, you could just add it to the ClickAway listeners.

If there's something I'm missing, please let me know 🙏

from react-click-away-listener.

I don't think adding a class name to the component would be a breaking change. If you make it an optional prop and use the package classnames you can still pass their class name to the component too. I think it is a pretty sane default for the component to have a class name though.

from react-click-away-listener.

Nah, something could go wrong with that, as I mentioned earlier that it could already be targeted. We don't have a metric to work with, and there's a slight chance a developer might have used that same class name just to style the div.

Besides, I don't buy the idea of adding the classnames library just to solve a specific problem that could be solved in alternative ways.

One way I could think of right now is by using data attributes. We could add a data-click-away-listener to the base. What do you think?

from react-click-away-listener.

It wouldn't have any adverse effects. The classnames library already solves that problem by purging duplicate class names as shown here by using a dedupe method.

Personally, the better option would be to not make it a div at all. You could have the functionality attached to the child by using React.cloneElement. You would also cease to stuffing the DOM with additional elements that way. It doesn't really make sense to render something that is just checking if a Mouse or Touch event is outside of its child's bounds. It's not like you are going to style a ClickOutsideListener. It is a pure functionality component.

from react-click-away-listener.

One of the biggest issues by not doing cloneElement is that all of the event listeners are hi-jacked by your click listener. If I am making a dropdown or modal component that has click or keydown React synthetic events that it has to listen to, they would need to be applied to your click listener and not the modal or dropdown component. This would be a major breaking change though. Which means you would have to bump the major version up.

from react-click-away-listener.

I definitely agree with you on using cloneElement instead 💪

from react-click-away-listener.

I just opened a PR for it here #28. You can review when you have the time.

from react-click-away-listener.

We can close this since we've released a new version that uses cloneElement.

from react-click-away-listener.