-
Notifications
You must be signed in to change notification settings - Fork 235
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
Click Analytics Plugin Update #1457
Conversation
@@ -75,21 +71,24 @@ export class DomContentHandler implements IContentHandler { | |||
} | |||
} else { | |||
// traverse up the DOM to find the closest parent with data-* tag defined | |||
contentElement = walkUpDomChainWithElementValidation(element, this._isTracked, dataTagPrefix); | |||
elementContent = extend(elementContent, this._populateElementContentwithDataTag( contentElement, element, dataTagPrefix, parentDataTagPrefix)); | |||
//contentElement = walkUpDomChainWithElementValidation(element, this._isTracked, dataTagPrefix); |
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.
Don't we still want to do this -- just not for the id
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.
We are doing this only when user has specified the parentDataTag. This tag does two things. IF present it auto populates parent content and also used as an identifier to stop traversing up the DOM to capture content name and value of elements when encountered with this tag.
extensions/applicationinsights-clickanalytics-js/src/Interfaces/Datamodel.ts
Outdated
Show resolved
Hide resolved
extensions/applicationinsights-clickanalytics-js/src/Interfaces/Datamodel.ts
Outdated
Show resolved
Hide resolved
extensions/applicationinsights-clickanalytics-js/src/events/PageAction.ts
Outdated
Show resolved
Hide resolved
extensions/applicationinsights-clickanalytics-js/src/events/PageAction.ts
Outdated
Show resolved
Hide resolved
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.
Approved with suggestions
Following updates have been made: