Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it more easily extendable #14

Closed
sindresorhus opened this issue Aug 4, 2016 · 10 comments · Fixed by #44
Closed

Make it more easily extendable #14

sindresorhus opened this issue Aug 4, 2016 · 10 comments · Fixed by #44

Comments

@sindresorhus
Copy link
Owner

The append and prepend options solve most use-cases, but there are still some people that want 100% control. Would be nice to expose a way to build the menu with ready made components, like lego. I'm thinking:

require('electron-context-menu')({
    menu: x => [
        x.COPY,
        {
            title: 'foo'
        },
        x.INSPECT
    ]
});

Where x.COPY is a Symbol that includes the built-in cut/copy/paste menus. That way it would be easy to add many more built-in menu types users could pick from.

@ghost
Copy link

ghost commented Aug 20, 2016

+1 for this definitely.

I find myself wanting to add SelectAll to the default cut copy paste commands.

I think your above syntax is a good approach, something like:

require('electron-context-menu')({
    prepend: params => [],
    append: params => [],
    menu: defaults => [
        defaults.COPY,
        <custom menu item>,
        defaults.INSPECT
    ]
});

and it the menu item isn't set, obviously default back to normal.

@sindresorhus
Copy link
Owner Author

I find myself wanting to all SelectAll to the default cut copy paste commands.

@callodacity Keep in mind that "Select all" is never in the context menu on macOS or Linux, but rather in the top menubar "Edit" menu. Only on Windows it's in the context menu. I should probably create a built-in for that. The reason this module is opinionated is to ensure users implement items correctly, like this case.

@ghost
Copy link

ghost commented Aug 20, 2016

@sindresorhus "Select All" is in the context menu on macOS when right-clicking inside any textarea or input (try it in the write comments section or the main text input on google). But I definitely agree that keeping it standard is always best. However, think it's important that this module is extendable / customisable too.

@sindresorhus
Copy link
Owner Author

@callodacity That's just Chrome being non-native as usual. Try Safari or any of the builtin macOS apps.

However, think it's important that this module is extendable / customisable too.

Yup, that's the plan.

@ghost
Copy link

ghost commented Aug 20, 2016

@sindresorhus Good point, I missed that.
Keep up the good work then mate 👍

@jarivo
Copy link
Contributor

jarivo commented Jul 2, 2017

@sindresorhus

As I mentioned in #43, I've started working on this issue.
In the current implementation, some options are displayed depending on the context (image, link).
How would we let the user change them? Just like a regular item in the menu property and let the context menu handle whether or not they should be displayed? We could also choose to create a new property to the items for images, links...

@sindresorhus
Copy link
Owner Author

but new features could take advantage of faster speeds

The item itself decides whether or not it should be visible based on the context.

@JoniVR
Copy link
Contributor

JoniVR commented Feb 19, 2019

Is anyone still working on this?
I think this would be required for sindresorhus/caprine#248.

I can try implementing it if not. Is there already a consensus on how it would be best solved?

@sindresorhus
Copy link
Owner Author

@JoniVR #44 You could help out by reviewing that one (and maybe also help fixing the merge conflicts if you have the time).

@JoniVR
Copy link
Contributor

JoniVR commented Feb 21, 2019

@sindresorhus Sure 👍🏻

sindresorhus pushed a commit that referenced this issue Apr 9, 2019
Fixes #14 
Fixes #43

Co-authored-by: Joni Van Roost <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants