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

Event delegation should skip "disabled" elements #134

Closed
mairatma opened this issue Jul 21, 2016 · 15 comments
Closed

Event delegation should skip "disabled" elements #134

mairatma opened this issue Jul 21, 2016 · 15 comments
Assignees
Labels

Comments

@mairatma
Copy link
Contributor

From @mairatma on July 5, 2016 17:37

Events don't trigger for disabled elements natively, even when bubbling. Our event delegation doesn't respect that though, since it catches the event in the component's element and doesn't check for disabled states.

Need to investigate which elements are automatically skipped natively (button for sure, but maybe more of them too).

Copied from original issue: mairatma/metal-events#2

Copied from original issue: mairatma/metal-lerna#2

@mairatma mairatma added the bug label Jul 21, 2016
@mairatma
Copy link
Contributor Author

From @fernandosouza on July 16, 2016 18:53

👍

@mairatma
Copy link
Contributor Author

From @fernandosouza on July 17, 2016 22:45

Hi, @mairatma. I'm taking care of this issue now. I'll send you any feedback as soon as I can.

@mairatma
Copy link
Contributor Author

From @fernandosouza on July 18, 2016 2:44

@mairatma. I've followed the steps in the README.md but I've got this error after running the "npm test" command.

I don't think it's related to the Metal, but could you give me a hand?

@ pretest /Users/fernandosouza/Projects/metal/source-code
npm run compile

@ compile /Users/fernandosouza/Projects/metal/source-code
lerna run compile

Too many arguments.

npm ERR! Darwin 15.5.0
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "run" "compile"
npm ERR! node v5.11.1
npm ERR! npm v3.8.6
npm ERR! code ELIFECYCLE
npm ERR! @ compile: lerna run compile
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @ compile script 'lerna run compile'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! lerna run compile
npm ERR! You can get information on how to open an issue for this project with:
npm ERR! npm bugs
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! npm owner ls
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR! /Users/fernandosouza/Projects/metal/source-code/npm-debug.log
npm ERR! Test failed. See above for more details.

@mairatma
Copy link
Contributor Author

Weird, is this the full log? Seems like it's no really saying where the problem is, so it's hard to figure it out. Can you run again and make sure to output the whole thing, from your command (which doesn't show up here, but I assume is npm test to the end of the error?

@mairatma
Copy link
Contributor Author

From @yuchi on July 18, 2016 15:8

I believe it’s there, just badly formatted:

> @ pretest /Users/fernandosouza/Projects/metal/source-code
> npm run compile

> @ compile /Users/fernandosouza/Projects/metal/source-code
> lerna run compile

Too many arguments.

npm ERR! Darwin 15.5.0
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "run" "compile"
npm ERR! node v5.11.1
npm ERR! npm v3.8.6
npm ERR! code ELIFECYCLE
npm ERR! @ compile: lerna run compile
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @ compile script 'lerna run compile'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! lerna run compile
npm ERR! You can get information on how to open an issue for this project with:
npm ERR! npm bugs 
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! npm owner ls 
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR! /Users/fernandosouza/Projects/metal/source-code/npm-debug.log
npm ERR! Test failed. See above for more details.

@mairatma
Copy link
Contributor Author

From @mthadley on July 18, 2016 16:58

I also tried to run the tests right after the mono-repo was created, but I saw the same error. It seemed strange since I couldn't find any docs on the run command for lerna.

@mairatma
Copy link
Contributor Author

hmmm weird, I'm going to try to reproduce by recreating this now

@mairatma
Copy link
Contributor Author

Oh, actually I think I know what it is. My README.md instructions are wrong, it tells you to install lerna via npm -g i lerna but that will install version 1.x, while we're using 2.x already (they recommend using 2.x for new projects, but leave latest as 1.x).

So you need to install lerna via npm install -g lerna@^2.0.0-beta instead. That's probably why the command isn't working.

@mairatma
Copy link
Contributor Author

Fixed the instructions here: 619c71c

@mairatma
Copy link
Contributor Author

From @fernandosouza on July 18, 2016 17:28

Now it's working. Thanks @mairatma.

Some commands I have to use sudo but it's working.

@mairatma
Copy link
Contributor Author

Yeah, I usually need sudo for installing global dependencies, but it shouldn't be necessary for other things. Did you need it with any lerna command or sth like that?

@mairatma
Copy link
Contributor Author

From @fernandosouza on July 18, 2016 18:34

Yeah. I've needed it to sudo lerna bootstrap and sudo npm test (maybe for lerna run compile command) and also to install lerna.

@mairatma
Copy link
Contributor Author

Fixed by d2293a1.

@fernandosouza
Copy link
Contributor

fernandosouza commented Sep 3, 2016

@mairatma. I think I found the problem that Chema mentioned. Maybe we should reopen this issue and I will send you a new PR.

@mairatma mairatma reopened this Sep 5, 2016
@mairatma
Copy link
Contributor Author

mairatma commented Oct 5, 2016

This has now been fixed in master :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants