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

Add StopPropagation for events #45

Merged
merged 1 commit into from Jun 26, 2016
Merged

Conversation

dave
Copy link
Contributor

@dave dave commented Jun 10, 2016

No description provided.

@dmitshur
Copy link
Contributor

LGTM, but people more familiar with this project should decide if this is in scope.

@@ -113,6 +114,14 @@ func (l *EventListener) PreventDefault() *EventListener {
return l
}

// StopPropagation prevents further propagation of the current event in the capturing and bubbling phases.
Copy link
Member

Choose a reason for hiding this comment

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

(nit) prefer to wrap this at 80 chars

Copy link
Contributor

@dmitshur dmitshur Jun 25, 2016

Choose a reason for hiding this comment

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

Just curious, is that your personal preference, or do you think that it's best practice (listed somewhere else)?

I agree for this comment, because it's top level, that it's nicer to wrap it to 80-100~ish column lines (which is optimal width for reading).

But there are other comment types that provide very specific detail, like inline notes, that I don't mind keeping very long and single-line. That way they're out of sight except when you really need to read about that particular detail. Here's an example of what I mean. Disclaimer: I use a computer where I can scroll horizontally as easily as I can scroll vertically.

Copy link
Member

Choose a reason for hiding this comment

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

Just curious, is that your personal preference, or do you think that it's best practice (listed somewhere else)?

I agree for this comment, because it's top level, that it's nicer to wrap it to 80-100~ish column lines (which is optimal width for reading).

I wouldn't say that "you should always use 80 chars" but I would say "as a general rule of thumb, wrap at 80 chars". In my mind, it doesn't apply to all situations but does to most (e.g. the case here).

But there are other comment types that provide very specific detail, like inline notes, that I don't mind keeping very long and single-line. That way they're out of sight except when you really need to read about that particular detail. Here's an example of what I mean. Disclaimer: I use a computer where I can scroll horizontally as easily as I can scroll vertically.

For these, I don't have too much preference. I think they are OK off to the side like this, and also OK as comments before that line wrapped at 80 chars. Both are okay in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been using 100 chars as a guide, but I'm perfectly happy with 80. Obviously there's times like this where breaking a comment would cause gofmt to break the nice indenting.

@slimsag
Copy link
Member

slimsag commented Jun 25, 2016

LGTM except one minor comment

@dave
Copy link
Contributor Author

dave commented Jun 26, 2016

Done.

@neelance
Copy link
Contributor

LGTM

@neelance neelance merged commit b1c4db8 into hexops:master Jun 26, 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.

None yet

4 participants