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

Events #10

Merged
merged 2 commits into from
Jun 4, 2018
Merged

Events #10

merged 2 commits into from
Jun 4, 2018

Conversation

JRJurman
Copy link
Member

@JRJurman JRJurman commented Jun 4, 2018

Summary

This is the first of an org-wide change to support events dynamically in Tram-One. This branches from Tram-One/tram-one#76

This change embeds a list of events into elements. They can then be accessed using the .events property. This gives Tram-One access to the list of events bound to an element, which is not natively available on the element.

Tests

A test has been added to verify that events are attached to the element. All tests pass locally.

const handleOnAttrSetter = element => prop => prop.key.slice(0, 2) === 'on' ? ![element[prop.key] = prop.value] : true
const handleNamespaceAttrSetter = (element, namespace) => prop => namespace ? ![element.setAttributeNS(null, prop.key, prop.value)] : true
const handleOnAttrSetter = element => prop => prop.key.slice(0, 2) === 'on' ? addEventToElement(element, prop.key, prop.value) || false : true
const handleNamespaceAttrSetter = (element, namespace) => prop => namespace ? element.setAttributeNS(null, prop.key, prop.value) || false : true

Choose a reason for hiding this comment

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

isn't this the same as

namespace || !element.setAttributeNS(null, prop.key, prop.value)

also isn't A || false always going to be A? Or is this because it requires a boolean value even if undefined?

Choose a reason for hiding this comment

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

Ahh, after reading the top a bit more I think I get some of the logic, I still feel like you should be watching how you are using A || false

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

2 participants