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

right bug fix #665 #676

Closed
wants to merge 1 commit into from
Closed

right bug fix #665 #676

wants to merge 1 commit into from

Conversation

mgkirs
Copy link

@mgkirs mgkirs commented May 18, 2016

item.addEventListener('click', this.selectItem.bind(this, item, true));//bug #665
may be correct to fix the compiler

its compilate in:
['mousedown', 'touchstart'].forEach(function (name) {
item.addEventListener(name, function (event) {
_this2.selectItem(item, true);
event.preventDefault();
});
});

what we need:

['mouseup', 'touchend'].forEach(function (name) {
item.addEventListener(name, function (event) {
_this2.selectItem(item, true);
event.preventDefault();
});
});

item.addEventListener('click', this.selectItem.bind(this, item, true));//bug #665
  its compilate in:
['mousedown', 'touchstart'].forEach(function (name) {
	        item.addEventListener(name, function (event) {
	          _this2.selectItem(item, true);
	          event.preventDefault();
	        });
	      });
@jhchen
Copy link
Member

jhchen commented May 18, 2016

Why is mouseup better or more correct than mousedown?

@mgkirs
Copy link
Author

mgkirs commented May 18, 2016

mousedown its fast but user can do other events before last events was the end
and unusability when the button vanish while the mouse is still pressed.
But for touch screen, you right, touchstart is better.

@jhchen
Copy link
Member

jhchen commented May 19, 2016

Sure some interactions like highlighting the dropdown text would not be possible but I'm not sure any are common nor merit support. The reason click was changed is it caused loss of focus issues that are prevented with mousedown + preventDefault. If there's a common interaction that's lost that should be supported feel free to re-comment to reopen.

@jhchen jhchen closed this May 19, 2016
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.

2 participants