Skip to content

Track clicks when element inside of <a href> tag is clicked.#54

Merged
ankane merged 12 commits into
ankane:masterfrom
fleitz:master
Oct 7, 2020
Merged

Track clicks when element inside of <a href> tag is clicked.#54
ankane merged 12 commits into
ankane:masterfrom
fleitz:master

Conversation

@fleitz

@fleitz fleitz commented Jul 6, 2020

Copy link
Copy Markdown
Contributor
<a href="#foo"> <div> <img src="clickable_image"> </div> </a>
Expected result: Clicking on image results in tracking

Actual Result:
Clicking on image does not result in tracking.

@ankane

ankane commented Oct 6, 2020

Copy link
Copy Markdown
Owner

Hey @fleitz, thanks for the PR, and sorry for the delay (have been focused on other projects). This change makes sense to me. Instead of setting matchedElement on the event, it would be good to make this the matched element (as libraries like jQuery do).

@fleitz

fleitz commented Oct 6, 2020

Copy link
Copy Markdown
Contributor Author

Yeah, that sounds like a way cleaner way to implement.

@fleitz

fleitz commented Oct 7, 2020

Copy link
Copy Markdown
Contributor Author

@ankane implemented.

@ankane ankane left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @fleitz, looking better. Added a few comments inline.

Comment thread src/index.js
Comment thread src/index.js Outdated
Comment thread src/index.js Outdated

@ankane ankane left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @fleitz, added a few more comments. Please follow the existing spacing everywhere.

Comment thread src/index.js Outdated
Comment thread src/index.js
return obj;
}

function eventProperties(e) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No change needed on this line.

Comment thread src/index.js Outdated
Comment thread src/index.js Outdated
@ankane ankane merged commit b1d5dd1 into ankane:master Oct 7, 2020
@ankane

ankane commented Oct 7, 2020

Copy link
Copy Markdown
Owner

Thanks @fleitz! This is a great feature.

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