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

Add menu option #44

Merged
merged 10 commits into from
Apr 9, 2019
Merged

Add menu option #44

merged 10 commits into from
Apr 9, 2019

Conversation

jarivo
Copy link
Contributor

@jarivo jarivo commented Jul 7, 2017

This PR "fixes" both #14 and #43

So basically the user can now:

  • Specify a menu-property in the configuration to override the default menu structure
  • Give a transform-function to the predefined actions in order to manipulate e.g. the data that will be copied to the clipboard
  • Override the click method. This will not work in combination with the transform so if you want to override the default click handler then you're basically on your own.

The menuItem itself will determine wheter it should be visible or not. (Although this should be easy to implement when needed)

append and prepend should still work as expected. Although they now have an extra argument with the default actions.

I've changed the fixture to represent the new features.

require('.')({
	labels: {
		cut: 'Configured Cut',
		copy: 'Configured Copy',
		paste: 'Configured Paste',
		save: 'Configured Save Image',
		copyLink: 'Configured Copy Link',
		inspect: 'Configured Inspect'
	},
	prepend: (x) => [x.CUT({transform: (content) => "modified_cut_" + content})],
	menu: (x) => [
		x.SEPARATOR(),
		x.COPY_LINK({transform: (content) => "modified_link_" + content}),
		x.SEPARATOR(),
		{
			label: 'Unicorn'
		},
		x.SEPARATOR(),
		x.COPY({transform: (content) => "modified_copy_" + content}),
		{
			label: 'Invisible',
			visible: false
		},
		x.PASTE({transform: (content) => "modified_paste_" + content})
	],
	append: (x) => [x.SAVE_IMAGE()]
});

TODO

  • Update README.md
  • Fix CI

Fixes #14
Fixes #43

package.json Outdated Show resolved Hide resolved
fixture.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
fixture.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Override the click method. This will not work in combination with the transform so if you want to override the default click handler then you're basically on your own.

I don't really see the point of this. If you override click, you might as well just create your own menu item.

@jarivo
Copy link
Contributor Author

jarivo commented Jul 8, 2017

@sindresorhus

I've also removed this. Initially I added it with the idea of users maybe don't want to deal with "when should a menu item be visible" depending on the context.

@jarivo
Copy link
Contributor Author

jarivo commented Jul 28, 2017

Any update on this PR?

readme.md Outdated Show resolved Hide resolved
@jarivo jarivo closed this Feb 6, 2019
@sindresorhus sindresorhus reopened this Feb 21, 2019
@sindresorhus
Copy link
Owner

@jarivo I'm really sorry I never got around to merging this. It just got lost in all my tabs... I'm willing to prioritize reviewing it now if you fix the merge conflict, if not, no worries either.

@jarivo
Copy link
Contributor Author

jarivo commented Feb 21, 2019

@JoniVR @sindresorhus

I'm happy to help out fixing the merge conflicts 😄

@JoniVR
Copy link
Contributor

JoniVR commented Feb 21, 2019

@jarivo
I have some modified files (the conflicting ones) here which resolve the conflicts, but some functionality isn't working 100% yet. Should I send you the contents or just submit a PR with the changes to your fork? If you'd prefer doing everything yourself then that's fine too, it would be great if this PR could get implemented! 😄

@jarivo
Copy link
Contributor Author

jarivo commented Feb 21, 2019

@JoniVR
Thanks!
I guess the easiest way is a PR to my fork 👍

@jarivo
Copy link
Contributor Author

jarivo commented Feb 21, 2019

TODO

  • Complete definition file
  • Check README

@sindresorhus sindresorhus changed the title Menu should be easily extendable Add menu option Apr 9, 2019
@sindresorhus sindresorhus merged commit bb1b073 into sindresorhus:master Apr 9, 2019
sindresorhus added a commit that referenced this pull request Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alter behaviour of default actions/roles e.g. when copying a link Make it more easily extendable
4 participants