gogoair / react-combo-select

React dropdown for select and multiselect
MIT License
28 stars 9 forks source link

Fixes #49 fix console error by letting event bubble to helper function #50

Open quangta93 opened 5 years ago

MilosRasic commented 5 years ago

Thanks for the PR @quangta93. Good catch, but the solution doesn't eliminate the core of the issue, so it fixes the problem with clicking on the dropdown icon but possibly breaks another use case: custom handling of dropdown item click. The reason why it works is removing the event.stopPropagation() call ReactSvgIcon component. The event will now bubble up but if a custom handleClick() is passed it will never get called.

I think a simple solution here would be to add a guard to onClick() function in ReactSvgIcon instead of removing it. Something like:

    const onClick = e => {
        if (!handleClick) return;

        e.stopPropagation();
        return handleClick();
    };

Feel free to prove me wrong if that's the case. It's been a while since I've used or looked at this code.

quangta93 commented 5 years ago

@MilosRasic I believe my changes don't break custom handling of dropdown item click. What I did was lifting the handling of click events from ReactSvgIcon into helpers/index.js. event.stopPropagation() is still called inside helpers/index.js and the existence of handleClick is also checked inside that function. I think ReactSvgIcon is simply a "dumb" component so it should not interrupt the event bubbling process i.e. click events should bubble to higher level.

In addition, all SVG icons are now using onClick props, which is a more conventional way of naming things.