Skip to content

RUM-7631: Display captured text when the text has Ellipsis overflow#2446

Merged
ambushwork merged 1 commit into
developfrom
yl/compose/text-overflow
Dec 11, 2024
Merged

RUM-7631: Display captured text when the text has Ellipsis overflow#2446
ambushwork merged 1 commit into
developfrom
yl/compose/text-overflow

Conversation

@ambushwork

@ambushwork ambushwork commented Dec 10, 2024

Copy link
Copy Markdown
Member

What does this PR do?

On App side, if a text is too long, overflow can be defined to omit the overflow text, in Session Replay we need to display it correctly as much as possible:

Text( text = FAKE_LONG_TEXT,  overflow = TextOverflow.Ellipsis)

The PR use reflection to retrieve the real text which will be put in StaticLayout if the text overflow happens.

Note that if overflow = TextOverflow.Clip, we are not able to handle it.

Motivation

RUM-7631

Demo

Texts in yellow and red background are using TextOverflow.Ellipsis, Green one is using TextOverflow.Clip

Screenshot 2024-12-10 at 14 37 00

Anything else we should know when reviewing?

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 force-pushed the yl/compose/text-overflow branch from 835220c to 4e74cd1 Compare December 10, 2024 14:00
@codecov-commenter

codecov-commenter commented Dec 10, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 46.42857% with 15 lines in your changes missing coverage. Please review.

Project coverage is 70.05%. Comparing base (d737ab4) to head (11ccc65).
Report is 30 commits behind head on develop.

Files with missing lines Patch % Lines
...y/compose/internal/reflection/ComposeReflection.kt 0.00% 8 Missing ⚠️
...onreplay/compose/internal/utils/ReflectionUtils.kt 0.00% 6 Missing ⚠️
...ionreplay/compose/internal/utils/SemanticsUtils.kt 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2446      +/-   ##
===========================================
- Coverage    70.09%   70.05%   -0.04%     
===========================================
  Files          769      769              
  Lines        28509    28531      +22     
  Branches      4778     4789      +11     
===========================================
+ Hits         19981    19985       +4     
- Misses        7183     7207      +24     
+ Partials      1345     1339       -6
Files with missing lines Coverage Δ
...ernal/mappers/semantics/TextSemanticsNodeMapper.kt 90.00% <100.00%> (+2.00%) ⬆️
...ionreplay/compose/internal/utils/SemanticsUtils.kt 77.05% <87.50%> (+1.42%) ⬆️
...onreplay/compose/internal/utils/ReflectionUtils.kt 2.13% <0.00%> (-0.31%) ⬇️
...y/compose/internal/reflection/ComposeReflection.kt 0.00% <0.00%> (ø)

... and 29 files with indirect coverage changes

@ambushwork
ambushwork force-pushed the yl/compose/text-overflow branch from 4e74cd1 to f796202 Compare December 10, 2024 15:11
@ambushwork
ambushwork marked this pull request as ready for review December 10, 2024 15:12
@ambushwork
ambushwork requested review from a team as code owners December 10, 2024 15:12
0xnm
0xnm approved these changes Dec 10, 2024
View reviewed changes
Comment thread features/dd-sdk-android-session-replay-compose/consumer-rules.pro Outdated
Comment thread .../datadog/android/sessionreplay/compose/internal/mappers/semantics/TextSemanticsNodeMapper.kt Outdated
Comment thread ...c/test/kotlin/com/datadog/android/sessionreplay/compose/internal/utils/SemanticsUtilsTest.kt
Comment thread ...c/test/kotlin/com/datadog/android/sessionreplay/compose/internal/utils/SemanticsUtilsTest.kt Outdated
Comment thread ...c/test/kotlin/com/datadog/android/sessionreplay/compose/internal/utils/SemanticsUtilsTest.kt Outdated
mariusc83
mariusc83 approved these changes Dec 10, 2024
View reviewed changes

@mariusc83 mariusc83 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

@ambushwork
ambushwork force-pushed the yl/compose/text-overflow branch from f796202 to 7dc5f2c Compare December 10, 2024 15:45
@ambushwork
ambushwork force-pushed the yl/compose/text-overflow branch from 7dc5f2c to 11ccc65 Compare December 11, 2024 10:27
@ambushwork
ambushwork merged commit 8504c86 into develop Dec 11, 2024
@ambushwork
ambushwork deleted the yl/compose/text-overflow branch December 11, 2024 13:01
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
@0xnm 0xnm 0xnm 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.

5 participants

@ambushwork @codecov-commenter @mariusc83 @0xnm @jonathanmos