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-3684] Capture scroll record on shadow dom elements #2708

Merged

Conversation

N-Boutaib
Copy link
Contributor

Motivation

Add support of scrolls inside of shadow dom elements for session replay.

Changes

Listen to scroll events on shadow roots

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@N-Boutaib N-Boutaib changed the title [RUM-3684] Capture scroll record on shadow dom elements ✨ [RUM-3684] Capture scroll record on shadow dom elements Apr 15, 2024
Copy link

cit-pr-commenter bot commented Apr 15, 2024

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 156.53 KiB 156.53 KiB 6 B 0.00%
Logs 55.34 KiB 55.34 KiB 0 B 0.00%
Rum Slim 103.06 KiB 103.06 KiB 0 B 0.00%
Worker 25.21 KiB 25.21 KiB 0 B 0.00%

@N-Boutaib N-Boutaib marked this pull request as ready for review April 15, 2024 15:02
@N-Boutaib N-Boutaib requested a review from a team as a code owner April 15, 2024 15:02
Copy link
Contributor

@cy-moi cy-moi left a comment

Choose a reason for hiding this comment

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

Thanks for walking me through this!

.run(async ({ intakeRegistry }) => {
const div = await getNodeInsideShadowDom('my-scrollable-div', 'button')

await div.click()
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ question: ‏any reason to have a scroll triggered on click rather than scrolling directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, I just wanted to have a way to trigger the scroll once everything is setup (calling scrollTo on the element from the test isn't working because the returned wdio.Element doesn't allow this)

Copy link
Contributor

Choose a reason for hiding this comment

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

Making the test itself trigger a scroll would be easier to follow but if it is not possible, adding a comment that clarify the intent could help

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✔️

packages/rum/src/domain/record/record.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.40%. Comparing base (2045899) to head (e4802f7).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2708      +/-   ##
==========================================
+ Coverage   93.34%   93.40%   +0.05%     
==========================================
  Files         240      240              
  Lines        6986     6986              
  Branches     1540     1541       +1     
==========================================
+ Hits         6521     6525       +4     
+ Misses        465      461       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@N-Boutaib N-Boutaib requested a review from bcaudan April 17, 2024 10:01
@N-Boutaib N-Boutaib requested a review from bcaudan April 19, 2024 08:48
@N-Boutaib N-Boutaib requested a review from bcaudan April 19, 2024 12:36
Copy link
Contributor

@bcaudan bcaudan left a comment

Choose a reason for hiding this comment

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

LGTM!

@N-Boutaib N-Boutaib merged commit 75e8f93 into main Apr 22, 2024
17 checks passed
@N-Boutaib N-Boutaib deleted the najib.boutaib/RUM-3684-capture-scoll-records-on-shadow-root branch April 22, 2024 09:17
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