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 support for generating custom targets in the attribution build #585

Open
wants to merge 2 commits into
base: rework-interaction-logic
Choose a base branch
from

Conversation

philipwalton
Copy link
Member

Fixes #561 and #580.

This PR is an alternative approach to #562 (built on top of my proposed changes in #583) that also adds a new top-level generateTarget configuration option which allows developers to pass in their own function—as an alternative to the libraries internal getSelector() function.

All of the "target" attribution fields that previously reported the result of the internal getSelector() function will now report whatever value is returned from the passed generateTarget() function.

I believe there are two benefits to this approach:

  1. Developers whose HTML markup does not use meaningful or predictable class names may find the current logic unhelpful and prefer to configure their own.
  2. Developers who want a reference to the underlying DOM element can use this function to get it, without this library having to retain that reference for the majority of users who don't need it.

Breaking changes in this PR include (similar to #562):

  1. LCPAttribution.element has been renamed to LCPAttribution.target (for consistency, similar to Save a reference to the LCP and CLS targets in case they are removed from the DOM #562).
  2. LCPAttribution.interactionTargetElement has been removed (to address the issue in Keeping elements around for attribution can leak memory #580).

TODO: update the types in the README after we get agreement on the method names and descriptions added here.

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.

1 participant