search

vkbansal / react-contextmenu

Project is no longer maintained
MIT License
1.4k stars 375 forks source link

In MenuItem, don't override className prop, but extend it #293

Closed wuppious closed 5 years ago

wuppious commented 5 years ago

Currently, you can give MenuItem classNames through the attributes prop. These are then applied to the element, but also overriding any className given as a direct prop. https://github.com/vkbansal/react-contextmenu/blob/22d299e986367e81bcde575b4abeffdb9ace0a5c/src/MenuItem.js#L56-L65

The reason why this might be problematic, is that some css-in-js libraries such as styled-components apply an automatically generated className to the props, which then gets overridden in the render method.

My suggestion would be to accept this className prop and extend it instead of overriding.

wuppious commented 5 years ago

Meanwhile, a workaround is to wrap the MenuItem into a functional component and apply the generated className to the attributes.

const MenuItemWithClassName = props => (
  <MenuItem {...props} attributes={{ className: props.className }} />
);
vkbansal commented 5 years ago

Good catch!. This library was written when CSS-in-JS libraries were not a thing.

Now that they are a thing, it should be taken care of. Will you be willing to submit a PR?

wuppious commented 5 years ago

I can have a look during the weekend.

wuppious commented 5 years ago

@vkbansal PR is done, however it fails due to Code Climate config error. I don't believe I can affect that, so please check into it.

vkbansal commented 5 years ago

No issues. I'll take care of it

On Sun, Sep 1, 2019, 5:41 PM Nico Nysten notifications@github.com wrote:

@vkbansal https://github.com/vkbansal PR is done, however it fails due to Code Climate config error. I don't believe I can affect that, so please check into it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vkbansal/react-contextmenu/issues/293?email_source=notifications&email_token=ABNH234BOLZ7OW5IVT4WCLLQHOWQFA5CNFSM4ISB6QSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5UBBPQ#issuecomment-526913726, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNH23YQZEXWXMEQHHF7KBDQHOWQFANCNFSM4ISB6QSA .