fix: update the logic of tag_name to prefer a ph-label from a parent …

[steffanlevet]

Improve tag_name when touch events are nested

Problem

This addresses #145

Specifically; when using react-native-gesture-handler, and the ph-label is set on a TouchableOpacity or similar, the label is obscured, because the touch event is triggered against a nested child.

An example of such a case:

<TouchableOpacity
      ph-label="my-label"
      onPress={() => ...}
    >
      <View style={...}>
        <Text style={...}>
          My Heading
        </Text>
        <Text style={...}>
My sub heading
</Text>
      </View>
    </TouchableOpacity>

Changes

This change adds an additional pass over the selected elements to promote any ph-label from a parent onto a child.
In the case above, this would result in the nested View and Text children getting the tag_name 'my-label' rather than the current inferred value, be it the displayName or similar.

This change also works when there are multiple ph-label properties set in the tree, the nearest parent with a label is chosen.

In the case there is no ph-label the existing behaviour is preserved.

This change makes the display of these captured touches far more reasonable when viewing them in the dashboard - instead of "My Heading" being the tag_name attached to an event, the intended label is used.

It is worth noting, this could be considered a breaking change. The breaking nature is really quite minor, and it is unlikely the behaviour changing is depended upon, but the information presented in the dashboard could be different for any given touch before and after this release.

Finally, it is also possible an event receives a tag_name that seems non sensical. Consider the following:

<ScrollView
ph-label="my-top-level-scroll"
>
<ManyNestedViews>
<TouchableOpacity
      onPress={() => ...}
    >
      <View style={...}>
        <Text style={...}>
          My Heading
        </Text>
        <Text style={...}>
My sub heading
</Text>
      </View>
    </TouchableOpacity>
</ManyNestedViews>
</ScrollView>

Here, when the TouchableOpacity receives the touch, the resulting tag_name will be 'my-top-level-scroll'. That could be quite confusing to say the least. This seems like a reasonable trade off since it seems like an anti-pattern to be using ph-label in this way.

Release info Sub-libraries affected

Bump level

• Major
• Minor
• Patch

Libraries affected

• All of them
• posthog-web
• posthog-node
• posthog-react-native

Changelog notes

• Added support for X

[marandaneto]

I'd say this is considered a fix/improvement to the current behavior as discussed before.
The inferred naming now in this situation isn't super useful so the changes improve it even if it's a sort of breaking change in behavior, it's a best-effort approach.

@steffanlevet thanks for the PR.
Would you mind adding an entry to the changelog?

[marandaneto]

@steffanlevet lint and tests (only 1) are broken, would you mind addressing them?

[marandaneto]

LGTM, thanks @steffanlevet

[steffanlevet]

@marandaneto I addressed the lint and bumped the version/updated the changelog.
The test I am not sure about. Running the react-native tests all pass, it seems the tests failing are part of other packages?
Thanks

[marandaneto]

@marandaneto I addressed the lint and bumped the version/updated the changelog. The test I am not sure about. Running the react-native tests all pass, it seems the tests failing are part of other packages? Thanks

@steffanlevet did you commit/push your changes? I don't see any changelog changes.
See your commit.

Tests are green now, just the changelog missing really.

[steffanlevet]

@marandaneto you are right 🤦 . That is all here now.