Skip to content

RUM-3455 Cleanup SR code #1910

Merged
xgouchet merged 1 commit into
developfrom
xgouchet/RUM-3455/fix_sr_bugs
Mar 15, 2024
Merged

RUM-3455 Cleanup SR code #1910
xgouchet merged 1 commit into
developfrom
xgouchet/RUM-3455/fix_sr_bugs

Conversation

@xgouchet

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR performs some cleanup in our Session Replay code

  • Use dependency injection instead of relying of default constructors for internal classes
  • Use interface instead of tight coupling with objects
  • Improves detection of background color based on drawables

Motivation

Originally, it was to better handle the mask layer in Ripple Drawables + the color of gradients when using tint (which resolves to a color filter).
And then I just tried to make the code cleaner and it impacted more in the end 😅

@xgouchet
xgouchet requested review from a team as code owners March 12, 2024 13:13
@xgouchet
xgouchet marked this pull request as draft March 12, 2024 14:32
* @param drawable the drawable to convert
* @return the color as an Int (in 0xAARRGGBB order), or null if the drawable is mostly invisible
*/
fun mapDrawableToColor(drawable: Drawable): Int?

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.

we can put @InternalApi annotation here

@xgouchet
xgouchet force-pushed the xgouchet/RUM-3455/fix_sr_bugs branch 5 times, most recently from 82a2bec to b109884 Compare March 14, 2024 09:59
@xgouchet
xgouchet marked this pull request as ready for review March 14, 2024 10:30
@xgouchet
xgouchet requested review from 0xnm and mariusc83 March 14, 2024 10:30

@0xnm 0xnm 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.

nice! I added few comments, but they are not blocking.

@xgouchet
xgouchet force-pushed the xgouchet/RUM-3455/fix_sr_bugs branch from b109884 to a5ca09d Compare March 14, 2024 13:02
@codecov-commenter

codecov-commenter commented Mar 14, 2024

Copy link
Copy Markdown

Codecov Report

Merging #1910 (a5ca09d) into develop (f46eb76) will increase coverage by 0.00%.
Report is 6 commits behind head on develop.
The diff coverage is 90.32%.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop    #1910    +/-   ##
=========================================
  Coverage    83.38%   83.38%            
=========================================
  Files          477      480     +3     
  Lines        16922    17159   +237     
  Branches      2514     2534    +20     
=========================================
+ Hits         14109    14307   +198     
- Misses        2120     2154    +34     
- Partials       693      698     +5     
Files Coverage Δ
...m/datadog/android/core/internal/utils/NumberExt.kt 0.00% <ø> (ø)
...essionreplay/material/MaskSliderWireframeMapper.kt 100.00% <100.00%> (ø)
...d/sessionreplay/material/MaskTabWireframeMapper.kt 100.00% <100.00%> (ø)
...sessionreplay/material/MaterialExtensionSupport.kt 100.00% <100.00%> (ø)
...id/sessionreplay/material/SliderWireframeMapper.kt 97.47% <100.00%> (-0.03%) ⬇️
...droid/sessionreplay/material/TabWireframeMapper.kt 98.33% <100.00%> (+0.22%) ⬆️
...adog/android/sessionreplay/SessionReplayPrivacy.kt 100.00% <100.00%> (ø)
...dog/android/sessionreplay/SessionReplayRecorder.kt 96.15% <100.00%> (+1.03%) ⬆️
.../sessionreplay/internal/recorder/MappingContext.kt 100.00% <100.00%> (ø)
...essionreplay/internal/recorder/SnapshotProducer.kt 88.24% <100.00%> (+0.36%) ⬆️
... and 46 more

... and 26 files with indirect coverage changes

@xgouchet
xgouchet merged commit 9859578 into develop Mar 15, 2024
@xgouchet
xgouchet deleted the xgouchet/RUM-3455/fix_sr_bugs branch March 15, 2024 09:42
@xgouchet xgouchet added this to the 2.7.0 milestone Apr 5, 2024
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.

4 participants