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

Do not prevent default click handling in Element editor #1445

Merged
merged 1 commit into from
May 18, 2018

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented May 17, 2018

Check boxes, their labels, and submit buttons need to handle their own click
events.

What is this pull request for?

Closes #1444

Notable changes (remove if none)

I'm not very sure about this change as I don't quite get why the preventDefault call is in that method at all. With this change, clicks will actually click buttons, labels and inputs. That's most of the area of an Element editor.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

I'm not very sure about this change as I don't quite get why the preventDefault call is in that method at all.

You are right. This even works without.

# Element submit button needs to keep it's default event
e.preventDefault() unless $target.is(':submit')
# Buttons, inputs and labels need to handle their clicks.
e.preventDefault() unless $target.is(':submit, input, label')
Copy link
Member

Choose a reason for hiding this comment

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

Just tested it. It even works with e.preventDefault completely removed.

Check boxes, their labels, and submit buttons need to handle their own click
events.
@mamhoff mamhoff force-pushed the quick-checkbox-fix branch from 5d98ced to 8447e4f Compare May 17, 2018 20:51
@mamhoff
Copy link
Contributor Author

mamhoff commented May 17, 2018

Ha! Fixed a frontend bug! Thanks for the review, I deleted the two lines. :)

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks

@tvdeyen tvdeyen merged commit 36a9f7e into AlchemyCMS:master May 18, 2018
@tvdeyen tvdeyen added this to the 4.1 milestone May 18, 2018
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