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

Allow to control click and context propagation #141

Merged
merged 6 commits into from
Aug 21, 2021

Conversation

ngmy
Copy link
Contributor

@ngmy ngmy commented Aug 16, 2021

I think it's more convenient to enable browser click and right-click by default.
For example, click to open URL in an item, or right-click to use the browser developer tool.

jkanban.js Outdated
@@ -54,8 +54,8 @@ var dragula = require('dragula');
dragBoard: function (el, source) {},
dragendBoard: function (el) {},
dropBoard: function (el, target, source, sibling) {},
click: function (el) {},
context: function (el, e) {},
click: null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not recommend that. I guess it would be nice to have a kind of property or even a callback function to get if user wants to have native click or context.

jkanban.js Outdated
@@ -508,6 +508,9 @@ var dragula = require('dragula');
}

function __onclickHandler (nodeItem, clickfn) {
if (typeof self.options.click !== 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See that browser event is cancelled by preventDefault method. So if we have a property to control if jKanban has to propagate click or context this if will be simpler like ´if (!self.ownedHandlers.includes('click')) { return }`

jkanban.js Outdated
@@ -516,6 +519,9 @@ var dragula = require('dragula');
}

function __onContextHandler(nodeItem, contextfn) {
if (typeof self.options.context !== 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then here we can check ownedHandlers as well like ´if (!self.ownedHandlers.includes('context')) { return }`

jkanban.js Outdated
click: function (el) {},
context: function (el, e) {},
click: null,
context: null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about that, I could suggest you to create a property called ownedHandlers or so, in order to control what event jKanban will handle ('click', 'context').

@ngmy ngmy changed the title Enable browser click and right-click by default Allow to control click and context propagation Aug 19, 2021
@ngmy
Copy link
Contributor Author

ngmy commented Aug 19, 2021

@marcosrocha85
Is it like this...?
Changed to control propagation of events rather than calling callbacks.
I'm not sure about the property name.

@riktar riktar merged commit 0b03131 into riktar:master Aug 21, 2021
@riktar
Copy link
Owner

riktar commented Aug 21, 2021

Merged, thanks @ngmy

@ngmy ngmy deleted the enable-browser-click branch August 21, 2021 16:05
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.

None yet

3 participants