retejs / context-menu-plugin

https://retejs.org
MIT License
11 stars 43 forks source link

React version #15

Closed pcvonz closed 1 year ago

pcvonz commented 5 years ago

Hey, I'd like to use this, but I'm not too keen on adding Vue to my React project. So, I'm considering porting the Vue files to React.

I'm curious if anyone has any thoughts on the possibility of making this framework agnostic, or perhaps giving the user the ability to select the renderer. Otherwise I'll just fork this project and make a React version.

Ni55aN commented 5 years ago

framework agnostic

Coding on vanilla js? Not me :)

I'll just fork this project and make a React

Yes, you can, but it will lead to a divergence of changes.

giving the user the ability to select the renderer

I prefer this way (I think peer dependencies and require() will suite). I could to prepare it for integrating multiple *-menu folders for different renderers on the weekend

pcvonz commented 5 years ago

Coding on vanilla js? Not me :)

:laughing:

I prefer this way (I think peer dependencies and require() will suite). I could to prepare it for integrating multiple *-menu folders for different renderers on the weekend

Awesome, I'll hold off until you make that change. How would you go about adding React to this project? I only have experience with babel and webpack. I see https://github.com/retejs/context-menu-plugin/blob/master/rete.config.js has a reference to Vue, so I'm guessing React would go somewhere in there?

Ni55aN commented 5 years ago

Rete CLI's config based on Rollup

has a reference to Vue, so I'm guessing React would go somewhere in there?

Yes, the globals property is needed to prevent dependencies from being included in the bundle.

Ni55aN commented 5 years ago

@pcvonz I have refactored the code and added ability to use different renderer-based menus (next branch). Also implemented is the basic React menu without a search and disappearing by timeout.

import ContextMenuPlugin, { 
    VueMenu,
    VueComponents,
    ReactMenu,
    ReactComponents,
    IMenu
} from 'rete-context-menu-plugin';

editor.use(ContextMenuPlugin, {
    Menu: ReactMenu
});
pcvonz commented 5 years ago

I imported the next branch into my project and noticed an error. JSX in react gets transformed into a call to React.createElement. So, anytime you use JSX you have to import React. Item.jsx, index.jsx, and menu.jsx all need to import React. Works great otherwise :+1:

pcvonz commented 5 years ago

In order to fit the react context menu into the project I'm working on, I would need to be able to inject my own components into the menu in order to override the styles. Otherwise I'd have to include a globally scoped CSS file (which isn't the worst thing honestly, just not how CSS is usually handled in the current project).

Maybe it'd be useful to define the wrapper components for the menu? Basically allow this kind of pattern:

// A new Rete project

function Menu(props) {
    return (
        <div {...props}>
            {props.children}
        </div>
    )
}

function Item(props) {
    return (
        <div {...props}>
            {props.children}
        </div>
    )
}

function ReteEditor() {
...
        editor.use(ContextMenuPlugin,
            {
                Menu: ReactContextMenu,
                Components: { // Inject wrapper components
                    Menu: Menu,
                    Item: Item
                }
            }
        );

...
}

And then in context-menu react components:

//rete-context-menu

// Menu.js
import './style.css';
import Item from './Item';
import Context from './context';
import React from 'react';

export default ({ items, position: [x, y], visible, args, onClose, Components }) => {
    if(!visible) return null;

    return (
        <Context.Provider value={{ args, onClose}}>
            <Components.Menu className="context-menu" style={{ left: x+'px', top: y+'px' }}> // Wrapper component
                {items.map(item => (
                    <Item ItemComponent={Components.Item} item={item} />
                ))}
            </Components.Menu>
        </Context.Provider>
    )
}

// Item.js

class Item extends React.Component {
...
    render() {
        const { item: { title, subitems }, ItemComponent} = this.props;
        const { visibleSubitems } = this.state;

        return (
            <ItemComponent
                className={'item' + (subitems? ' hasSubitems': '')}
                onClick={this.onClick}
                onMouseOver={() =>
                        this.setState({ visibleSubitems: true })}
                        onMouseLeave={() => this.setState({ visibleSubitems: false })}
                    >
                        {title}
                        {subitems && visibleSubitems && <div className="subitems">
                            {subitems.map((subitem) => <Item item={subitem}/>)}
                        </div>}
                    </ItemComponent>
        )
    }
...

}
Ni55aN commented 5 years ago

@pcvonz is the any progress?

pcvonz commented 5 years ago

You can take a look at the changes here. https://github.com/bargreenellingson/context-menu-plugin/tree/next

pcvonz commented 5 years ago

Oops, left some stuff uncommitted. The Components parameter should make more sense now. Basically, it'd be used to add additional functionality to the context-menu-plugin's Menu and Item component without rewriting them. For example: Adding a custom class which is hashed by webpack:

image

Ni55aN commented 5 years ago

@pcvonz HOC ?

Ni55aN commented 5 years ago

What is the problem to create component based on exist Menu?

pcvonz commented 5 years ago

What is the problem to create component based on exist Menu?

Wouldn't that require rewriting ReactMenu?

Ni55aN commented 5 years ago

Wouldn't that require rewriting ReactMenu?

Its require, otherwise there is another one property and two subproperties

calix commented 4 years ago

Anything new regarding this issue? Plans to make a new release?

jtwnel commented 4 years ago

Hi, is there going to be further progress on this issue?

Codelica commented 3 years ago

Any chance this is still being considered in 2021 ? :)

rajasegar commented 3 years ago

@Ni55aN Any chance the next branch merged into the main branch and published to npm?

rajasegar commented 2 years ago

@Codelica I have published a React only version in npm, you can try that and let me know for any issues https://www.npmjs.com/package/rete-context-menu-plugin-react

Codelica commented 2 years ago

@rajasegar Nice work! I've been pulled into another project, but will be back to Rete in the spring and will be sure to check it out. 👍

Adrian-Serna commented 1 year ago

@Codelica I have published a React only version in npm, you can try that and let me know for any issues https://www.npmjs.com/package/rete-context-menu-plugin-react

It works great, thank you very much. I only had a little problem, because in the documentation of the site it says:

import ContextMenuPlugin, { 
    ReactMenu,
} from 'rete-context-menu-plugin';

and it should be:

import ContextMenuPlugin, { 
    ReactMenu,
} from 'rete-context-menu-plugin-react';

Thank you!

rete-js[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.