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

RUM-7215: Add AndroidComposeViewMapper to support popup #2395

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

ambushwork
Copy link
Member

What does this PR do?

In Jetpack compose, if the composable content is added by setContent{ } in the activity, then the wrapper view is ComposeView, but for the pop up like bottom sheet dialog or Drop down menu, they use AndroidComposeView as the wrapper in a separated DecorView.

So we need to support AndroidComposeView mapping otherwise the Session Replay can not resolve any content in the popup decor view.

In this PR, a small refactor is done in order to share the common logic between ComposeViewMapper and AndroidComposeViewMapper, here is the structure change:

image

Motivation

RUM-7215

Replay Demo

Known issue in demo:
Drop down menu doesn't have the correct position in replay

https://mobile-integration.datadoghq.com/rum/replay/sessions/26a23f05-aa91-427e-9f77-4b3457ac4ba2?applicationId=38030dde-f9f9-4e52-9443-b9804a030080&seed=029b6a93-9744-4fa8-84fe-b166b9a163fe&ts=1731675244745

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 86.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 69.78%. Comparing base (60631b4) to head (dd7ea78).
Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
...se/internal/mappers/semantics/ComposeViewMapper.kt 83.33% 1 Missing and 2 partials ⚠️
...ernal/mappers/semantics/RootSemanticsNodeMapper.kt 50.00% 1 Missing and 2 partials ⚠️
...rnal/mappers/semantics/AndroidComposeViewMapper.kt 93.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2395      +/-   ##
===========================================
- Coverage    69.79%   69.78%   -0.01%     
===========================================
  Files          760      762       +2     
  Lines        28310    28341      +31     
  Branches      4761     4763       +2     
===========================================
+ Hits         19757    19777      +20     
- Misses        7240     7243       +3     
- Partials      1313     1321       +8     
Files with missing lines Coverage Δ
...d/sessionreplay/compose/ComposeExtensionSupport.kt 92.00% <100.00%> (+4.50%) ⬆️
...rnal/mappers/semantics/AndroidComposeViewMapper.kt 93.33% <93.33%> (ø)
...se/internal/mappers/semantics/ComposeViewMapper.kt 83.33% <83.33%> (ø)
...ernal/mappers/semantics/RootSemanticsNodeMapper.kt 82.05% <50.00%> (ø)

... and 30 files with indirect coverage changes

@ambushwork ambushwork marked this pull request as ready for review November 15, 2024 13:43
@ambushwork ambushwork requested review from a team as code owners November 15, 2024 13:43
@ambushwork ambushwork force-pushed the yl/compose/add-android-compose-view-mapper branch from ace54a7 to dd7ea78 Compare November 15, 2024 15:11
@ambushwork ambushwork requested a review from 0xnm November 15, 2024 15:16
@ambushwork ambushwork merged commit 8896373 into develop Nov 15, 2024
24 checks passed
@ambushwork ambushwork deleted the yl/compose/add-android-compose-view-mapper branch November 15, 2024 15:54
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.

3 participants