-
-
Notifications
You must be signed in to change notification settings - Fork 259
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
don't click when disabled #384
Conversation
@@ -59,7 +59,10 @@ export default function click(target) { | |||
throw new Error(`Element not found when calling \`click('${target}')\`.`); | |||
} | |||
|
|||
__click__(element); | |||
if (!element.hasAttribute('disabled')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should check the property instead of the attribute 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
I'm confused. I understand that disabled inputs can't be filled, but can't they be clicked? That's surprising |
@cibernox I think you're confusing |
I'm not. I only expected disabled inputs to not be fillable or focusable, I didn't know they weren't clickable |
@@ -59,7 +59,10 @@ export default function click(target) { | |||
throw new Error(`Element not found when calling \`click('${target}')\`.`); | |||
} | |||
|
|||
__click__(element); | |||
if (!element.disabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to do something like:
let isDisabledFormControl = isFormControl(element) && element.disabled === true;
if (!isDisabledFormControl) {
__click__(element);
}
Specifically, adding disabled
to random elements doesn't have an effect (e.g. <div disabled></div>
still fires events) as it isn't a global attribute. AFAIK disabled
is only a thing for form controls...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Thank you @kellyselden! |
Closes #383.
Probably affects other helpers.