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.

Sign up for GitHub

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

Jump to bottom

Remove Event.stopPropagation() from generated dist file #1

Open
rasy-js opened this issue Nov 9, 2017 · 11 comments
Open

Remove Event.stopPropagation() from generated dist file #1

rasy-js opened this issue Nov 9, 2017 · 11 comments

Comments

@rasy-js
Copy link

rasy-js commented Nov 9, 2017

Hello, awesome plugin, but he does not work for me. Chrome v 61.0.3163.100.
I do not have errors in console. Pallete is drew, but not to change value in attribute. And do not highlight circles.

<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="UTF-8">
	<title>test</title>
</head>
<body>
	<md-color-picker id="picker" value="#abcdef" palette="material-accent" default-tint="300" use-spectrum-picker="true"></md-color-picker>
	<script src="dist/js/md-color-picker.js"></script>
</body>
</html>
@rasy-js
Copy link
Author

rasy-js commented Nov 10, 2017 •
edited
Loading

Picker need listener type "change" to get variable in event object.
Why i didn't find this information in docs?
I had to pick the demo:)

var picker = document.getElementById('picker');
function listenerEvent (event) {
	console.log(event);
	var color = event.detail[0];
	console.log('Selected Color:', color);
	picker.value = color;
}
picker.addEventListener('change', listenerEvent, false);

@rasy-js
Copy link
Author

rasy-js commented Nov 11, 2017

And, please, remove stopPropagation from code :)

@BennyAlex
Copy link
Owner

BennyAlex commented Nov 14, 2017 •
edited
Loading

@rasy-js Hello, thank you for your feedback :D
Can you explain me what you mean by "remove stopPropagation"?

I will add the information about the change event to the docs ;)

@BennyAlex
Copy link
Owner

Description about the event added by 63af3b5

@rasy-js
Copy link
Author

rasy-js commented Nov 14, 2017

@BennyAlex , now event is not bubbling, if register event on document. And then to click the picker elements.
I had removed Event.stopPropagation() from code, and event bubbled.

@BennyAlex
Copy link
Owner

BennyAlex commented Nov 15, 2017 •
edited
Loading

@rasy-js
Hey, this plugin is created from vue and vue-custom-element and then transformed with webpack to a module.
The dist/md-color-picker.js file is gererated automatically from the source files.
I simply call this.$emit('change', color.value). Maybe there is a setting to remove the geratoin of Event.stopPropagation() but maybe this is a default behaviour which can't be changed

@BennyAlex
Copy link
Owner

BennyAlex commented Nov 15, 2017 •
edited
Loading

I planning to release the color picker as a basic javascript module without vue and vue-custom-element, which would reduce the size of the picker drastically, then Event.stopPropagation() would be removed, too

@BennyAlex BennyAlex changed the title The value does not change Remove Event.stopPropagation() from gernerated dist file Nov 15, 2017
@BennyAlex BennyAlex changed the title Remove Event.stopPropagation() from gernerated dist file Remove Event.stopPropagation() from generated dist file Nov 15, 2017
@rasy-js
Copy link
Author

rasy-js commented Nov 15, 2017

It's very good idea, then can to close issue.

@BennyAlex
Copy link
Owner

@rasy-js
But as far as I know, register an event on the document isn't a good idea. For example, when there is another element which also sends an 'change' event, this would trigger the eventlistener, too. Or if you want to use 2 color pickers on the same page, you dont know which one sent the event.

@BennyAlex
Copy link
Owner

It is better to get the element (e.g. by ID) and then register the event directly on it.

@rasy-js
Copy link
Author

rasy-js commented Nov 15, 2017 •
edited
Loading

@BennyAlex , Event delegation is good practice https://javascript.info/event-delegation
One eventlinstener vs two. example -

document.addEventListener('change', function(e) {
	var target = e.target;
	
	switch(target.id) {
		case 'picker':
			// some code...
			break;
		case 'picker1':
			// some code...
			break;
		default:	

	}

});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rasy-js @BennyAlex