Disableguard

[sielay]

#41 + ability to pass function that checks if item should be disabled

[fkhadra]

Hey @sielay,

I'll review the pull request during this weekend. Thanks

[sielay]

It may require some docs. I can provide use cases. Basically your lib works for me, but sometimes my options heavily depends on data bound to elements clicked and passing that data or rendering conditional menus based on it wasn't easy/possible/nice.

[fkhadra]

@sielay could you provide an example of how it works ?

Thanks

[sielay]

From actual project:

Passing data

<ContextMenuProvider
    id="attachment-menu"
    className="inputIcon"
    renderTag="div"
    event="onClick"
    data={{
        callback: this.onAttachement.bind(this)
    }}
    >
        <img src="/brand/assets/sprites/add.svg" alt="Add attachment" />
</ContextMenuProvider>

<ContextMenu
                    id="attachment-menu"
                    animation="pop">
                    <Item onClick={(...args) => (upload(args[3]))}>Upload</Item>
                    <Item onClick={(...args) => (pickFromLibrary(args[3]))}>Library Item</Item>
</ContextMenu>

Disable guard

<If check={this.checkEncrypt.bind(this)}>
                <Item
                    onClick={(...args) => this.decrypt(args[3].comment, args[3].decrypt)}
                    disableGuard={this.isAttachment.bind(this)}
                >Decrypt</Item>
            </If>

You can see there If component I added as it was easier to do conditional rendering using your lib, here is component code:

import * as React from "react";
import { ReactNode } from "react";

export class If extends React.Component {
    public props: Readonly<{ children?: ReactNode }> & Readonly<{
        outerData?: any,
        check: (outerDatadata: any) => boolean,
        render?: (data) => any 
    }>;

    constructor(props) {
        super(props);
        console.log("IF constructor", props, this);
    }

    public cloneItem = item => React.cloneElement(item, {
        outerData: this.props.outerData || item.props.outerData
    });

    public render() {

        if (this.props.check(this.props.outerData)) {

            if (this.props.render) {
                return this.props.render(this.props.outerData);
            }

            return React.Children.map(
                React.Children.toArray(this.props.children),
                this.cloneItem,
            );
        }
        return null;
    }
}

[fkhadra]

Hey @sielay,

Thanks for the PR again.

Regarding the disableGuard props, I think that it will be easier to use the existing props disable to avoid any confusion. To do so we need to allow both boolean and function on disable props. Then all we have to do is to check if it's a boolean or a function.

Instead of outerData maybe you could use dataFromProvider just to be consistent with others props like refsFromProvider. What do you think?

You export a triggerContextMenu function, I'm wondering what is the use case for that? Maybe I'm missing something here.

Thank you

[sielay]

Heh. Yes there is a case for it. I mixed your lib with react-selection ended up having context providers that overlapped. I will try to provide more code examples. I like your suggestions. Can we park it for a week? Need some time to apply changes. Thanks for review.

[sielay]

Sorry for delay. I had loads of work. Will try to present clear version with examples soon.

[fkhadra]

Hello,

Actually, I rewrote a big part of the library. It's on the next branch. I already made the change to the disable props and also the data to the contextmenuprovider.

I'll make a rebase when it's ready so you won't lose your contribution

[sielay]

I am closing this PR. I will create new, consolidated with If component