Skip to content

RUM-6897: Fix scrolling issue & Add DrawableCopier interface for drawable copying for image wireframe#2354

Merged
ambushwork merged 2 commits into
release/2.15.0from
yl/ga/fix-scrolling-freezing
Oct 28, 2024
Merged

RUM-6897: Fix scrolling issue & Add DrawableCopier interface for drawable copying for image wireframe#2354
ambushwork merged 2 commits into
release/2.15.0from
yl/ga/fix-scrolling-freezing

Conversation

@ambushwork

@ambushwork ambushwork commented Oct 28, 2024

Copy link
Copy Markdown
Member

What does this PR do?

In Session Replay, we need to copy the drawable in case of any GC or modifying original resource issue.
However each mappers have slightly different way of copying the drawable, for example in most of case it's enough to just copy the drawable like :

originalDrawable.constantState?.newDrawable(resources)

But in some mappers like SwtichMapper we need to append the tint list and the drawable state.

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)

@ambushwork
ambushwork marked this pull request as ready for review October 28, 2024 14:02
@ambushwork
ambushwork requested review from a team as code owners October 28, 2024 14:02
jonathanmos
jonathanmos approved these changes Oct 28, 2024
View reviewed changes

@jonathanmos jonathanmos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

mariusc83
mariusc83 approved these changes Oct 28, 2024
View reviewed changes
Comment thread .../main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier.kt
@codecov-commenter

codecov-commenter commented Oct 28, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 87.91209% with 11 lines in your changes missing coverage. Please review.

Project coverage is 70.30%. Comparing base (b68f6b3) to head (73490b8).
Report is 3 commits behind head on release/2.15.0.

Files with missing lines Patch % Lines
...lay/internal/recorder/mapper/SwitchCompatMapper.kt 79.17% 10 Missing ⚠️
...ternal/recorder/resources/DefaultDrawableCopier.kt 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@                Coverage Diff                 @@
##           release/2.15.0    #2354      +/-   ##
==================================================
- Coverage           70.33%   70.30%   -0.03%     
==================================================
  Files                 740      741       +1     
  Lines               27633    27646      +13     
  Branches             4628     4628              
==================================================
+ Hits                19434    19435       +1     
- Misses               6903     6917      +14     
+ Partials             1296     1294       -2
Files with missing lines Coverage Δ
...replay/internal/recorder/mapper/ImageViewMapper.kt 97.22% <100.00%> (+5.12%) ⬆️
.../recorder/resources/DefaultImageWireframeHelper.kt 95.33% <100.00%> (+0.03%) ⬆️
...ay/internal/recorder/resources/ResourceResolver.kt 95.24% <100.00%> (-1.88%) ⬇️
...order/mapper/BaseAsyncBackgroundWireframeMapper.kt 94.74% <100.00%> (-0.26%) ⬇️
...ndroid/sessionreplay/utils/ImageWireframeHelper.kt 66.67% <100.00%> (+6.67%) ⬆️
...ternal/recorder/resources/DefaultDrawableCopier.kt 50.00% <50.00%> (ø)
...lay/internal/recorder/mapper/SwitchCompatMapper.kt 88.24% <79.17%> (-4.22%) ⬇️

... and 26 files with indirect coverage changes

@ambushwork
ambushwork merged commit 2383221 into release/2.15.0 Oct 28, 2024
@ambushwork
ambushwork deleted the yl/ga/fix-scrolling-freezing branch October 28, 2024 15:13
@ambushwork ambushwork mentioned this pull request Oct 31, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
@jonathanmos jonathanmos jonathanmos approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ambushwork @codecov-commenter @mariusc83 @jonathanmos