-
Notifications
You must be signed in to change notification settings - Fork 0
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
Context Menu Plugin Scoping #113
Comments
$('table').contextMenu({
menuItems: [
{
label: "Copy Table",
onClick: function() {
// do something
}
},
{
label: "Copy Cell",
onClick: function() {
// do something else
}
}
]
}); vs $('table').on('contextmenu', function(event) {
$(this).contextMenu({
"Copy Table": function() {
// do something
},
"Copy Cell": function() {
// do something else
}
});
event.preventDefault();
}); |
|
For a |
If the plugin only opens the context menu, it makes little sense to call it on an element (or selection). It would make more sense as a utility function, rather than a plugin. |
Suggested usage $('table').on('contextmenu', function(event) {
var target = $(this);
if ( // textrange check ) {
return true;
}
qcode.contextMenu({
position: [event.clientX, event.clientY],
menuItems: [
{
label: "Copy Table",
action: function() {
target.tableCopy();
}
}
]
});
event.preventDefault();
}); |
contextMenu function should:
|
@bvw do you want to add some input on this, or should I go ahead with with the above design? |
Yes please as you see fit. |
Suggested range check: if ( window.getSelection().rangeCount > 0
&& ! window.getSelection().getRangeAt(0).collapsed
) {
return true;
} |
Design a plugin for custom context menus (replacing browser default context menu)
Acceptance Criteria:
The text was updated successfully, but these errors were encountered: