Conversation
a946128 to
faf8b33
Compare
This comment has been minimized.
This comment has been minimized.
e09f5bb to
79d52ab
Compare
79d52ab to
1526987
Compare
1526987 to
56efc15
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56efc15ec1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private fun buildRootCanonicalPath(root: View, screenNamespace: String): String { | ||
| val rootPathSegment = getViewPathSegment(root, null) | ||
| // Root view (e.g., DecorView) is not interactable, so we don't cache its identity. | ||
| // We only need its path as the prefix for descendant paths. | ||
| return "$appIdentifier/$screenNamespace/$rootPathSegment" |
There was a problem hiding this comment.
Disambiguate root view identity across windows
When multiple windows are present (e.g., a dialog over an activity), buildRootCanonicalPath only prefixes the canonical path with appIdentifier/screenNamespace plus the root view path segment, but it does not incorporate any window-specific identifier. For root views like DecorView (no resource id, same class name), different windows on the same screen can yield identical root paths, which then produce identical permanentId values for corresponding view hierarchies. That breaks the “globally unique” guarantee and can merge heatmap taps across different windows. Consider including a window-specific token (e.g., window hash/z-order) or another root-unique discriminator in the root canonical path.
Useful? React with 👍 / 👎.
What does this PR do?
Track ui elements with unique identifiers that are sent as part of the wireframes. Also send this identifier with taps and in this way correlate taps with views.
Important note: currently in the json schemas the new property
permanent_iddoes not appear last in the ordering. This means that we would technically be breaking backwards compatibility by using the change this way - so a decision on this is necessary before the pr could be merged. If the schemas are updated then the pr needs to incorporate those changes.Motivation
Support heatmaps on mobile.
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)